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

Expose hard_fault panicking behavior #737

Merged
merged 2 commits into from
Mar 10, 2023
Merged

Expose hard_fault panicking behavior #737

merged 2 commits into from
Mar 10, 2023

Conversation

ia0
Copy link
Contributor

@ia0 ia0 commented Mar 8, 2023

This PR factors the hard-faulting logic out of the panic handler and makes it publicly available. The reasoning is that users may want to use that hard-faulting logic as the defmt::panic_handler to avoid double panic printing.

Alternative options:

  1. Factor the hard-faulting logic out but don't expose it publicly. Instead, expose a feature that would define the hard-faulting logic as defmt::panic_handler.
  2. Same as above, but reuse the print-defmt feature, essentially always setting the defmt::panic_handler.

Alternative (2) is probably the easiest for the user. The downside would be if for some reason the defmt::panic_handler should be set to something else instead. I don't see any examples, but that's a risk.

Alternative (1) is still easy (and that feature could be a default feature, or alternatively there could be a disabling feature) while keeping flexibility in case a user needs to define a custom defmt::panic_handler.

The current PR is the obvious non-breaking and flexible solution, but requires the user to define the defmt::panic_handler when needed.

@Urhengulas
Copy link
Member

Thank you!

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 10, 2023

Build succeeded:

  • ci

@bors bors bot merged commit ce767b4 into knurling-rs:main Mar 10, 2023
@ia0 ia0 deleted the probe branch March 21, 2023 09:22
@ia0
Copy link
Contributor Author

ia0 commented Mar 21, 2023

Thanks @Urhengulas ! Do you know when the next release of panic-probe would be expected? (I'm wondering if I should override my dependencies to defmt, defmt-rtt, and panic-probe with [patch.crates-io] or if I should wait for a release.)

@Urhengulas
Copy link
Member

Thanks @Urhengulas ! Do you know when the next release of panic-probe would be expected? (I'm wondering if I should override my dependencies to defmt, defmt-rtt, and panic-probe with [patch.crates-io] or if I should wait for a release.)

We will make a release soon!

@ia0
Copy link
Contributor Author

ia0 commented Mar 21, 2023

Thanks!

Then I'll wait and update google/wasefire#25 to use the new version instead of patching the dependencies.

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