Skip to content
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

Prepare for PR #195

Open
wants to merge 4 commits into
base: level3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions lm75bd/lm75bd.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,33 @@ error_code_t lm75bdInit(lm75bd_config_t *config) {
return ERR_CODE_SUCCESS;
}

/**
* @brief Converts data from LM75B temperature sensor to Celcius
*
* @param devAddr - I2C device address of sensor
* @param temp - Pointer to store converted temperature value in Celsius
* @return ERR_CODE_SUCCESS if reading and conversion succeed
*/
error_code_t readTempLM75BD(uint8_t devAddr, float *temp) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check that temp isnt null before use

/* Implement this driver function */


// Stores temperature register value
uint8_t sendBuffer = 0x0U;
uint8_t buffer[2];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initialize the buffer

uint16_t temperature = 0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this down to after the i2c calls as this variable will not be used if the calls fails with the RETURN_IF_ERROR_CODE macro. Also, make this an int16_t


// Use pointer register to select internal temperature register
i2cSendTo(devAddr, &sendBuffer, 1U);

// Get data on temperature sensor
i2cReceiveFrom(devAddr, buffer, 2U);
Comment on lines +43 to +46

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Wrap both calls inside the RETURN_IF_ERROR_CODE macro
  • Use sizeof(sendBuffer) instead of 1
  • Use sizeof(buffer) instead of 2


// Convert buffer to one binary integer
temperature = ((buffer[0] << 8) | buffer[1]) >> 5;

if((temperature >> 10) == 0) *temp = (float)(temperature * 0.125);
// 0xFFFF deletes extra ones from flipped temperature data
else *temp = -(((~temperature) & (0xFFFF >> 5)) + 1)*0.125;
Comment on lines +51 to +53

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If temperature is a int16_t then you don't need to do these lines just *temp=(float)(temperature * 0.125); will do


return ERR_CODE_SUCCESS;
}

Expand Down
3 changes: 2 additions & 1 deletion lm75bd/lm75bd.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
#include <stdint.h>

/* LM75BD I2C Device Address */
#define LM75BD_OBC_I2C_ADDR /* Define the address here */
#define LM75BD_OBC_I2C_ADDR 0x4FU /* Define the address here */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just use the hex value without the U ending of the team

// A0, A1, A2 all connected to VCC

/* LM75BD Configuration Values */
#define LM75BD_DEV_OP_MODE_NORMAL 0x00U
Expand Down
30 changes: 28 additions & 2 deletions services/thermal_mgr/thermal_mgr.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,46 @@ void initThermalSystemManager(lm75bd_config_t *config) {

}

/**
* @brief Sends event to thermal manager queue
*
* @param event - Pointer to thermal manager event to send
* @return ERR_CODE_SUCCESS if the event is successfully sent
*/
error_code_t thermalMgrSendEvent(thermal_mgr_event_t *event) {
/* Send an event to the thermal manager queue */

xQueueSend(thermalMgrQueueHandle, event, 0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • In general, you should be checking that pointers aren't null before use. So check that event isn't null
  • Check that thermal mgr queue handle isn't null, returning invalid state
  • You should be checking the return value of non-void functions for error codes

return ERR_CODE_SUCCESS;
}

/**
* @brief OS interrupt handler for LM75B temperature sensor
*
*/
void osHandlerLM75BD(void) {
/* Implement this function */
float temperature;
readTempLM75BD(LM75BD_OBC_I2C_ADDR, &temperature);
if(temperature > 75.0){ // only below T hys is safe

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the macros defined in the lm75bd.h file instead of hardcoding 75

overTemperatureDetected();
}
else safeOperatingConditions();
Comment on lines +62 to +67

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't be reading the temperature inside of the isr as the i2c calls are blocking. Just send an event and have the the thermal mgr task handle the event

}

/**
* @brief Thermal management task for temperature events
*
* @param pvParameters - Pointer to parameters passed to task
*/
static void thermalMgr(void *pvParameters) {
/* Implement this task */
while (1) {

thermal_mgr_event_t data = *(thermal_mgr_event_t *) pvParameters;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, pvParameters isnt of type thermal_mgr_event_t but of the config which isn't the same. So I would just change the line to initialize an empty event instead

if(xQueueReceive(thermalMgrQueueHandle, pvParameters, 0) == pdTRUE && data.type == THERMAL_MGR_EVENT_MEASURE_TEMP_CMD){
float temperature;
readTempLM75BD(LM75BD_OBC_I2C_ADDR, &temperature);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log the error code returned

addTemperatureTelemetry(temperature);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't get to this line if the readTemp fails

}
}
}

Expand Down
1 change: 1 addition & 0 deletions sys/logging/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ void logSetLevel(log_level_t newLogLevel);
* @return error_code_t
*
*/

error_code_t logLog(log_level_t msgLevel, const char *file, uint32_t line, const char *s, ...);

#ifdef __cplusplus
Expand Down
Loading