Improvements to deepsleep UM #4456
Conversation
|
thank you for contributing! |
|
Thank you for the feedback! I didn’t notice an existing deep-sleep module in the repository when I started working on this. My goal was to create a module specifically focused on managing sleep when the battery voltage is low, while adding multiple wake-up methods, including touch wake-up and preset wake-up. Additionally, I included configurable IO pin settings to achieve optimal power efficiency. I’ll look into integrating these features with the existing deep-sleep module to ensure better compatibility and functionality for users. @DedeHai |
|
the PR for the deep-sleep UM was pending for quite some time, I wanted to run some more long-term tests but finally I got it merged just about after you opened this PR. |
|
The deep-sleep UM primarily uses a wakeup timer as its trigger source from deep-sleep status. Shouldn't this functionality be distinct from the battery UM? Regarding pullup/pulldown configuration: it works well for some pins like 12, 13, 14, 15, and 27, but not reliably for others like 25 and 26. Is there any way we can override or influence this "feature" in the IDF? |
how did you figure that?
deep sleep has nothing to do with battery UM, so dont know what you mean.
did you test to disable pullup/down successfully? |
|
if (presetWake) {
Unlike the battery usermod, the deep-sleep usermod supports preset delays, highlighting their distinct functionalities."
I found that hold mode(rtc_gpio_hold_en) performs reliably on IO25, but it doesn't seem to work consistently on IO26. Do you have any insights into why this happens? |
I thought you were referring to the existing deep-sleep UM, its a bit confusing both are called that :)
I dont know what you mean by that.
I do not unfortunately. you need to dig though the IDF code and the espressif forum as well as the datasheet to see if there is a hardware bug, IDF bug or if that is a feature. So to sum up:
|
|
Add only the touch option and preset wakeup timer |
|
Are you referring to this line:
I have successfully run this code on the ESP32. However, I cannot test it on the ESP32-C3 as I don’t have this board. |
Thanks for your feedback. I forgot that this function is used in another part of my repository's main branch for the user info UI, and it is useful for other users. I will add this to the WLED repository now.
I found this code for esp32 c3 |
if you do: add a #define users wanting debug info can use to add it. the wake-up cause is of no use to normal users. |
|
done |
|
What does draft status mean in this repository? |
|
draft means not finished and not ready to merge |
|
what sleps are needed to complete the pull request? |
|
fix the ones I marked |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
usermods/deep_sleep/usermod_deep_sleep.h (1)
29-45:⚠️ Potential issueSentinel value of –1 wraps to 255 in
uint8_t→ logic & touch wake‑up always enabled
touchPinis declareduint8_t, so assigning-1via the default macro yields255.
Later the code checksif (enableTouchWakeup && touchPin)which treats any non‑zero value as valid, unintentionally enabling touch wake‑up on boot.-#define DEEPSLEEP_WAKEUP_TOUCH_PIN -1 +// 0xFF (255) is out‑of‑range for any real GPIO and survives the +// uint8_t assignment without wrapping. +#define DEEPSLEEP_WAKEUP_TOUCH_PIN 0xFF ... - uint8_t touchPin = DEEPSLEEP_WAKEUP_TOUCH_PIN; + uint8_t touchPin = DEEPSLEEP_WAKEUP_TOUCH_PIN; // 0xFF means “unused”And later:
-if (enableTouchWakeup && touchPin) { +if (enableTouchWakeup && touchPin != 0xFF) {
🧹 Nitpick comments (4)
wled00/pin_manager.h (1)
69-71: Add trailing comma for enum consistencyEvery other enumerator in this list ends with a comma. Keeping the style consistent avoids noisy diffs when additional IDs get appended later.
- UM_DEEP_SLEEP = USERMOD_ID_DEEP_SLEEP // 0x37 // Usermod "usermod_deep_sleep.h" -- Needs pins to control pmos, nmos, other devices + UM_DEEP_SLEEP = USERMOD_ID_DEEP_SLEEP, // 0x37 // Usermod "usermod_deep_sleep.h" -- Needs pins to control pmos, nmos, other devicesusermods/deep_sleep/readme.md (2)
4-4: Insert a comma for grammatical clarityMissing comma after “reboots” makes the sentence hard to parse.
-... deep sleep it reboots so ***make sure ... +... deep sleep it reboots, so ***make sure ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~4-~4: A comma might be missing here.
Context: ...st preset time. Once it wakes from deep sleep it reboots so ***make sure to use a boo...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~4-~4: Possible missing comma found.
Context: ... time. Once it wakes from deep sleep it reboots so ***make sure to use a boot-up preset...(AI_HYDRA_LEO_MISSING_COMMA)
64-64: Bullet style violates markdown‑lint rule MD004The list currently mixes
*and-. Using a single style keeps the doc lint‑clean.-* `DEEPSLEEP_WAKEUP_TOUCH_PIN` - specify GPIO pin used for touch-based wakeup +- `DEEPSLEEP_WAKEUP_TOUCH_PIN` - specify GPIO pin used for touch‑based wakeup🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
64-64: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
usermods/deep_sleep/usermod_deep_sleep.h (1)
118-158:findNextTimerInterval()uses INT_MAX without#include <limits.h>Some toolchains don’t pull in
INT_MAXtransitively. Add the header to
avoid portability warnings/errors.#include <limits.h>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
usermods/deep_sleep/readme.md(2 hunks)usermods/deep_sleep/usermod_deep_sleep.h(10 hunks)wled00/pin_manager.h(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
wled00/pin_manager.h (2)
usermods/pixels_dice_tray/pixels_dice_tray.h (1)
USERMOD_ID_PIXELS_DICE_TRAY(530-530)usermods/deep_sleep/usermod_deep_sleep.h (1)
USERMOD_ID_DEEP_SLEEP(358-360)
🪛 LanguageTool
usermods/deep_sleep/readme.md
[uncategorized] ~4-~4: A comma might be missing here.
Context: ...st preset time. Once it wakes from deep sleep it reboots so ***make sure to use a boo...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~4-~4: Possible missing comma found.
Context: ... time. Once it wakes from deep sleep it reboots so ***make sure to use a boot-up preset...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
usermods/deep_sleep/readme.md
64-64: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
usermods/deep_sleep/readme.md (1)
59-64:⚠️ Potential issueDefault value description contradicts implementation
The text states that
DEEPSLEEP_DISABLEPULL“(default is ebnabled)” (typo + logic).
Indeep_sleep.cppthe macro defaults to1, which disables the pull‑resistor.
Please align the documentation with the actual behaviour and fix the typo.-* `DEEPSLEEP_DISABLEPULL` - if defined, internal pullup/pulldown is disabled in deep sleep (default is ebnabled) +* `DEEPSLEEP_DISABLEPULL` - if defined, internal pull‑resistor is disabled in deep sleep (default: disabled)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
59-59: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
60-60: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
61-61: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
62-62: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
63-63: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
64-64: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
🧹 Nitpick comments (3)
usermods/deep_sleep/readme.md (2)
3-5: Grammar & readability – add missing article and comma"low power capabilities of th ESP" is missing an “e” in “the”, and the sentence on line 4 would read more smoothly with a comma after “deep sleep”.
-This usermod unleashes the low power capabilities of th ESP: when you power off ... -During deep sleep the ESP is shut down completely: ... +This usermod unleashes the low‑power capabilities of the ESP: when you power off ... +During deep sleep, the ESP is shut down completely: ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...eep usermod This usermod unleashes the low power capabilities of th ESP: when you power ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~4-~4: Possible missing comma found.
Context: ... time. Once it wakes from deep sleep it reboots so ***make sure to use a boot-up preset...(AI_HYDRA_LEO_MISSING_COMMA)
64-70: Markdown style inconsistencyLine 64 switches from “‑” to “*” list markers, triggering MD004.
Stick to one bullet style throughout the file for consistency.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
64-64: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
usermods/deep_sleep/deep_sleep.cpp (1)
74-104: Minor: spelling in helperphase_wakeup_reason()Variable
resonshould bereasonfor clarity.- static char reson[20]; + static char reason[20]; ... - strcpy(reson, "RTC_IO"); + strcpy(reason, "RTC_IO"); ... - return reson; + return reason;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
usermods/deep_sleep/deep_sleep.cpp(10 hunks)usermods/deep_sleep/readme.md(2 hunks)wled00/pin_manager.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/pin_manager.h
🧰 Additional context used
🪛 LanguageTool
usermods/deep_sleep/readme.md
[uncategorized] ~4-~4: Possible missing comma found.
Context: ... time. Once it wakes from deep sleep it reboots so ***make sure to use a boot-up preset...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
usermods/deep_sleep/readme.md
64-64: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
|
Hey! This pull request has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs. |
github-actions
bot
added
the
stale
|
Cpmpletely forgot about this... need to catch up |
netmindz
added
keep
|
Thanks for the review! I’ve fixed the formatting issues. |
|
thanks. can you summarize on what ESP boards, what GPIOs and what sleep intervals you tested this? |
|
Here are the test details: Boards tested: ESP32 DevKitC (WROOM-32) ESP32-S3 GPIOs tested for wakeup: GPIO15 (EXT1) GPIO0 (RTC IO button wake) Sleep intervals tested: 10 seconds 60 seconds 10 minutes 12 hours |
DedeHai
changed the title
- fix powerup behaviour: now works as intended i.e. if no sleep delay set, it enters deepsleep on powerup but only then - also wake if no macro is defined - apply macro after wake up even if the window is missed - respect date-range setting of macro - removed non-existing pin from touch list - removed non existing voltageCheck setting - fixed incorrect threshold setting - code cleanup
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
usermods/deep_sleep/deep_sleep.cpp (2)
298-311: Unresolved: Superfluous out-of-bounds array access.Line 300 is a no-op statement that indexes one element past the array end. This was flagged in a previous review.
#ifndef CONFIG_IDF_TARGET_ESP32C3 // dropdown for touch wakeupPin - touch_sensor_channel_io_map[SOC_TOUCH_SENSOR_NUM]; oappend(SET_F("dd=addDropdown('DeepSleep','touchPin');"));
221-227: Unresolved:touchThresholdis undefined and guard condition is incorrect.This was flagged in a previous review but remains unfixed:
touchThresholdis not declared—this code will not compile.touchPincheck should use>= 0(requires changing type toint8_tas noted above).Apply this diff:
- if (enableTouchWakeup && touchPin) { + if (enableTouchWakeup && touchPin >= 0) { + constexpr uint32_t touchThreshold = 40; // TODO: make configurable #ifdef SOC_TOUCH_VERSION_2 // S2 and S3 use much higher thresholds touchSleepWakeUpEnable(touchPin, touchThreshold << 4); // ESP32 S2 & S3: lower threshold = more sensitive #else touchSleepWakeUpEnable(touchPin, touchThreshold); // ESP32: higher threshold = more sensitive #endif }
🧹 Nitpick comments (1)
usermods/deep_sleep/deep_sleep.cpp (1)
269-274: Missing default values for new config fields.
enableTouchWakeupandpresetWakedon't specify default values ingetJsonValue(), soconfigCompletewill be false if these keys are missing from config (e.g., on first upgrade).#ifndef CONFIG_IDF_TARGET_ESP32C3 - configComplete &= getJsonValue(top["enableTouchWakeup"], enableTouchWakeup); + configComplete &= getJsonValue(top["enableTouchWakeup"], enableTouchWakeup, false); configComplete &= getJsonValue(top["touchPin"], touchPin, DEEPSLEEP_WAKEUP_TOUCH_PIN); #endif - configComplete &= getJsonValue(top["presetWake"], presetWake); + configComplete &= getJsonValue(top["presetWake"], presetWake, true);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
usermods/deep_sleep/deep_sleep.cpp(7 hunks)wled00/fcn_declare.h(1 hunks)wled00/pin_manager.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/pin_manager.h
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/fcn_declare.h
🧠 Learnings (9)
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T02:27:39.220Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-18T02:59:33.666Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T01:34:34.358Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
🧬 Code graph analysis (1)
wled00/fcn_declare.h (1)
wled00/ntp.cpp (2)
isTodayInDateRange(348-372)isTodayInDateRange(348-348)
🔇 Additional comments (8)
wled00/fcn_declare.h (1)
203-203: LGTM!The function declaration is correctly placed in the ntp.cpp section and matches the implementation signature shown in the relevant code snippets.
usermods/deep_sleep/deep_sleep.cpp (7)
3-5: LGTM!Correct use of
CONFIG_IDF_TARGET_ESP32C3to exclude touch sensor support on ESP32-C3, which lacks touch peripherals.
26-31: LGTM!RTC-persistent variables are correctly declared for deep sleep state tracking. The default touch pin of -1 clearly indicates "disabled".
73-120: LGTM on the timer calculation logic.The weekday handling and multi-day time difference calculations are correct. The function properly uses
isTodayInDateRangefor date filtering and iterates through all timer slots to find the minimum interval.
130-136: LGTM!Debug output is properly guarded with
#ifdef WLED_DEBUG, addressing previous reviewer feedback. The wakeup cause check correctly resetswakeupPresetfor non-timer wakeups.
139-171: LGTM!The boot delay and preset application logic handles edge cases well, including the fallback mechanism at line 160-165 that prevents the device from becoming bricked if misconfigured.
237-252: LGTM!Configuration persistence correctly handles the new fields and appropriately excludes touch-related settings on ESP32-C3.
229-229: Good practice: WiFi shutdown before deep sleep.Explicitly calling
WiFi.mode(WIFI_OFF)ensures the WiFi module is fully powered down before entering deep sleep, maximizing power savings.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
usermods/deep_sleep/deep_sleep.cpp (1)
303-303: Superfluous & out-of-range array access.This statement indexes one element past the end of the array and has no effect. Remove it.
- touch_sensor_channel_io_map[SOC_TOUCH_SENSOR_NUM];
🧹 Nitpick comments (1)
usermods/deep_sleep/deep_sleep.cpp (1)
224-230: Missing validation for touch pin capability.
touchPinis used directly without verifying it's a valid touch-capable GPIO. While the UI dropdown constrains selections, manually edited configs could specify invalid pins. Consider adding validation similar topin_is_valid().+ bool touch_pin_is_valid(uint8_t pin) { + for (int i = 0; i < SOC_TOUCH_SENSOR_NUM; i++) { + if (touch_sensor_channel_io_map[i] == pin) return true; + } + return false; + } ... - if (enableTouchWakeup) { + if (enableTouchWakeup && touch_pin_is_valid(touchPin)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/deep_sleep/deep_sleep.cpp(7 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T02:27:39.220Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-18T02:59:33.666Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T01:34:34.358Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
🔇 Additional comments (2)
usermods/deep_sleep/deep_sleep.cpp (2)
178-192: LGTM: Timer wakeup calculation now properly validated.The fixes from previous review have been applied correctly:
findNextTimerInterval()return is validated (> 1 && < INT_MAX) before arithmetic- User-defined interval properly takes precedence when preset interval is 0 or larger
129-134: LGTM: Debug output is now conditional.Wakeup cause printing is properly wrapped in
#ifdef WLED_DEBUGas requested by reviewer.
pin init is now working correctly, tested on all platforms
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @usermods/deep_sleep/deep_sleep.cpp:
- Around line 279-282: The touchPin loaded via getJsonValue(top["touchPin"],
touchPin, DEEPSLEEP_WAKEUP_TOUCH_PIN) lacks validation; implement validation
analogous to the wakeupPin check by adding a helper (e.g., validateTouchPin) or
inline logic that verifies touchPin is within SOC_TOUCH_SENSOR_NUM and mapped in
touch_sensor_channel_io_map before accepting it when enableTouchWakeup is true;
update the configComplete boolean accordingly and ensure any invalid touchPin
sets configComplete to false and logs/handles the error similar to the wakeupPin
path.
- Around line 26-28: The default DEEPSLEEP_WAKEUP_TOUCH_PIN is set to 1 which is
invalid for classic ESP32 (GPIO1 is not touch-capable); update the macro logic
to choose a platform-appropriate default: detect target macros (e.g.,
CONFIG_IDF_TARGET_ESP32_S2 / CONFIG_IDF_TARGET_ESP32S3 or equivalent
ESP32S2/ESP32S3 defines) and keep 1 for S2/S3 but set DEEPSLEEP_WAKEUP_TOUCH_PIN
to a touch-capable GPIO for classic ESP32 (for example 4) otherwise; modify the
preprocessor block around DEEPSLEEP_WAKEUP_TOUCH_PIN to use conditional
#ifdef/#elif checks so the default is correct per platform.
- Around line 212-239: The code calls touchSleepWakeUpEnable(touchPin, ...)
without verifying touchPin is touch-capable; wrap the touch wakeup block so it
only runs when enableTouchWakeup is true and digitalPinToTouchChannel(touchPin)
>= 0 (i.e., check digitalPinToTouchChannel(touchPin) before calling
touchSleepWakeUpEnable), leaving existing threshold branching intact; optionally
consider reserving the configured wakeupPin/touchPin in PinManager elsewhere to
avoid conflicts with other usermods.
🧹 Nitpick comments (4)
usermods/deep_sleep/readme.md (2)
4-4: Clarify preset wake behavior in documentation.The phrase "automation timer set to the nearest preset time" could be more explicit. Based on the code implementation (which wakes 1 minute before the next scheduled preset), consider revising to: "automation timer (wakes 1 minute before the next scheduled preset)".
This addresses the reviewer's request for clearer documentation on scheduler/preset wake behavior.
31-31: Fix hyphenation in technical term.Use hyphens in compound adjective: "pull-up/pull-down" instead of "pull up/down".
📝 Proposed fix
-- ESP32: GPIO 0, 2, 4, 12-15, 25-39 note: input-only GPIOs 34-39 do not have internal pull up/down resistors, external resistors are required +- ESP32: GPIO 0, 2, 4, 12-15, 25-39 note: input-only GPIOs 34-39 do not have internal pull-up/pull-down resistors, external resistors are requiredusermods/deep_sleep/deep_sleep.cpp (2)
73-80: Add documentation for public helper method.This public helper method lacks documentation explaining its purpose, parameters, return value, and edge case behavior (e.g., when both times are equal).
📝 Suggested documentation
+ /** + * Calculate time difference in minutes between two times (hour:minute pairs). + * If time2 is earlier than time1, assumes time2 is on the next day. + * @return Time difference in minutes (always non-negative) + */ int calculateTimeDifference(int hour1, int minute1, int hour2, int minute2) {
308-321: Touch pin dropdown generation looks correct.The conditional UI generation for touch-capable pins uses
touch_sensor_channel_io_mapandSOC_TOUCH_SENSOR_NUMappropriately. The >= 0 check (line 313) correctly filters out invalid entries.Note: Line 310 declares the array but the declaration syntax
touch_sensor_channel_io_map[SOC_TOUCH_SENSOR_NUM];doesn't actually use it—this line could be removed as the array is already declared in the included header.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
usermods/deep_sleep/deep_sleep.cppusermods/deep_sleep/readme.md
🧰 Additional context used
🧠 Learnings (28)
📚 Learning: 2026-01-03T16:42:26.035Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.035Z
Learning: In WLED PR #5267, the bus allocation strategy for single I2S mode was changed: ESP32 now uses single I2S on bus 8 (the last bus, digitalCount=9) instead of bus 0; ESP32-S2 uses single I2S on bus 4 (digitalCount=5). Memory calculation logic must account for which specific bus uses I2S in single mode, not just check digitalCount <= 8.
Applied to files:
usermods/deep_sleep/readme.md
📚 Learning: 2025-11-30T15:29:00.726Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4456
File: usermods/deep_sleep/deep_sleep.cpp:224-230
Timestamp: 2025-11-30T15:29:00.726Z
Learning: In WLED, `touchThreshold` is a global variable defined in wled00/wled.h and is accessible to all usermods. It stores the touch sensitivity threshold for touch-capable GPIO pins on ESP32 variants and should not be flagged as undefined when used in usermods.
Applied to files:
usermods/deep_sleep/readme.mdusermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2026-01-01T13:18:14.738Z
Learnt from: softhack007
Repo: wled/WLED PR: 5251
File: usermods/user_fx/user_fx.cpp:225-227
Timestamp: 2026-01-01T13:18:14.738Z
Learning: In WLED effects, when computing time differences between unsigned long timestamps (e.g., `strip.now`, `lastBumpUpdate`), always cast the subtraction result to `int` before converting to `float` to ensure correct behavior on ESP32-C3 (RISC-V). Use pattern: `float(int(timestamp1 - timestamp2))` instead of `float(timestamp1 - timestamp2)`.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED timer system, even if a timer is enabled (enable bit set in timerWeekday), it will have no effect if the preset selected (timerMacro) is 0, because there can never be a preset with ID 0.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-04-28T20:51:29.773Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, `bri` is a global variable used for brightness control.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-28T09:53:42.670Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/data/index.js:2406-2413
Timestamp: 2025-09-28T09:53:42.670Z
Learning: In WLED, code paths that appear unreachable may actually be safety fallbacks for low-heap conditions. These fallbacks are intentional defensive programming measures to ensure UI functionality is preserved when system resources are constrained.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-24T18:52:34.117Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4956
File: wled00/data/edit.htm:295-303
Timestamp: 2025-09-24T18:52:34.117Z
Learning: WLED file system stores all files in the root directory only (flat structure), so file paths don't have subdirectories. When working with WLED file editors, using just the filename (name) rather than full path is correct since there are no nested directories.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-04-26T12:51:04.993Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-22T12:12:53.486Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T02:27:39.220Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-18T02:59:33.666Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T01:34:34.358Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-03-29T01:22:54.617Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
🪛 Cppcheck (2.19.0)
usermods/deep_sleep/deep_sleep.cpp
[error] 37-37: #error Platform not supported
(preprocessorErrorDirective)
🪛 LanguageTool
usermods/deep_sleep/readme.md
[grammar] ~31-~31: Use a hyphen to join words.
Context: ...ly GPIOs 34-39 do not have internal pull up/down resistors, external resistors ar...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (11)
usermods/deep_sleep/readme.md (1)
64-64: Documentation looks good.The new build flag is properly documented. However, ensure that the default value (pin 1) is a valid touch-capable GPIO across all supported ESP32 variants in the implementation.
usermods/deep_sleep/deep_sleep.cpp (10)
35-35: Good safety improvement: disabled by default.Changing the default to
enabled = falseprevents devices from being bricked by misconfiguration, addressing a key reviewer concern. Users must explicitly enable the usermod, ensuring they understand the implications.
129-134: LGTM: Debug output properly conditional.The wake-up reason output is now properly guarded by
WLED_DEBUG, addressing the reviewer's request. The logic to resetwakeupPresetwhen the wake source is not a timer is correct.
179-193: Wake interval selection logic is correct.The logic correctly selects the minimum non-zero wake interval between the preset-based wake (1 minute before next timer) and the user-defined interval. The implementation properly handles cases where either or both are disabled.
240-240: Good power management practice.Shutting down WiFi before entering deep sleep is correct for minimizing power consumption.
255-258: Config persistence correctly handles platform differences.The conditional persistence of touch wakeup fields for non-ESP32C3 platforms is correct, as C3 lacks touch sensor support.
329-329: UI documentation is clear.The informational text for the preset wake feature is clear and helpful for users.
3-5: The touch sensor header conditional is correct.The
CONFIG_IDF_TARGET_ESP32C3guard properly excludes only the C3 variant, which lacks touch sensor support. The headersoc/touch_sensor_periph.his available on all other ESP32 variants (ESP32, ESP32-S2, ESP32-S3). Touch sensor version differences (SOC_TOUCH_VERSION_2) between S2/S3 and original ESP32 are handled appropriately elsewhere in the codebase.
82-119: This function is private, not public. The review comment incorrectly identifiesfindNextTimerInterval()as a public helper when it is declared in theprivate:section of theDeepSleepUsermodclass (line 82, withprivate:at line 34).The concerns about documenting side effects and the need for comprehensive documentation apply primarily to public APIs. For private implementation details, modifying global state is expected and normal. The global variable
wakeupPresetis already documented at its declaration (line 30) withRTC_DATA_ATTR uint8_t wakeupPreset = 0; // preset to apply after deep sleep wakeup (0 = none), set to timer macro preset.The weekday arithmetic logic is correct:
weekdayMondayFirst()returns 1-7 (Monday=1, Sunday=7), and the modulo wrapping at lines 98-100 properly handles the conversion from 0-6 to 1-7 range when adding day offsets.Likely an incorrect or invalid review comment.
166-168: Direct GPIO control is the standard relay handling pattern in WLED usermods.The TODO comment is valid to clarify intent, but the implementation is correct. Both deep_sleep and WLED's multi_relay usermod use direct
digitalWrite()for relay control—there is no higher-level WLED relay API to call. If state management is desired, consider referencing multi_relay's approach, but direct GPIO manipulation is the established pattern in WLED usermods.
195-211: PinManager cannot be used for deep sleep GPIO operations; the current direct GPIO approach is necessary.The PinManager API only provides
allocatePin,deallocatePin, and related functions for standard GPIO resource tracking. It lacks the deep-sleep-specific functions needed here:gpio_hold_en/dis,gpio_pullup/pulldown_en/dis, andesp_deep_sleep_enable_gpio_wakeup. These RTC IO configuration functions are ESP-IDF-specific and required for the pin state to persist during deep sleep. The direct GPIO calls are not a bypass—they are the only way to configure deep sleep wakeup pins. This pattern is consistent with other WLED usermods likepixels_dice_tray.cpp, which uses the same approach (lines 287–288).The comment on line 211 documenting the pullup disable behavior is intentional; it describes a documented ESP32-C3 RTC GPIO quirk that the implementation intentionally leverages.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @usermods/deep_sleep/deep_sleep.cpp:
- Around line 279-282: After reading enableTouchWakeup and touchPin in
readFromConfig (where getJsonValue is used), validate touchPin the same way
wakeupPin is validated: check touchPin against the platform's valid touch sensor
channels and mark configComplete = false if it's out of range; if available,
reuse the same helper used for wakeupPin validation (or the platform-specific
touch-channel validator) to perform the check and fall back to
DEEPSLEEP_WAKEUP_TOUCH_PIN when invalid so UI dropdowns and touch wakeup logic
use a safe value.
- Around line 230-236: The code enables touch wakeup unconditionally when
enableTouchWakeup is true; first validate that touchPin maps to a valid touch
channel by checking touch_sensor_channel_io_map[touchPin] (or the project's
equivalent mapping) is not -1 before calling touchSleepWakeUpEnable; if the
mapping is invalid, skip enabling touch wakeup and emit a warning via the
existing logger (or clear enableTouchWakeup) so you don't call
touchSleepWakeUpEnable with an invalid pin; keep the platform-specific
SOC_TOUCH_VERSION_2 shift logic but only run it after the mapping check and
reference enableTouchWakeup, touchPin, touchSleepWakeUpEnable, and
touch_sensor_channel_io_map in the fix.
- Around line 147-149: The code clears the powerup flag immediately when
sleepDelay > 0 which is premature; only clear powerup after the configured delay
has elapsed. Change the logic so that when sleepDelay > 0 you initialize
lastLoopTime if needed but do NOT set powerup = false there; instead, after
checking the elapsed time (millis() - lastLoopTime >= sleepDelay) and right
before entering sleep, set powerup = false and proceed with the sleep path. Keep
lastLoopTime initialization as-is and avoid resetting it when clearing powerup.
- Line 112: The assignment wakeupPreset = timerMacro[i] should not
unconditionally accept a zero preset; either skip timers where timerMacro[i] ==
0 or document intent. Update the loop/logic that sets wakeupPreset (refer to
timerMacro and wakeupPreset in deep_sleep.cpp) to check if timerMacro[i] != 0
before assigning/processing, mirroring the checks used in wled00/ntp.cpp and
EEPROM logic, or add a clear comment explaining that a zero value intentionally
means “wake with no preset” if that behavior is desired.
- Line 176: The wakeupPin is being configured directly with pinMode(wakeupPin,
INPUT) instead of being claimed via WLED's pin manager; in setup() (after config
load) call PinManager::allocateMultiplePins() or the appropriate allocate method
using PinOwner::UM_DEEP_SLEEP to reserve wakeupPin (and any related pins) before
configuring it, handle allocation failures (release or log) and only then call
pinMode/wakeup configuration; update deep_sleep.cpp to use the PinManager
allocation APIs and reference PinOwner::UM_DEEP_SLEEP when claiming the pin.
🧹 Nitpick comments (2)
usermods/deep_sleep/deep_sleep.cpp (2)
83-83: Replace magic number with named constant.The hardcoded timestamp
1672502400(2023-01-01 00:00:00 UTC) should be replaced with a named constant for clarity and maintainability.♻️ Suggested improvement
+// Minimum valid timestamp for NTP-synced time (2023-01-01 00:00:00 UTC) +#define MIN_VALID_TIMESTAMP 1672502400 + int findNextTimerInterval() { - if (localTime < 1672502400) { // time invalid before NTP sync 2023-1-1 + if (localTime < MIN_VALID_TIMESTAMP) { // time invalid before NTP syncBased on learnings, magic numbers should be replaced with meaningful constants when they represent significant values.
211-211: Fix typo and clarify confusing comment.The comment contains a typo ("hole_en" should be "hold_en") and states "pullups are disabled for some reason," which contradicts the manual pullup configuration just performed above. Based on PR discussion,
gpio_hold_enbehavior is platform/pin-specific and may require further investigation.📝 Suggested comment improvement
- gpio_hold_en((gpio_num_t)wakeupPin); // if hole_en is used, pullups are disabled for some reason, lets make use of that + gpio_hold_en((gpio_num_t)wakeupPin); // hold GPIO state during deep sleep (note: hold behavior with pulls is pin-specific on C3)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/deep_sleep/deep_sleep.cpp
🧰 Additional context used
🧠 Learnings (29)
📓 Common learnings
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
📚 Learning: 2025-11-30T15:29:00.726Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4456
File: usermods/deep_sleep/deep_sleep.cpp:224-230
Timestamp: 2025-11-30T15:29:00.726Z
Learning: In WLED, `touchThreshold` is a global variable defined in wled00/wled.h and is accessible to all usermods. It stores the touch sensitivity threshold for touch-capable GPIO pins on ESP32 variants and should not be flagged as undefined when used in usermods.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2026-01-01T13:18:14.738Z
Learnt from: softhack007
Repo: wled/WLED PR: 5251
File: usermods/user_fx/user_fx.cpp:225-227
Timestamp: 2026-01-01T13:18:14.738Z
Learning: In WLED effects, when computing time differences between unsigned long timestamps (e.g., `strip.now`, `lastBumpUpdate`), always cast the subtraction result to `int` before converting to `float` to ensure correct behavior on ESP32-C3 (RISC-V). Use pattern: `float(int(timestamp1 - timestamp2))` instead of `float(timestamp1 - timestamp2)`.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED timer system, even if a timer is enabled (enable bit set in timerWeekday), it will have no effect if the preset selected (timerMacro) is 0, because there can never be a preset with ID 0.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-04-28T20:51:29.773Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, `bri` is a global variable used for brightness control.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-28T09:53:42.670Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/data/index.js:2406-2413
Timestamp: 2025-09-28T09:53:42.670Z
Learning: In WLED, code paths that appear unreachable may actually be safety fallbacks for low-heap conditions. These fallbacks are intentional defensive programming measures to ensure UI functionality is preserved when system resources are constrained.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-24T18:52:34.117Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4956
File: wled00/data/edit.htm:295-303
Timestamp: 2025-09-24T18:52:34.117Z
Learning: WLED file system stores all files in the root directory only (flat structure), so file paths don't have subdirectories. When working with WLED file editors, using just the filename (name) rather than full path is correct since there are no nested directories.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-04-26T12:51:04.993Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-11-22T12:12:53.486Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-03-29T19:21:52.806Z
Learnt from: ComicoX
Repo: wled/WLED PR: 4626
File: wled00/button.cpp:116-118
Timestamp: 2025-03-29T19:21:52.806Z
Learning: ESP32-C6 does not have built-in touch sensors, unlike other ESP32 variants.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T02:27:39.220Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-18T02:59:33.666Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-08-29T01:34:34.358Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
📚 Learning: 2025-03-29T01:22:54.617Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Applied to files:
usermods/deep_sleep/deep_sleep.cpp
🪛 Cppcheck (2.19.0)
usermods/deep_sleep/deep_sleep.cpp
[error] 37-37: #error Platform not supported
(preprocessorErrorDirective)
🔇 Additional comments (7)
usermods/deep_sleep/deep_sleep.cpp (7)
29-30: LGTM: RTC-persisted state variables.The use of
RTC_DATA_ATTRto persistpowerupandwakeupPresetacross deep sleep cycles is correct and necessary for the preset wake-up functionality.
40-41: Verify touch implementation matches reviewer requirements.The reviewer requested adding touch as a GPIO config option similar to pullup/pulldown handling, rather than as separate boolean and pin fields. The current implementation uses separate
enableTouchWakeupandtouchPinmembers. Confirm this approach was agreed upon or if the reviewer expects a different design.Based on PR comments, the reviewer emphasized that only sleep-specific GPIO handling should be implemented and requested touch be added as a GPIO config option.
52-70: LGTM: Platform-specific pin validation.The pin validation correctly checks RTC-capable GPIOs for each ESP32 variant and includes a helpful note about input-only pins on ESP32.
129-134: LGTM: Debug output properly gated.The wakeup cause debug output is correctly wrapped in
#ifdef WLED_DEBUGas requested by the reviewer, and the preset reset logic appropriately handles non-timer wakeups.
328-328: LGTM: Preset wake documentation added.The informational text clearly documents the preset wake feature as requested by the reviewer.
166-168: The relay control code is correct and consistent with WLED's design.Direct GPIO manipulation via
digitalWrite()does not bypass WLED's pin manager. WLED core itself uses this exact pattern: the relay pin is allocated viaPinManager::allocatePin(rlyPin, true, PinOwner::Relay)in configuration (cfg.cpp, set.cpp), and then controlled withdigitalWrite()(wled.cpp:598, button.cpp:379/390). The deep_sleep usermod correctly follows the same approach by using the globally-allocatedrlyPinvariable. There is no separate "WLED relay API function"—digitalWrite()after pin manager allocation is the standard relay control mechanism throughout the codebase.Likely an incorrect or invalid review comment.
308-320: The implementation is correct. ThetouchSleepWakeUpEnable()function signature isvoid touchSleepWakeUpEnable(uint8_t pin, touch_value_t threshold)where the first parameter expects a GPIO pin number, not a touch channel number. The dropdown correctly:
- Iterates through touch channels (0 to SOC_TOUCH_SENSOR_NUM)
- Maps each channel to its GPIO pin via
touch_sensor_channel_io_map[touchchannel]- Stores and passes the GPIO pin number to
touchSleepWakeUpEnable(), which is exactly what the API expectsNo changes are needed.
| } | ||
|
|
||
| int findNextTimerInterval() { | ||
| if (localTime < 1672502400) { // time invalid before NTP sync 2023-1-1 |
There was a problem hiding this comment.
This mean that the usermod will stop working in 2038, when 32bit unixtime will "roll over" and start a 0.
https://en.wikipedia.org/wiki/Year_2038_problem
(sorry for the late feedback, just stumbled over this)
There was a problem hiding this comment.
Instead of hard-coded number magic, you might want to look into our "toki" class that can especially tell you what kind of time source WLED is using at the moment.
There was a problem hiding this comment.
thanks for the feedback, there were quite a lot of issues with this PR initially, I fixed most of them but this one I missed.
4 participants
The SleepManager module for WLED is designed to manage power consumption by enabling the ESP32 to enter deep sleep based on idle time or battery voltage levels. This module allows for more efficient battery use by automatically putting the device to sleep and waking it up based on predefined conditions. It is especially useful for battery-powered setups where minimizing power consumption is essential.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.