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

Implement Thermal Mgr Task #285

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

sardormirk
Copy link
Contributor

Purpose

Implement Thermal Manager Task
Created a skeleton thermal manager task function to replace the health collector

New Changes

  • Created a new thermal manager module to replace health collector
  • Added the task configuration to obc_scheduler_config
  • added necessary declarations to obc watchdog and digital watchdog

Outstanding Changes

  • Needs to measure other ICs (not just LM75BD temp sensor) and log their telemetry data
  • Needs to add alerts for fault conditions
  • Needs algorithm to control battery heater

Copy link

Pull reviewers stats

Stats of the last 120 days for UWOrbital:

User Total reviews Time to review Total comments
dgobalak 25 1d 5h 24m 134
Navtajh04 18 4d 7h 34m 108
Yarik-Popov 12 6d 11h 3m 90
PratyushMakkar 5 7d 14h 7m 22
Gjjjiang 4 4d 15h 52m 14
manumanuk 2 35d 14h 41m 5
twilkhoo 1 4m 0
JoelManYunLee 1 17h 57m 4
kepler452b123 1 7d 4h 25m 11
⚡️ Pull request stats


static obc_error_code_t collectObcLm75bdTemp(void);
void obcTaskInitThermalMgr() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This task should init the lm75bd


#define HEALTH_COLLECTION_PERIOD_MS 60000UL
#define THERMAL_MGR_PERIOD_MS 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on our telemetry sheet, it should take the temperature once every minute, not once a sec. Also add U to the end

#define TASK_HEALTH_COLLECTOR_WATCHDOG_TIMEOUT portMAX_DELAY
#define TASK_THERMAL_MGR_WATCHDOG_TIMEOUT pdMS_TO_TICKS(1000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this with changed tick delay


static obc_error_code_t collectObcLm75bdTemp(void);
void obcTaskInitThermalMgr() {}
static obc_error_code_t collectObcLm75bdTemp(void);
Copy link
Contributor

Choose a reason for hiding this comment

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

Push this above the obcTaskInit function

@Yarik-Popov Yarik-Popov changed the title fixed thermal mgr task hopefully no merge conflicts this time Implement thermal mgr task Mar 30, 2024
@Yarik-Popov Yarik-Popov changed the title Implement thermal mgr task Implement Thermal Mgr Task Mar 30, 2024
@@ -23,7 +23,7 @@
#define TASK_PAYLOAD_MGR_WATCHDOG_TIMEOUT portMAX_DELAY
#define TASK_TIMEKEEPER_WATCHDOG_TIMEOUT portMAX_DELAY
#define TASK_ALARM_MGR_WATCHDOG_TIMEOUT portMAX_DELAY
#define TASK_HEALTH_COLLECTOR_WATCHDOG_TIMEOUT portMAX_DELAY
#define TASK_THERMAL_MGR_WATCHDOG_TIMEOUT pdMS_TO_TICKS(1000)
Copy link
Contributor

Choose a reason for hiding this comment

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

watchdog timeout shouldl generally be 2x task rate


while (1) {
LOG_IF_ERROR_CODE(collectObcLm75bdTemp());
vTaskDelay(pdMS_TO_TICKS(HEALTH_COLLECTION_PERIOD_MS));
Copy link
Contributor

Choose a reason for hiding this comment

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

we should be implementing these mailboxes in this pr https://discord.com/channels/831191521595621387/1124186460866740314/1181054461494444082

Don;t need to have other tasks sending anything to the queue right now but the queues and thermal mgr side shpuld be set up

}

static obc_error_code_t collectObcLm75bdTemp(void) {
static obc_error_code_t collectObcLm75bdTemp(void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: fix indent


static obc_error_code_t collectObcLm75bdTemp(void);
void obcTaskInitThermalMgr() {}
static obc_error_code_t collectObcLm75bdTemp(void);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: fix indent

@@ -36,4 +45,5 @@ static obc_error_code_t collectObcLm75bdTemp(void) {
RETURN_IF_ERROR_CODE(addTelemetryData(&obcTempVal));
Copy link
Contributor

Choose a reason for hiding this comment

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

make telem a flag that is passed in since we may want to just read the value sometimes to make sure we are safe without adding that as telemetry

@superkor superkor self-assigned this Oct 26, 2024
@@ -171,3 +171,5 @@ obc_error_code_t cc1120GetState(cc1120_state_t *stateNum);
* @return obc_error_code_t - Whether or not the setup was a success
*/
obc_error_code_t cc1120Init(void);

obc_error_code_t readTempCC1120(float *temp);
Copy link

Choose a reason for hiding this comment

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

static obc_error_code_t collectObcCC1120Temp(void);
static obc_error_code_t setObcCC1120TempReading(void);
static obc_error_code_t resetObcCC120TempConfig(void);
Copy link

Choose a reason for hiding this comment

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

This will be handled in a different PR (driver functions are being written in it - not in the scope of this PR)

LOG_IF_ERROR_CODE(collectObcDs3232Temp());
LOG_IF_ERROR_CODE(collectObcDs3232Temp()); // RTC
// BMS in pr https://github.com/UWOrbital/OBC-firmware/pull/187

Copy link

Choose a reason for hiding this comment

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

BMS driver code does not exist; still in PR #187

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants