Ahmed: Onboarding tasks implemented #185
Conversation
elsaigh
changed the title
services/thermal_mgr/thermal_mgr.c
Outdated
| return ERR_CODE_INVALID_ARG; | ||
| } | ||
|
|
||
| xQueueSend(thermalMgrQueueHandle, event, (TickType_t) 0); |
There was a problem hiding this comment.
In general, you should always check the return value of non-void functions for an error code and handle it accordingly. In this case, return queue full if an error occurs
services/thermal_mgr/thermal_mgr.c
Outdated
| } | ||
|
|
||
| static void thermalMgr(void *pvParameters) { | ||
| static void thermalMgr(void *pvParameters) { // what is pvParameters used for? |
There was a problem hiding this comment.
This is just a comment, you can resolve this once you read it. pvParameters isn't used but i can be used to send values to the task on start at runtime that can't be done via macros at compile time
services/thermal_mgr/thermal_mgr.c
Outdated
| thermal_mgr_event_t osEvent = {0}; | ||
| osEvent.type = THERMAL_MGR_EVENT_OS_CMD; |
There was a problem hiding this comment.
| thermal_mgr_event_t osEvent = {0}; | |
| osEvent.type = THERMAL_MGR_EVENT_OS_CMD; | |
| thermal_mgr_event_t osEvent = {.type=THERMAL_MGR_EVENT_OS_CMD}; |
Just so you know, you can also do this.
Yarik-Popov
changed the title
services/thermal_mgr/thermal_mgr.c
Outdated
| return ERR_CODE_INVALID_STATE; | ||
| } | ||
|
|
||
| error_code_t sendErrorCode = xQueueSend(thermalMgrQueueHandle, event, (TickType_t) 0); |
There was a problem hiding this comment.
The xQueueSend function doesn't return an error_code_t but a different type of error. The easier way to handle this is just to move this so that its inside the if statement comparison so you don't need to care about the type of the return value
services/thermal_mgr/thermal_mgr.c
Outdated
| if (thermalMgrQueueHandle == NULL || event == NULL) { | ||
| return ERR_CODE_INVALID_STATE; | ||
| } |
There was a problem hiding this comment.
Event being null will still return invalid state
|
Lgtm, dm me your email and I'll add you to the notion |
Purpose
Would like to have my thermalMgr task reviewed as it is having some issue.
New Changes
Testing