Add deep-sleep functionality with button wake support - #109
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Summary by CodeRabbit
WalkthroughThe ESP32 now exposes a disabled-by-default Home Assistant “Power off” button. The command enables GPIO holds and enters deep sleep. DeskService releases holds during startup, and ButtonHandler updates deep-sleep wake-up selection. ChangesPower-off deep-sleep control
Sequence Diagram(s)sequenceDiagram
participant HomeAssistantHandler
participant CommandTopic
participant DeskService
participant ESP32
HomeAssistantHandler->>CommandTopic: Publish {"action":"power"}
CommandTopic->>DeskService: Deliver power action
DeskService->>ESP32: Enable GPIO holds and enter deep sleep
Priority: ⬇️ Low Change: Feature Merge Risk: 🟡 Moderate · up to After waking from deep sleep, the device may report an output state that does not match the physical output. This should be resolved before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit taps the power command, Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 97b8a528-74cf-4246-9bf0-0122a001d3b8
📒 Files selected for processing (4)
README.mdsrc/esp/ButtonHandler.cppsrc/esp/DeskService.cppsrc/esp/HomeAssistantHandler.cpp
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🔇 Additional comments (3)
src/esp/HomeAssistantHandler.cpp (1)
283-294: LGTM!src/esp/DeskService.cpp (1)
367-372: LGTM!README.md (1)
255-255: LGTM!
The AVR holds the buttons in HIGH state, so there's no need for pull resistors during deep sleep, except for the logic level shifter (`OE`).
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 766436a6-3240-42a9-b272-f34e70a2c0be
📒 Files selected for processing (2)
src/esp/ButtonHandler.cppsrc/esp/DeskService.cpp
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🔇 Additional comments (1)
src/esp/ButtonHandler.cpp (1)
16-23: LGTM!
|
@coderabbitai review |
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Release the GPIO holds before restoring PIN_OE. · DeskService.cpp:40
src/esp/DeskService.cpp:40
🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRelease the GPIO holds before restoring
PIN_OE.After deep-sleep wake,
gpio_hold_en()can still lockPIN_OE. On supported targets,gpio_deep_sleep_hold_en()can keep that hold active. Therefore, thedigitalWrite()at Line 40 can be ignored whileenableis updated from NVS. This can leave software state different from the physical OE level.Disable both holds before the write, using the same guard and sequence as
setOutputEnable().Proposed fix
if (nvs_get_u8(handle, "oe", &_enable) == ESP_OK) { enable = static_cast<bool>(_enable); +#if SOC_GPIO_SUPPORT_HOLD_IO_IN_DSLP && !SOC_GPIO_SUPPORT_HOLD_SINGLE_IO_IN_DSLP + gpio_deep_sleep_hold_dis(); +#endif + gpio_hold_dis(static_cast<gpio_num_t>(PIN_OE)); digitalWrite(PIN_OE, enable ? HIGH : LOW); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: e417c66c-9b80-47b8-815f-2362a24262af
📒 Files selected for processing (1)
src/esp/DeskService.cpp
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🔇 Additional comments (1)
src/esp/DeskService.cpp (1)
371-376: LGTM!Also applies to: 437-445
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 7324734f-e21a-4be9-a39f-637f99dfcdc0
📒 Files selected for processing (1)
src/esp/DeskService.cpp
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
|
🤖 Completed: Generate docstrings for PR #109 — View PR #113 |
Document button wake sources and device power actions
Implement deep-sleep mode that allows waking the device using buttons. This includes enhancements to the button handler and integration with the Home Assistant for power control. The DeskService now supports a power action to initiate deep sleep.