search

CalgaryToSpace / CTS-SAT-1-OBC-Firmware

Firmware for the CTS-SAT-1 ("FrontierSat") mission. Runs on the STM32-based Onboard Computer.
MIT License
7 stars 0 forks source link

Telecommands to disable and enable the heartbeat #34

Closed JohnathanBurchill closed 3 months ago

JohnathanBurchill commented 3 months ago

Two convenience telecommands for adjusting the verbosity of the heartbeat telemetry while developing firmware.

DeflateAwning commented 3 months ago

Fantastic! Didn't even know I wanted this! :)

  1. Would you mind rebasing off of main (git checkout main, git pull, git checkout burchill-heartbeat-telecommands, git rebase main, resolve any conflicts [utilize ChatGPT if confused], git push -f)? It says there's a confict here, which suggests you just need to rebase.

  2. Could you also please change the type of the volatile configuration variable from int to uint8_t.

Thanks :)

JohnathanBurchill commented 3 months ago

Will do. Also, will rename to "heartbeat_on" and "heartbeat_off", slightly shorter versions which start with something more descriptive.

JohnathanBurchill commented 3 months ago

Is the workflow at this point to push the commit which addresses your points? Or would I squash first?

JohnathanBurchill commented 3 months ago

Went ahead and rebased to fix that conflict.

DeflateAwning commented 3 months ago

Is the workflow at this point to push the commit which addresses your points? Or would I squash first?

Doesn't matter a ton either way here! Seems most open source communities have their own preferences with those practices.

In larger PRs, I suppose the ideal way is to do an interactive rebase where you squash commits into just a few meaningful commit messages, leaving changes from reviews as an additional commit per review.

DeflateAwning commented 3 months ago

I made a tiny tweak to re-enable the LED blinking regardless of the heartbeat UART status, as it's a very helpful way to identify if a task/thread has stalled/failed to yield for a while. Industry-standard practice for RTOS systems.

Squash and merge when you're ready!

JohnathanBurchill commented 3 months ago

That makes sense.