Skip to content

Conversation

@reitermarkus
Copy link
Contributor

@reitermarkus reitermarkus commented Apr 28, 2025

In release mode, the panic_persist::report_panic_info was never reached because the panic handler would block when writing to the RTT target (since none is connected).

Only set BlockIfFull in debug mode, and replace unsafe UpChannel::conjure with with_terminal_channel while we're at it.

@jordens
Copy link
Member

jordens commented Apr 28, 2025

Why would this only be triggered in release?

IIRC the unsafe conjure is there to avoid deadlock/borrow failure. with_terminal_channel looks like it would panic when nested.

@jordens
Copy link
Member

jordens commented Apr 28, 2025

And a hang on panic doesn't sound too bad here. IIRC the watchdog should kick in.
I can see the peristent panic stuff being moved earlier though.

@reitermarkus
Copy link
Contributor Author

Why would this only be triggered in release?

Right, it isn't. It's triggered when no debug probe is connected. However there is already an assumption in the panic_handler that when debug_assertions are disabled, the device should reset rather than loop.

And a hang on panic doesn't sound too bad here. IIRC the watchdog should kick in.

The watchdog does indeed kick in, but I'd like the device to immediately reset without delay rather than wait for 4 seconds for the watchdog.

Maybe adding an rtt-target feature that can be disabled is the better option?

@jordens
Copy link
Member

jordens commented Apr 29, 2025

Ack the rtt feature.

  • Ensure that with rtt we don't enable the watchdog. It makes debugging difficult
  • Early panic persist
  • Blocking panic dump over rtt with unsafe conjure (or argue that there is no deadlock).

The reboot on panic never actually solves any problem or bug. I'm fine with keeping it as is. It seems slightly better than the alternative. But I won't spend time on "optimizing the panic experience". Rather fix the cause of the panic.

@reitermarkus reitermarkus force-pushed the rtt-target-panic-handler branch from 3f233c1 to e9ec34b Compare April 29, 2025 09:19
@reitermarkus reitermarkus force-pushed the rtt-target-panic-handler branch from e9ec34b to 8c6c052 Compare April 29, 2025 09:20

#[cfg(debug_assertions)]
loop {}
cortex_m::asm::bkpt();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this, combined with watchdog.stop_on_debug(...), enough, or do you additionally want to completely disable the watchdog when the rtt feature is enabled?

Copy link
Member

Choose a reason for hiding this comment

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

Ah. Just watchdog.stop_on_debug is fine. Good find.

@reitermarkus
Copy link
Contributor Author

@jordens, anything still missing here?

@reitermarkus reitermarkus requested a review from jordens August 6, 2025 10:22
@jordens jordens added this pull request to the merge queue Aug 6, 2025
@jordens
Copy link
Member

jordens commented Aug 6, 2025

The review request was.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Aug 6, 2025
@jordens jordens added this pull request to the merge queue Aug 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2025
@reitermarkus reitermarkus requested a review from jordens August 13, 2025 10:21
Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

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

Linker script needs fixing AFAICT

@reitermarkus
Copy link
Contributor Author

Seems like rust-lld got more strict about whitespace. Should be fixed now.

@reitermarkus reitermarkus requested a review from jordens August 18, 2025 10:27
@jordens jordens enabled auto-merge August 18, 2025 11:10
@jordens jordens added this pull request to the merge queue Aug 18, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 18, 2025
@jordens
Copy link
Member

jordens commented Aug 18, 2025

+ probe-rs download --chip STM32F407ZGTx --log-file /dev/null --probe 0483:3754:003C002F5632500A20313236 target/thumbv7em-none-eabihf/release/booster
     Finished in 49.05s
+ probe-rs reset --chip STM32F407ZGTx --log-file /dev/null --probe 0483:3754:003C002F5632500A20313236 --connect-under-reset
 WARN probe_rs::architecture::arm::core::armv7m: Core is running, but we expected it to be halted
+ sleep 30
+ ping -c 5 -w 20 booster-hitl
+ python3 -m miniconf --broker mqtt.ber.quartiq.de --discover dt/sinara/booster/+ /telemetry_period=5
+ python3 hitl/basic.py --broker mqtt.ber.quartiq.de --prefix dt/sinara/booster/+
Traceback (most recent call last):
  File "/home/runner/actions-runner/_work/hitl/hitl/hitl/basic.py", line 45, in channel_on
    await booster.miniconf.set(f'/channel/{channel}/state', initial_state)
  File "/home/runner/actions-runner/_work/hitl/hitl/vpy/lib/python3.9/site-packages/miniconf/miniconf.py", line 144, in set
    ret = await self._do(
  File "/home/runner/actions-runner/_work/hitl/hitl/vpy/lib/python3.9/site-packages/miniconf/miniconf.py", line 134, in _do
    return await fut
miniconf.miniconf.MiniconfException: ('Error', 'Variant absent (depth: 2)')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/actions-runner/_work/hitl/hitl/hitl/basic.py", line 162, in <module>
    main()
  File "/home/runner/actions-runner/_work/hitl/hitl/hitl/basic.py", line 151, in main
    sys.exit(loop.run_until_complete(test()))
  File "/usr/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
    return future.result()
  File "/home/runner/actions-runner/_work/hitl/hitl/hitl/basic.py", line 147, in test
    await test_channel(booster, channel, tele.messages)
  File "/home/runner/actions-runner/_work/hitl/hitl/hitl/basic.py", line 63, in test_channel
    async with channel_on(booster, channel, 'Powered'):
  File "/usr/lib/python3.9/contextlib.py", line 175, in __aenter__
    return await self.gen.__anext__()
  File "/home/runner/actions-runner/_work/hitl/hitl/hitl/basic.py", line 49, in channel_on
    await booster.miniconf.set(f'/channel/{channel}/state', 'Off')
miniconf.miniconf.MiniconfException: ('Error', 'Variant absent (depth: 2)')
+ finish
+ mosquitto_pub -h mqtt.ber.quartiq.de -t cmnd/tasmota_4F64D0/POWER -m OFF
Error: Process completed with exit code 1.

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.

2 participants