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

improve hard fault message #8297

Merged
merged 2 commits into from
Aug 20, 2023
Merged

Conversation

dhalbert
Copy link
Collaborator

I reworked the safe mode messages in #7577. I rewrote
"Crash into the HardFault_Handler." to be
"Fault detected by hardware.".

This turns out to be confusing, because it sounds like the hardware is failing. This PR changes it to
"Hard fault: memory access or instruction error."
which is the definition of hard fault on most CPU's.

I made this change in 8.2.x to get it into the next 8.2.x release, and it can be merged up into main.

jepler
jepler previously approved these changes Aug 19, 2023
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Writing error messages is tough. Thanks for trying to make this one better!

@dhalbert dhalbert force-pushed the better-hard-fault-message branch from d0cb42c to 10a022d Compare August 19, 2023 16:40
@dhalbert
Copy link
Collaborator Author

I consolidated some error messages to get the failed builds not to overflow. Interestingly, some changes increased the build size, such as removing all the periods and some thes from the safe-mode messages.

@dhalbert dhalbert requested a review from jepler August 19, 2023 17:27
@bill88t
Copy link

bill88t commented Aug 20, 2023

Are you sure the Hard fault: is needed?
I am not sure it adds anything of value.

A cpu / stack crash is a hardfault, but that doesn't matter for the user.

@DJDevon3
Copy link

DJDevon3 commented Aug 20, 2023

I imagine hard fault is a reference to the exception handler that caught it. I think that's a good thing to include. Might even be better to include the file & or memory address that failed? Copy/pasting the same "my board says it crashed into the hard fault handler" doesn't really give the devs much info to go on.

Dan is right, with ""Fault detected by hardware."" can't tell you how many times in CP help channel a beginner gets that message and then immediately asks "Did I brick my board?" It's a very scary message to a beginner.

@bill88t
Copy link

bill88t commented Aug 20, 2023

Might even be better to include the file & or memory address that failed?

If it was possible to include this much info, it should probably be displayed in a seperate line / section.

But I am pretty sure CP has no idea about source files even when compiled with DEBUG=1.

It may be possible to capture the backtrace (addresses) though.
Not that it would be useful on a non-debug build.

Copy/pasting the same "my board says it crashed into the hard fault handler" doesn't really give the devs much info to go on.

Agreed. Then maybe we should add a link to a debugging tutorial / hardfault crash course?
Something like:
"You just experienced a hardfault, this is a list of common faults and causes, check for related issues, file an issue, maybe debug it like this, decode the backtrace like this, gdb+openocd like this"

The guides would be port specific due to how different the procedure is.
I would gladly help with the esp and rp2 debug guides.

@dhalbert
Copy link
Collaborator Author

This is a safemode reason, and they are listed in the safemode guide (and also in the doc): https://learn.adafruit.com/circuitpython-safe-mode. There is a message asking to file an issue that is printed at the same time as this.

@dhalbert
Copy link
Collaborator Author

Are you sure the Hard fault: is needed?

"Hard fault" is definitional, at least for ARM chips, so I am leaving it in because it describes what has happened in the hardware.

Copy link
Member

@gamblor21 gamblor21 left a comment

Choose a reason for hiding this comment

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

Shortened error messages are clear to me as is the hard fault one previously approved.

@dhalbert dhalbert merged commit 4593008 into adafruit:8.2.x Aug 20, 2023
@dhalbert dhalbert deleted the better-hard-fault-message branch August 20, 2023 16:16
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.

5 participants