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.

Sign up for GitHub

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

Jump to bottom

Fatma J Onboarding #196

Open
wants to merge 2 commits into
base: level3
Choose a base branch
from
Open

Fatma J Onboarding #196

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83901ae
did the thing
TheRealGecko Nov 1, 2024
7689834
made requested changes
TheRealGecko Nov 4, 2024
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
1 change: 1 addition & 0 deletions Testing/Temporary/CTestCostData.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
---
3 changes: 3 additions & 0 deletions Testing/Temporary/LastTest.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Start testing: Oct 31 19:30 EDT
----------------------------------------------------------
End testing: Oct 31 19:30 EDT
12 changes: 11 additions & 1 deletion lm75bd/lm75bd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,17 @@ error_code_t lm75bdInit(lm75bd_config_t *config) {

error_code_t readTempLM75BD(uint8_t devAddr, float *temp) {
Yarik-Popov marked this conversation as resolved.
Show resolved Hide resolved
/* Implement this driver function */

uint8_t sendBuf[1] = {0x0U};
uint8_t buffer[2];
Copy link

Choose a reason for hiding this comment

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

Initialize the buffer

i2cSendTo (devAddr, sendBuf, 1U);
i2cReceiveFrom (devAddr, buffer, 2U);
Copy link

Choose a reason for hiding this comment

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

  • wrap both lines inside of a RETURN_IF_ERROR_CODE
  • use sizeof(sendBuf) instead of 1
  • use sizeof(buffer) instead of 2

uint8_t a = 1U;
uint8_t mask = a << 7;
if (!(buffer[0] & mask)) {
*temp = ((buffer[0] << 3) | (buffer[1] >> 5)) * 0.125;
} else {
*temp = -(((~((buffer[0] << 3) | (buffer[1] >> 5))) & (0b0000011111111111)) + 1) * 0.125;
}
Copy link

Choose a reason for hiding this comment

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

the code is overly complicated

Suggested change
uint8_t a = 1U;
uint8_t mask = a << 7;
if (!(buffer[0] & mask)) {
*temp = ((buffer[0] << 3) | (buffer[1] >> 5)) * 0.125;
} else {
*temp = -(((~((buffer[0] << 3) | (buffer[1] >> 5))) & (0b0000011111111111)) + 1) * 0.125;
}
int16_t temperature = ((buffer[0] << 8) | buffer[1]) >> 5;
*temp = temperature * 0.125;

You might need to do some casting to get this to work

return ERR_CODE_SUCCESS;
}

Expand Down
3 changes: 2 additions & 1 deletion lm75bd/lm75bd.h
View file
Open in desktop
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 */

/* LM75BD Configuration Values */
#define LM75BD_DEV_OP_MODE_NORMAL 0x00U
Expand Down
16 changes: 15 additions & 1 deletion services/thermal_mgr/thermal_mgr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,32 @@ void initThermalSystemManager(lm75bd_config_t *config) {

error_code_t thermalMgrSendEvent(thermal_mgr_event_t *event) {
/* Send an event to the thermal manager queue */
xQueueSend(thermalMgrQueueHandle, event, 0); // why u throwing error
Copy link

Choose a reason for hiding this comment

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

  • check to make sure that thermal mgr queue isn't null before use, returning invalid state
  • check to make sure that the event isn't null
  • in general, you should be checking the return value of non void functions for errors and handling them accordingly


return ERR_CODE_SUCCESS;
}

void osHandlerLM75BD(void) {
/* Implement this function */
float temp;
readTempLM75BD(LM75BD_OBC_I2C_ADDR, &temp);

if(temp > 80) {
overTemperatureDetected();
} else if(temp < 75) {
Copy link

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 instead of hardocding the values

safeOperatingConditions();
}
Copy link

Choose a reason for hiding this comment

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

Do not read inside the os handler as it may block. Send an event and have the thermalMgr task function handle it

}

static void thermalMgr(void *pvParameters) {
/* Implement this task */
while (1) {

thermal_mgr_event_t data = *(thermal_mgr_event_t *) pvParameters;
Copy link

Choose a reason for hiding this comment

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

PvParameters is not of type thermal mgr event but of the lm75bd config so you shouldn't do this. Just thermal_mgr_event_t event={0};

if(xQueueReceive(thermalMgrQueueHandle, pvParameters, 0) == pdTRUE && data.type == THERMAL_MGR_EVENT_MEASURE_TEMP_CMD) { // ensure about the tick time + how to checl the type ajsdkfk
Copy link

Choose a reason for hiding this comment

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

  • Use portMAX_DELAY instead of 0
  • you should be logging if an invalid event was received

float temp;
readTempLM75BD(LM75BD_OBC_I2C_ADDR, &temp);
Copy link

Choose a reason for hiding this comment

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

Check the return value for an error code, log it if it exists and continue. we don't want to add a temp value that failed to read we don't know if temp will be a valid value

addTemperatureTelemetry(temp);
}
}
}

Expand Down
1 change: 0 additions & 1 deletion test/test_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ TEST(TestLm75bdDriver, TestReadTempLm75bdPosTempSuccess) {
customTempBuff[1] = 0x00U;

i2cReceiveFrom_fake.custom_fake = getTemp_custom_fake;

ASSERT_EQ(readTempLM75BD(addr, &temp), ERR_CODE_SUCCESS);
EXPECT_EQ(temp, 127.0f);
}
Expand Down
Loading