Harden the PM1 sleep/power-off paths: safe failure behavior and begin-time IRQ normalization - #300
Conversation
_clearWakeupInterrupt() now reports whether clearing the wakeup cause was able to communicate with the device involved, and _releaseWakeupPin() passes that up to its callers. deepSleep() keeps waiting indefinitely while the wakeup source is merely still active (a finger resting on the touch panel, Issue m5stack#91), exactly as before. But when the clear communication itself fails, the pin can never be released, and sleeping with EXT1 ANY_LOW armed on a low line would wake immediately and reboot in a loop; the device now stays awake and logs an error instead. lightSleep() only stops waiting in that case: an immediate wakeup from light sleep simply returns to the caller, so going to sleep is fine.
…erOff The timer path armed EXT1 ANY_LOW before waiting for the PM1 IRQ line to be released, so a release that never happened still went to sleep on a low line and woke immediately, rebooting in a loop. The release is now confirmed first, and when the wake path cannot be secured the device stays awake with an error log instead of sleeping. The shutdown command of powerOff() is retried a few times. When it keeps failing the device now arms the IRQ pin before the fallback deep sleep, so a single press of the power button can bring it back. (A double click of the power button or replugging USB always power-cycles the ESP32 through the PM1, so recovery was possible either way; this keeps the single-press path working and makes the failure visible.)
The PM1 keeps running across ESP32 resets, so its register state survives from whatever firmware ran before. Clear the wake source and the IRQ status, mask the unused GPIO IRQs, and return GPIO0 (the touch INT input) to a plain input, the same treatment the PaperMono setup already applies.
There was a problem hiding this comment.
Pull request overview
This PR hardens PM1-based sleep/power-off behavior (notably ToughC5-class boards where wake sources are funneled through the PM1 IRQ output) by preventing unsafe sleep entry when the wake-clear path is broken, and by normalizing PM1 IRQ state at startup to avoid inheriting stale PM1 configuration across ESP resets.
Changes:
- Make wake-source clearing report communication failure and use that signal to avoid entering deep sleep in configurations that would reboot-loop.
- Reorder/strengthen PM1 power-off and EXT1 arming so the wake pin is only armed after it is confirmed released; add retry/fallback behavior.
- Normalize ToughC5 PM1 IRQ/wakeup-related state at begin (clear wake/IRQ status, mask unused GPIO IRQs, restore GPIO0 mode).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utility/Power_Class.hpp | Extends _releaseWakeupPin to optionally return wake-clear communication status to callers. |
| src/utility/Power_Class.cpp | Implements hardened PM1 power-off + EXT1 arming order; adds comm-failure-aware wake-pin release waiting for deep/light sleep; adds ToughC5 PM1 IRQ normalization at begin. |
| src/M5Unified.hpp | Changes _clearWakeupInterrupt() to return bool indicating whether required device communication succeeded. |
| src/M5Unified.cpp | Implements boolean result propagation for _clearWakeupInterrupt() for AW9523 and PM1 paths. |
Suppressed comments (2)
src/utility/Power_Class.cpp:1634
clear_comm_okis initialized totruebefore waiting for wakeup-pin release in lightSleep. With the new "comm ever succeeded" meaning, it should start asfalseso a total communication failure can be detected reliably.
bool clear_comm_ok = true;
while (!_releaseWakeupPin(wpin, &clear_comm_ok))
src/utility/Power_Class.cpp:1538
clear_comm_okis initialized totruebefore waiting for wakeup-pin release in both deepSleep and lightSleep. With the new "comm ever succeeded" contract, this should start asfalse; otherwise a dead clear-communication path can be masked (and with the sticky update fix, it would remain true forever).
bool clear_comm_ok = true;
while (!_releaseWakeupPin(wpin, &clear_comm_ok))
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| bool Power_Class::_releaseWakeupPin(std::uint_fast8_t wakeup_pin, bool* clear_comm_ok) | ||
| { | ||
| // clear_comm_ok は「割り込み要因のクリア通信が一度でも成功したか」を返す。 | ||
| // ピンが解放されない理由が「要因がまだ生きている (指が触れている等)」なのか | ||
| // 「デバイスと通信できない (回復見込みなし)」なのかを呼び出し元が区別できる。 | ||
| bool comm_ok = false; | ||
| for (int retry = 8; retry > 0; --retry) | ||
| { | ||
| if (m5gfx::gpio_in(wakeup_pin)) { return true; } | ||
| M5._clearWakeupInterrupt(); | ||
| if (m5gfx::gpio_in(wakeup_pin)) { if (clear_comm_ok) { *clear_comm_ok = true; } return true; } | ||
| comm_ok |= M5._clearWakeupInterrupt(); | ||
| m5gfx::delay(5); | ||
| } | ||
| if (clear_comm_ok) { *clear_comm_ok = comm_ok; } | ||
| return m5gfx::gpio_in(wakeup_pin); |
There was a problem hiding this comment.
Fair point on the threshold: one failed window (8 attempts, ~40ms) was a thin basis for cancelling the sleep. 37e3ffa now requires the failure to persist across three consecutive windows (~150ms with no successful communication at all) before treating the clear path as dead, and the count resets as soon as one attempt succeeds; lightSleep() gets the same treatment. The per-call scope of clear_comm_ok is now stated explicitly in the comment.
Two details of the original description for the record: a released pin reporting success is intentional (the loop exits in that case), and while the wakeup source is active with working communication, clear_comm_ok stays true — so the misclassification required a full window of consecutive communication failures. The thicker threshold removes the transient-busy false positive that remained.
A single failed window of clear attempts (8 tries within one _releaseWakeupPin call, about 40ms) was enough for deepSleep() to stay awake. A device that is only momentarily busy on the bus should not cancel the sleep: require the failure to persist across three consecutive windows (about 150ms with no successful communication at all) before treating the clear path as dead, and reset the count as soon as one attempt succeeds. Same treatment for lightSleep(). Also state explicitly that the clear_comm_ok result of _releaseWakeupPin() covers the attempts of that call.
Hardens the sleep and power-off paths of the PM1-based wake configuration (ToughC5 and friends), following up on a review of the ToughC5 support.
Problems
deepSleep()waits for that line to be released before arming EXT1 ANY_LOW. The wait could not tell "the wakeup source is still active (finger on the panel)" apart from "the clear communication itself is failing" — the former is a legitimate indefinite wait (Issue M5Paper wakes too soon from deep sleep when touch wakeup is enabled - with solution #91), the latter never resolves.powerOff()ignored the result of the PM1 shutdown command and proceeded to a wake-less deep sleep. The device stays recoverable through the PM1 itself (double-click of the power button, or replugging USB, power-cycles the ESP32), but a single press could no longer wake it and nothing was logged.Changes (one commit each)
_clearWakeupInterrupt()reports whether clearing could communicate;deepSleep()still waits indefinitely for an active source exactly as before, but stays awake (with an error log) when the communication fails.lightSleep()just stops waiting in that case, since an immediate wakeup from light sleep is harmless.Notes on shared paths
_clearWakeupInterrupt()changed fromvoidtobool; the CoreS3 (AW9523) branch now returns the read result. No behavioral change for any board when communication works.deepSleep()is preserved unchanged.Testing