Repository navigation
Bring bounded display recovery and safe rendering to the standalone example - #31
Conversation
|
@coderabbitai review |
JGAguado
left a comment
There was a problem hiding this comment.
Thanks for bringing safe rendering and BUSY handling to the standalone example. Two requests before merging:
- One recovery implementation, not two. This PR adds a shared
display_recovery_budget, which fixes the unbounded retry loop when BUSY keeps toggling. The multi-device core (smart_plant_core.yaml) still has the older self-re-executingrecover_display_busywithout that budget. Please port the same recovery to the core, so both files behave the same and fixes don't drift apart. - Simpler docs. The new note in
programming.rstis aimed at newcomers. Details such as "30.510 seconds … event-loop scheduling latency" and the shutdown-hook discussion belong in a code comment. Something like "invalid readings show --, and the device waits for the e-paper to finish refreshing, with a bounded recovery if it doesn't" is enough there.
Minor: on Windows with MinGW, test_standalone_display errors because -fsanitize=undefined needs libubsan, and tearDownClass fails while removing the temp dir. Skipping the sanitizer when it isn't available would make the tests portable.
|
Addressed all three findings from your review:
The merged PR #30 is included. Both normal and fault paths keep its configurable low-battery sleep; a runtime regression test covers that merge. All 39 tests and strict Sphinx HTML pass. Validation uses pinned ESPHome 2026.9.0, synthetic inputs and private local builds. No device, NAS, broker or production build service was touched. The native Windows path has not been run on Windows; the no-UBSan fallback and real compilation-failure case were exercised locally. Please re-review the corrected head. The previous changes-requested review remains yours to clear. I do not treat the fixes or local validation as your approval. Final combined-source firmware builds all pass (ESPHome 2026.9.0):
Exact head: |
JGAguado
left a comment
There was a problem hiding this comment.
All three requests addressed: the core and standalone now share identical refresh, settle, recovery-budget and fault scripts (the only difference left is the core's low-battery sleep duration), the newcomer note is short, and the tests no longer depend on UBSan. Approving.
|
All five open PRs are approved. They conflict pairwise (mostly
CI must stay green on each rebased head. |
…very Drop the globals duplicated by the #31 merge. Let the parity test accept the multi-device extensions (fault hook, counter, logs, MQTT status) and the per-profile sleep_after_display. Bound the flow tests by each profile's on_display_fault delay. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017fn1KdJJTY7LedVqZ7SxcA
Behavior
The standalone example has older display guards and boot ordering. The shared core also retries BUSY recovery without a total time budget, so an oscillating BUSY signal can keep the device awake.
Both entry points now use the same refresh cancellation, settling, recovery budget and reset actions. Missing readings show
--. Numeric labels keep measured values while needles and icons stay in range. One timer bounds all recovery retries, including the final sleep delay. A new refresh cancels pending recovery and releases an interrupted reset pulse.This revision addresses the maintainer review:
smart_plant_core.yaml, with regression tests for both transport profiles and standalone.The branch includes upstream PR #30. Normal and emergency sleep preserve its configurable low-battery duration and explicit OTA confirmation.
Verification
python -B -m unittest discover -s tests -v: 39 tests pass, no skips. This covers actual C++ lambdas and YAML automation, both scheduler orders, permanent/oscillating/late BUSY, cancellation during reset, fresh budgets and low-battery sleep duration.git -c core.whitespace=cr-at-eol diff --checkpass.The native Windows path has not been executed on Windows. Replacement fonts prove compilation, not exact artwork or glyphs. The recovery bound ends at the deep-sleep action; stock shutdown hooks can still send the display sleep command. No hardware test or firmware deployment was performed.
Rollback: revert this PR through a separate PR. A source revert does not deploy firmware. No fleet inventory, private identity, HA migration or deployment tooling is included.
AI assistance
Codex prepared the source and tests. The author checked both firmware entry points, the exact upstream merge, native test behavior, synthetic compiler fixtures, semantic diff and line endings.
Review
The prior maintainer request for changes needs re-review. The original CodeRabbit request received no status or completed review; no automated approval is claimed. Required CI and repository review rules apply.