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

[bootloader] Implement bootloader reset reasons #2773

Merged
merged 4 commits into from
May 28, 2024

Conversation

scott-brust
Copy link
Member

Problem

The RTL bootloader has not been updated to use the backup register HAL to communicate reset reason to the system part application. This leads to undefined reset reasons in the following scenarios

  1. When completing OTA bootloader update
  2. When completing DFU update via button reset/hold

Solution

The backup register HAL is implemented on RTL platform via retained SRAM, the bootloader just needs to write the "registers"

Steps to Test

Checkout branch, compile bootloader, flash
Flash app via reset button hold and DFU
Log reset reason ie (Log.info("Last Reset Reason %d", System.resetReason());)
Verify that it is correct (70 = firmware update, 40 = pin/power reset):

0000008051 [app] INFO: Last Reset Reason 70

Without bootloader update, the values are undefined:

0000058978 [app] INFO: Last Reset Reason -49520187

Example App

References

See comment here


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@scott-brust scott-brust marked this pull request as ready for review May 22, 2024 22:25
@scott-brust scott-brust added this to the 5.8.2 milestone May 23, 2024
@eberseth
Copy link
Contributor

Just a general question on whether we should be providing a reason for disconnection in the vitals publish as well? I did a button reset and the disconnect reason was "none". I then powered off my msom and got a reason of "unknown".

{
  "connection": {
    "status": "connecting",
    "error": 0,
    "attempts": 1,
    "disconnects": 0,
    "disconnect_reason": "none",
    "interface": "Cellular"
  },
  "coap": {
    "transmit": 9,
    "retransmit": 0,
    "unack": 0,
    "round_trip": 1526
  },
  "publish": {
    "rate_limited": 0
  }
}

@monkbroc
Copy link
Member

Connection vitals are not very useful. The counters tend to be 0 or 1. I think it's out of scope to rework that to be something more useful right now, but worth considering perhaps as part of ACM work.

@eberseth
Copy link
Contributor

Connection vitals are not very useful. The counters tend to be 0 or 1. I think it's out of scope to rework that to be something more useful right now, but worth considering perhaps as part of ACM work.

I agree changing it may be out-of-scope of the reason fixes. Just thinking that since we already know that we have reset for OTA or other that it would be easy to determine and report.

@scott-brust
Copy link
Member Author

Connection vitals are not very useful. The counters tend to be 0 or 1. I think it's out of scope to rework that to be something more useful right now, but worth considering perhaps as part of ACM work.

I agree changing it may be out-of-scope of the reason fixes. Just thinking that since we already know that we have reset for OTA or other that it would be easy to determine and report.

The vitals mostly contain system diagnostics stuff. The "connection" structure you point out is mostly the "Network" diagnostics. There are similar "Cloud" diagnostics (ie connect attempts, disconnects, disconnect reason). I dont know how well the Network disconnect reason diagnostic works

These are distinct from the System.resetReason(), which is not stored as a system diagnostic. I dont know if this is included in the vitals publish or not. I believe it should be published as a separate event

@scott-brust
Copy link
Member Author

Pulling in changes from #2771

@scott-brust scott-brust requested a review from monkbroc May 24, 2024 15:48
scott-brust and others added 4 commits May 28, 2024 10:06
1. When completing OTA bootloader update
2. When completing DFU update via button hold
Minor wdog test for msom platform
@scott-brust scott-brust force-pushed the fix/misc-wdogtest-bootloader-resetreason branch from 9c7a184 to 3852e14 Compare May 28, 2024 17:06
@scott-brust scott-brust merged commit 983cc02 into develop May 28, 2024
13 checks passed
@scott-brust scott-brust deleted the fix/misc-wdogtest-bootloader-resetreason branch May 28, 2024 17:56
@scott-brust scott-brust mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants