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

resolve crashes when running without display #4435

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

TychoVrahe
Copy link
Contributor

This PR resolve crashes when uninitialized display returns null ptr as framebuffer and firmware attempts to render into it.

We will need to run some stages of prodtest of T3W1 running without display and this enables it, but we should probably consider allowing this explicitly via some OTP flag, and implement somewhat more elegant way to shutdown when this is not allowed. however, I consider this out of scope in this PR.

@TychoVrahe TychoVrahe self-assigned this Dec 10, 2024
Copy link

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@TychoVrahe TychoVrahe marked this pull request as ready for review December 10, 2024 14:03
@TychoVrahe TychoVrahe requested a review from prusnak as a code owner December 10, 2024 14:03
@TychoVrahe TychoVrahe requested review from cepetr and removed request for prusnak December 10, 2024 14:03
@cepetr
Copy link
Contributor

cepetr commented Dec 11, 2024

Am I correct in understanding that T3W1 crashes in the bootloader?

The proposed solution is simple, but I suggest a slightly different approach. It's a bit complicated:

Detecting the Display

  • If the display is not detected or present, the display_init() function could return false. Alternatively, we could introduce a dedicated display_detect() function to handle this specific purpose.

Handling Uninitialized Displays

  • If the display is not initialized, we should avoid drawing on it. We might ensure this in bootui.c, which serves as an intermediate layer between the bootloader and the Rust UI.

This approach ensures proper handling of cases where the display is unavailable while maintaining a clear separation of responsibilities in the code. I believe we should avoid calling any screen_xxx functions if the display or touch interface is not initialized. Additionally, if display_get_frame_buffer() returns NULL while painting on screen, it should be treated as a fatal error.

@TychoVrahe
Copy link
Contributor Author

As discussed in person, lets consider this out of scope here. I created an issue for handling this later: #4440

@TychoVrahe TychoVrahe merged commit 0d3407b into main Dec 11, 2024
94 checks passed
@TychoVrahe TychoVrahe deleted the tychovrahe/display/no_disp_run branch December 11, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

2 participants