-
Notifications
You must be signed in to change notification settings - Fork 179
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
Tyler Chen Onboarding #163
base: level3
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
/* LM75BD Registers (p.8) */ | ||
#define LM75BD_REG_CONF 0x01U /* Configuration Register (R/W) */ | ||
|
||
|
||
error_code_t lm75bdInit(lm75bd_config_t *config) { | ||
error_code_t errCode; | ||
|
||
|
@@ -25,9 +26,22 @@ error_code_t lm75bdInit(lm75bd_config_t *config) { | |
return ERR_CODE_SUCCESS; | ||
} | ||
|
||
#define CONF_WRITE_POINTER_BUFF_SIZE 1U | ||
error_code_t readTempLM75BD(uint8_t devAddr, float *temp) { | ||
/* Implement this driver function */ | ||
|
||
error_code_t errCode; | ||
uint8_t buff[CONF_WRITE_POINTER_BUFF_SIZE] = {0}; | ||
errCode = i2cSendTo(devAddr, buff, CONF_WRITE_POINTER_BUFF_SIZE); | ||
if (errCode != ERR_CODE_SUCCESS) return errCode; | ||
Comment on lines
+34
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use
Comment on lines
+34
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to use RETURN_IF_ERROR_CODE macro instead |
||
|
||
uint8_t temp_bytes[2U] = {0}; // first interpret data as unsigned | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are going to define a macro for the length above, probably best to define a macro here as well |
||
errCode = i2cReceiveFrom(devAddr, temp_bytes, 2); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use sizeof(temp_bytes) instead of 2. Also you should use the naming convention of camelCase |
||
if (errCode != ERR_CODE_SUCCESS) return errCode; | ||
Comment on lines
+38
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to use the RETURN_IF_ERROR_CODE macro instead |
||
|
||
int data = temp_bytes[0]; | ||
data -= 2 * ((1 << 7) & data); // handle signed bit | ||
data = (data << 3) | (temp_bytes[1] >> 5); // get LSbyte | ||
Comment on lines
+41
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code can be simplified to |
||
*temp = (float) data * 0.125; | ||
return ERR_CODE_SUCCESS; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ static StackType_t thermalMgrTaskStack[THERMAL_MGR_STACK_SIZE]; | |
|
||
#define THERMAL_MGR_QUEUE_LENGTH 10U | ||
#define THERMAL_MGR_QUEUE_ITEM_SIZE sizeof(thermal_mgr_event_t) | ||
#define THERMAL_OVERTEMPERATURE_THRESHOLD 80.0 | ||
#define THERMAL_HYSTERESIS_THRESHOLD 75.0 | ||
|
||
static QueueHandle_t thermalMgrQueueHandle; | ||
static StaticQueue_t thermalMgrQueueBuffer; | ||
|
@@ -38,23 +40,43 @@ void initThermalSystemManager(lm75bd_config_t *config) { | |
thermalMgrQueueHandle = xQueueCreateStatic( | ||
THERMAL_MGR_QUEUE_LENGTH, THERMAL_MGR_QUEUE_ITEM_SIZE, | ||
thermalMgrQueueStorageArea, &thermalMgrQueueBuffer); | ||
|
||
} | ||
|
||
error_code_t thermalMgrSendEvent(thermal_mgr_event_t *event) { | ||
/* Send an event to the thermal manager queue */ | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. check to make sure event is not |
||
xQueueSend(thermalMgrQueueHandle, event, portMAX_DELAY); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Always check the return value for non-void functions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, since you are calling this function in an ISR, the block time should be 0 since you should never be blocking in an ISR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for this:
Also this should be 0 and not PORTMAX_DELAY as this will block the task which might not be the intended behavior especially in ISR |
||
return ERR_CODE_SUCCESS; | ||
} | ||
|
||
void osHandlerLM75BD(void) { | ||
/* Implement this function */ | ||
thermal_mgr_event_t event; | ||
event.type = THERMAL_MGR_EVENT_OS_INTERRUPT; | ||
thermalMgrSendEvent(&event); | ||
} | ||
|
||
static void thermalMgr(void *pvParameters) { | ||
/* Implement this task */ | ||
while (1) { | ||
|
||
thermal_mgr_event_t event; | ||
if (xQueueReceive(thermalMgrQueueHandle, &event, portMAX_DELAY)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Explicitly check against pdTRUE |
||
const lm75bd_config_t data = *(lm75bd_config_t *) pvParameters; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be outside of the loop so we don't constantly re create it the data and better to use a pointer instead of a regular struct |
||
float temperature; | ||
readTempLM75BD(data.devAddr, &temperature); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, you should be checking the return of non-void functions for error codes. This should be wrapped inside of a LOG_IF_ERROR_CODE and if(errCode!= success) continue; |
||
switch (event.type) { | ||
case THERMAL_MGR_EVENT_MEASURE_TEMP_CMD: | ||
addTemperatureTelemetry(temperature); | ||
break; | ||
case THERMAL_MGR_EVENT_OS_INTERRUPT: | ||
if (temperature > THERMAL_OVERTEMPERATURE_THRESHOLD) { | ||
overTemperatureDetected(); | ||
} | ||
if (temperature < THERMAL_HYSTERESIS_THRESHOLD) { | ||
safeOperatingConditions(); | ||
} | ||
break; | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
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.
Check that the temp is not null before use