Skip to content

Add Thermal Limit Events to Thermal Manager#364

Merged
JamesDCowley merged 7 commits intomainfrom
thermal-limit-events
Mar 31, 2026
Merged

Add Thermal Limit Events to Thermal Manager#364
JamesDCowley merged 7 commits intomainfrom
thermal-limit-events

Conversation

@JamesDCowley
Copy link
Copy Markdown
Contributor

@JamesDCowley JamesDCowley commented Mar 26, 2026

Add Thermal Limit Events to Thermal Manager

Description

Adds four events in Thermal Manager for when the face and battery temperature sensors read above or below a certain temperature. These thresholds are set with a parameter to the Thermal Manager.

These events can be used to log when the temperature is above or below a given temperature.

Related Issues/Tickets

How Has This Been Tested?

  • Unit tests
  • Integration tests
  • Z Tests
  • Manual testing (describe steps)

Screenshots / Recordings (if applicable)

Checklist

  • Written detailed sdd with requirements, channels, ports, commands, telemetry defined and correctly formatted and spelled
  • Have written relevant integration tests and have documented them in the sdd
  • Have done a code review with
  • Have tested this PR on every supported board with correct board definitions

Further Notes / Considerations

Comment on lines +5 to +16
### Parameters ###
@ Parameter for face temperature lower threshold in °C
param FACE_TEMP_LOWER_THRESHOLD: F64 default -20.0 id 0

@ Parameter for face temperature upper threshold in °C
param FACE_TEMP_UPPER_THRESHOLD: F64 default 60.0 id 1

@ Parameter for battery cell temperature lower threshold in °C
param BATT_CELL_TEMP_LOWER_THRESHOLD: F64 default -20.0 id 2

@ Parameter for battery cell temperature upper threshold in °C
param BATT_CELL_TEMP_UPPER_THRESHOLD: F64 default 60.0 id 3
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Mikefly123 How does this range look?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does look great to me! I will note though that on the Yearling satellites we consistently saw Face temperatures around -34C in eclipse, so I recommend throttling this event or adding some logic that will debounce it so we don't get too much spam.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

At what temps did we start seeing issues with hairsat?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the reminder on that actually! I would recommend changing that battery low temp threshold to something much more conservative, like 5C. If Lithium Ion batteries go below 0C they can still discharge but can suffer long term damage if you charge them at freezing temperatures.

As for the face temperatures, we still can generally read stalely down to -40C. Below that you start running into issues!

Copy link
Copy Markdown
Collaborator

@nateinaction nateinaction left a comment

Choose a reason for hiding this comment

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

Looks correct, waiting on @Mikefly123 for thoughts on default values.

@JamesDCowley JamesDCowley force-pushed the thermal-limit-events branch from 930b777 to 4b3b232 Compare March 27, 2026 20:41
@JamesDCowley JamesDCowley marked this pull request as ready for review March 31, 2026 00:02
@JamesDCowley JamesDCowley enabled auto-merge (squash) March 31, 2026 00:14
@JamesDCowley JamesDCowley merged commit 2df892f into main Mar 31, 2026
4 checks passed
@JamesDCowley JamesDCowley deleted the thermal-limit-events branch March 31, 2026 00:20
@github-project-automation github-project-automation bot moved this to Done in V1.X.X Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants