Heartbeat Refactor #1325
Heartbeat Refactor #1325
Conversation
| #include "app_canTx.h" | ||
| #include "app_canAlerts.h" | ||
|
|
||
| HeartbeatMonitorBoard vc_hbmonitor = { |
There was a problem hiding this comment.
Nit: This should probably be named something more consistent with the naming scheme on the rest of the codebase (VC_heartbeat_monitor or VC_hb_monitor?)
|
|
||
| void app_heartbeatMonitor_checkIn(void) | ||
| { | ||
| app_canTx_BMS_Heartbeat_set(true); |
There was a problem hiding this comment.
We can pass the heartbeat setter function to HeartbeatMonitorBoard, so that the hb monitor library can handle this step - then we can expose a method like app_heartbeatMonitorBoard_tick rather than app_heartbeatMonitorBoard_checkIn...
I'm just thinking that for the next dev using the heartbeat monitor, it seems like a reasonable mistake to forget to broadcast the heartbeat manually.
|
|
||
| void app_heartbeatMonitor_broadcastFaults(void) | ||
| { | ||
| if (block_faults) |
There was a problem hiding this comment.
fault blocking can also be set as a flag in the HeartbeatMonitorBoard struct if we want the end developer to use a flag to disable faults rather than manually checking it.
| } | ||
|
|
||
| #ifdef TARGET_TEST | ||
| void app_heartbeatMonitor_clearFaults(void) |
There was a problem hiding this comment.
Can we put this target check in the heartbeat monitor library?
| { | ||
| block_faults = block; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
I think that we can move a lot of the logic in this intermediate heartbeat layer to the heartbeat monitor library, that way the only thing left here will be configuration. Should help the next dev when they need to port a board (this looks like a site for a lot of potential mistakes).
| @@ -0,0 +1,84 @@ | |||
| #include "app_heartbeatMonitor.h" | |||
There was a problem hiding this comment.
Same notes from BMS apply here - I think we can move any remaining logic in this file and move it to the heartbeat monitor library, making this just a configuration file.
|
|
||
| static bool block_faults = false; | ||
|
|
||
| void app_heartbeatMonitor_init(bool block_faults_init) |
There was a problem hiding this comment.
The init wrapper here is nice though - we should keep this method here.
Lucien950
added
the
Build System/DevEx
Changelist
firmware/shared/src/app/app_heartbeatMonitorBoard.h)Testing Done
Resolved Tickets