Fix crash when all page switches are disabled#191
Open
librick wants to merge 1 commit into
Open
Conversation
Author
|
Still seeing crashes after further tests. Closing this for now so I can debug further |
If you turn off all display page switches in Home Assistant, the AirGradient ONE crashes (reproduced on v5.3.3, likely affects v5.3.4). I traced this to the rotation logic in display_sh1106_multi_page.yaml and display_sh1106_single_page.yaml. When every page switch is off, including display_blank_page, the on_page_change handlers each call display.page.show_next on their own page. This triggers the next page's on_page_change handler, which calls show_next again, and so on. Because there's no base case or enabled page to stop the loop, the call stack grows without bound, triggering a stack overflow. This happens within microseconds and crashes the device. The configuration.md doc says: >disabling all pages will result in the device rebooting but does not explain why. This proposed fix gates show_next calls behind a template binary_sensor (any_non_blank_page_enabled), which is true if and only if any non-blank page switch is on. If the blank page switch is the only switch that's on, the blank page is shown. If the blank page switch is off and every other page switch is also off, the blank page is shown. Applied to display_sh1106_multi_page.yaml and display_sh1106_single_page.yaml. display_ssd1306.yaml is not affected because its on_page_change only has a handler for the boot page, which bounds the chain of possible page skips. configuration.md is updated to reflect that disabling all pages no longer crashes the device, and that setting the blank page switch on its own is sufficient to display a blank page.
librick
force-pushed
the
fix-rotation-infinite-recursion
branch
from
709f291 to
bb30742
Compare
Author
|
Reopening. My first attempt had some bugs, turns out this is tricky to get right. Reproduced the existing behavior on v5.3.3, deployed my update, and verified it works as intended. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If you turn off all display page switches in Home Assistant, the AirGradient ONE crashes (reproduced on v5.3.3, likely affects v5.3.4). I traced this to the rotation logic in
display_sh1106_multi_page.yamlanddisplay_sh1106_single_page.yaml.When every page switch is off, including
display_blank_page, theon_page_changehandlers each calldisplay.page.show_nexton their own page. This triggers the next page'son_page_changehandler, which callsshow_nextagain, and so on. Because there's no base case or enabled page to stop the loop, the call stack grows without bound, triggering a stack overflow. This happens within microseconds and crashes the device.The
configuration.mddoc says:but does not explain why.
This proposed fix gates
show_nextcalls behind a templatebinary_sensor(any_non_blank_page_enabled), which is true if and only if any non-blank page switch is on.If the blank page switch is the only switch that's on, the blank page is shown. If the blank page switch is off and every other page switch is also off, the blank page is shown.
Applied to
display_sh1106_multi_page.yamlanddisplay_sh1106_single_page.yaml.display_ssd1306.yamlis not affected because itson_page_changeonly has a handler for the boot page, which bounds the chain of possible page skips.configuration.mdis updated to reflect that disabling all pages no longer crashes the device, and that setting the blank page switch on its own is sufficient to display a blank page.Stack Trace
Here's the stack trace from a boot after hitting the crash condition: