Shiv Krishnaswamy FW Onboarding #206
Conversation
lm75bd/lm75bd.c
Outdated
| @@ -27,7 +27,17 @@ error_code_t lm75bdInit(lm75bd_config_t *config) { | |||
|
|
|||
| error_code_t readTempLM75BD(uint8_t devAddr, float *temp) { | |||
| /* Implement this driver function */ | |||
|
|
|||
| error_code_t errCode; | |||
| uint8_t buf[2]; | |||
There was a problem hiding this comment.
good practice to initialize arrays
lm75bd/lm75bd.c
Outdated
|
|
||
| error_code_t errCode; | ||
| uint8_t buf[2]; | ||
| uint8_t tempReg = 0b00000000; |
There was a problem hiding this comment.
bits should be converted to hex for readability
lm75bd/lm75bd.c
Outdated
| RETURN_IF_ERROR_CODE(i2cSendTo(devAddr, &tempReg, 1)); | ||
| RETURN_IF_ERROR_CODE(i2cReceiveFrom(devAddr, buf, 2)); |
There was a problem hiding this comment.
nit: use sizeof here instead of hardcoding the size as 1 and 2
lm75bd/lm75bd.h
Outdated
| @@ -5,7 +5,7 @@ | |||
| #include <stdint.h> | |||
|
|
|||
| /* LM75BD I2C Device Address */ | |||
| #define LM75BD_OBC_I2C_ADDR /* Define the address here */ | |||
| #define LM75BD_OBC_I2C_ADDR 0b1001111 /* Define the address here */ | |||
| @@ -27,7 +27,17 @@ error_code_t lm75bdInit(lm75bd_config_t *config) { | |||
|
|
|||
| error_code_t readTempLM75BD(uint8_t devAddr, float *temp) { | |||
| /* Implement this driver function */ | |||
|
|
|||
There was a problem hiding this comment.
check if pointer arguments are null, if they are return invalid arg error code
| @@ -43,19 +43,36 @@ void initThermalSystemManager(lm75bd_config_t *config) { | |||
|
|
|||
| error_code_t thermalMgrSendEvent(thermal_mgr_event_t *event) { | |||
| /* Send an event to the thermal manager queue */ | |||
|
|
|||
| error_code_t errCode; | |||
| if(xQueueSend(thermalMgrQueueHandle, event, (TickType_t)0) == pdFALSE) | |||
There was a problem hiding this comment.
check if the queue has been initialized yet, return error code invalid state if it hasn't
services/thermal_mgr/thermal_mgr.c
Outdated
|
|
||
| error_code_t errCode; | ||
| if(xQueueSend(thermalMgrQueueHandle, event, (TickType_t)0) == pdFALSE) | ||
| return ERR_CODE_UNKNOWN; |
There was a problem hiding this comment.
if the queue send fails, it means that the queue is full (we have an error code for that)
services/thermal_mgr/thermal_mgr.c
Outdated
| while (1) { | ||
| if (xQueueReceive(thermalMgrQueueHandle, &event, (TickType_t) 0) == pdTRUE) { | ||
| float temp; | ||
| readTempLM75BD(LM75BD_OBC_I2C_ADDR, &temp); |
There was a problem hiding this comment.
you should be checking if the driver function fails, if it does you need to log it
services/thermal_mgr/thermal_mgr.c
Outdated
| /* Implement this task */ | ||
| thermal_mgr_event_t event; | ||
| while (1) { | ||
| if (xQueueReceive(thermalMgrQueueHandle, &event, (TickType_t) 0) == pdTRUE) { |
There was a problem hiding this comment.
when using event queues and an infinite loop in FreeRTOS, you want to set the receive wait period to portmax delay so that it blocks indefinitely until an event is received
services/thermal_mgr/thermal_mgr.c
Outdated
| if (event.type == THERMAL_MGR_EVENT_MEASURE_TEMP_CMD) { | ||
| addTemperatureTelemetry(temp); | ||
| }else if (event.type == THERMAL_MGR_EVENT_INTERRUPT_MEASURE) { | ||
| if (temp > LM75BD_DEFAULT_HYST_THRESH) | ||
| overTemperatureDetected(); | ||
| else | ||
| safeOperatingConditions(); | ||
| } |
There was a problem hiding this comment.
log an error if the event type is unknown (just add an else statement)
Purpose
Explain the purpose of the PR here, including references to any existing Notion tasks.
New Changes
Testing
Outstanding Changes