-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FWXV 246] Added Files for Review #242
[FWXV 246] Added Files for Review #242
Conversation
bps_indicator.h
Outdated
@@ -0,0 +1,15 @@ | |||
#ifndef BPS_INDICATOR_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use #pragma once instead of #ifndef for our header files
bps_indicator.h
Outdated
#include "outputs.h" | ||
|
||
// callback function to periodically toggle the state of the bps fault indicator | ||
static void prv_callback(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static functions should not be in header files.
bps_indicator.h
Outdated
static void prv_callback(); | ||
|
||
// functions to start/stop bps fault indicator | ||
static void start_bps_fault_indicator(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of leaving the brackets empty in these methods, it's better to use the void keyword. We also need to remove the static keyword here.
bps_indicator.c
Outdated
|
||
static void stop_bps_fault_indicator() { | ||
s_bps_fault_enabled = false; | ||
soft_timer_cancel(&s_bps_indicator_timer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: fix the indent.
Also, we don't need this cancel method, since we want the callback to be called a final time to turn off the hazard lights
Added files for review for my first task, please let me know of any issues/points to fix!