Sardor/thermal mgr task #283
Conversation
Merge is necessary to PR temp sensor test code
…figuration values
…al/OBC-firmware into sardor/temp-sensor-tests
There was a problem hiding this comment.
Check to see if there are any changes you made relative to the existing temp testing merged in by #216.
|
|
||
| static obc_error_code_t collectObcLm75bdTemp(void); | ||
|
|
||
| void obcTaskInitThermalMgr(void) { |
There was a problem hiding this comment.
This should init the lm75bd not the state manager
| while (1) { | ||
| LOG_IF_ERROR_CODE(collectObcLm75bdTemp()); | ||
| feedDigitalWatchdog(); | ||
| vTaskDelayUntil(&xLastWakeTime, pdMS_TO_TICKS(THERMAL_MGR_PERIOD_MS)); | ||
| } |
There was a problem hiding this comment.
This should receive from the queue and then handle each event accordingly.
There was a problem hiding this comment.
This task shouldn't need an event queue. It just periodically runs (similar to the gnc_mgr task). I'd just remove all the code related to events/queues from this task
| /* payload queue config */ | ||
| #define THERMAL_MANAGER_QUEUE_LENGTH 10U | ||
| #define THERMAL_MANAGER_QUEUE_ITEM_SIZE sizeof(thermal_mgr_event_t) | ||
| #define THERMAL_MANAGER_QUEUE_RX_WAIT_PERIOD pdMS_TO_TICKS(10) | ||
| #define THERMAL_MANAGER_QUEUE_TX_WAIT_PERIOD pdMS_TO_TICKS(10) |
There was a problem hiding this comment.
These should be moved to the .c file as they shouldn't be available to caller of module
| @@ -97,7 +97,7 @@ void obcTaskFunctionStateMgr(void *pvParameters) { | |||
| obcSchedulerInitTask(OBC_SCHEDULER_CONFIG_ID_COMMS_DOWNLINK_ENCODER); | |||
| obcSchedulerInitTask(OBC_SCHEDULER_CONFIG_ID_EPS_MGR); | |||
| obcSchedulerInitTask(OBC_SCHEDULER_CONFIG_ID_PAYLOAD_MGR); | |||
| obcSchedulerInitTask(OBC_SCHEDULER_CONFIG_ID_HEALTH_COLLECTOR); | |||
| obcSchedulerInitTask(OBC_SCHEDULER_CONFIG_ID_THERMAL_MGR); | |||
There was a problem hiding this comment.
Move this to the top of the schedule init section
| @@ -23,7 +23,7 @@ | |||
| #define TASK_PAYLOAD_MGR_WATCHDOG_TIMEOUT portMAX_DELAY | |||
| #define TASK_TIMEKEEPER_WATCHDOG_TIMEOUT portMAX_DELAY | |||
| #define TASK_ALARM_MGR_WATCHDOG_TIMEOUT portMAX_DELAY | |||
| #define TASK_HEALTH_COLLECTOR_WATCHDOG_TIMEOUT portMAX_DELAY | |||
| #define TASK_THERMAL_MGR_WATCHDOG_TIMEOUT portMAX_DELAY | |||
There was a problem hiding this comment.
This should be 2x the period of the thermal mgr task
|
|
||
| while (1) { | ||
| LOG_IF_ERROR_CODE(collectObcLm75bdTemp()); | ||
| feedDigitalWatchdog(); |
There was a problem hiding this comment.
Use digitalWatchdogTaskCheckIn (instead of feedDigitalWatchdog) to check in with the watchdog task. Look at gnc_mgr as an example
| while (1) { | ||
| LOG_IF_ERROR_CODE(collectObcLm75bdTemp()); | ||
| feedDigitalWatchdog(); | ||
| vTaskDelayUntil(&xLastWakeTime, pdMS_TO_TICKS(THERMAL_MGR_PERIOD_MS)); | ||
| } |
There was a problem hiding this comment.
This task shouldn't need an event queue. It just periodically runs (similar to the gnc_mgr task). I'd just remove all the code related to events/queues from this task
There was a problem hiding this comment.
Didn't we already merge in the lm75bd test code? This should be removed from this PR.
See https://github.com/UWOrbital/OBC-firmware/tree/main/obc/app/tools/interface_debug_tool
|
|
||
| #include <gio.h> | ||
|
|
||
| #define THERMAL_MGR_PERIOD_MS 60 |
There was a problem hiding this comment.
Is this task supposed to run every 60ms?
cc @Navtajh04
There was a problem hiding this comment.
It was to be determined later, I just put 60 as a place holder
There was a problem hiding this comment.
Based on our telemetry it seems that we read temperature values very min
| #include <os_task.h> | ||
| #include <os_queue.h> | ||
|
|
||
| #include <gio.h> |
There was a problem hiding this comment.
What're we using gio.h for here?
|
Being handled by #285 |
Purpose
Implement Thermal Manager Task
Created a skeleton thermal manager task function to replace the health collector
New Changes
Outstanding Changes