add deepsleep module for battery application #4456

elanworld · 2025-01-04T06:06:54Z

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.

DedeHai · 2025-01-04T06:43:02Z

thank you for contributing!
there are few things implemented in a too simple way though: you should not override any pins, use the pin manager or it will lead to conflicting pin configurations if a pin is used for something else.
There already is a battery usermod that can do all that and more, apart from deep-sleep and there is a deep-sleep usermod that can do the rest with the exception of touch-wake up. Did you try combining those two?

elanworld · 2025-01-07T09:37:05Z

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

DedeHai · 2025-01-07T10:00:54Z

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 battery usermod allows to trigger a preset depending on battery voltage, the deep-sleep usermod also allows delays. so only thing missing is touch wake-up.
Edit:
btw: pullup/pulldown in deep-sleep can not be set the way you do it (and it probably is also still wrong in my UM) reason is: there is a "feature" in the IDF that automatically sets them.

elanworld · 2025-01-07T12:30:17Z

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?

DedeHai · 2025-01-07T12:59:27Z

The deep-sleep UM primarily uses a wakeup timer as its trigger source from deep-sleep status.

how did you figure that?

Shouldn't this functionality be distinct from the battery UM?

deep sleep has nothing to do with battery UM, so dont know what you mean.

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?

did you test to disable pullup/down successfully?
yes, I found a way around: you can override it by setting it to hold mode.

elanworld · 2025-01-08T02:29:50Z

if (presetWake) {
nextWakeupMin = findNextTimerInterval() - 1; // wakeup before next preset
}
esp_sleep_enable_timer_wakeup(nextWakeupMin * 60ULL *
(uint64_t)1e6); // wakeup for preset

how did you figure that?

The battery usermod allows to trigger a preset depending on battery voltage, the deep-sleep usermod also allows delays.

Unlike the battery usermod, the deep-sleep usermod supports preset delays, highlighting their distinct functionalities."

you can override it by setting it to hold mode.

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?

DedeHai · 2025-01-08T05:20:05Z

if (presetWake) { nextWakeupMin = findNextTimerInterval() - 1; // wakeup before next preset } esp_sleep_enable_timer_wakeup(nextWakeupMin * 60ULL * (uint64_t)1e6); // wakeup for preset

I thought you were referring to the existing deep-sleep UM, its a bit confusing both are called that :)
This is actually something I wanted to implement as well so appreciate the addition. However, I do not like at all how you added the code to the existing UM. You changed the whole structure, making a review almost impossible as almost all code is replaced.
Also you broke functionality for C3 and S2. There is debug info in the code: displaying wake up reason should be debug only, the user has no benefit from that.
Configuring GPIOs should be left as it is, the way you implement it is flawed.

Unlike the battery usermod, the deep-sleep usermod supports preset delays, highlighting their distinct functionalities."

I dont know what you mean by that.

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 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:

keep the existing code and only change/add whats needed instead of replacing all the code with new functions
the additions in the readme should say what it does, instead it just gives hints (I still dont know what you mean by that even after reading the code) - it should also say how it handles preset wake-up: what you mean is scheduler wake up I think. Also I saw no code for GPIO handling for the FETs that are mentioned, so not sure what that line is about.
leave GPIO handling to WLED, implement sleep GPIO handling as it was
for touch: add a config option to the GPIO, like it is done for pullup/down
test on S3, C3 and S2 as well, use ifdefs where needed

elanworld · 2025-01-12T03:59:26Z

Add only the touch option and preset wakeup timer

DedeHai · 2025-01-12T10:02:47Z

still has that debug info
still does not work on C3 and ESP32

elanworld · 2025-01-12T15:39:48Z

displaying wake up reason should be debug only,
still has that debug info

Are you referring to this line:
DEBUG_PRINTF("boot type: %s\n", phase_wakeup_reason());

still does not work on C3 and ESP32

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.
Could you clarify what exactly is not working on the ESP32?

usermods/deep_sleep/usermod_deep_sleep.h

elanworld · 2025-01-13T03:32:25Z

still has that debug info

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.

still does not work on C3 and ESP32

I found this code for esp32 c3
#define SOC_TOUCH_SENSOR_NUM (0) /*! No touch sensors on ESP32-C3 */

DedeHai · 2025-01-13T06:40:56Z

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.

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.

elanworld · 2025-01-13T07:48:05Z

done

elanworld · 2025-01-14T01:33:23Z

What does draft status mean in this repository?

DedeHai · 2025-01-14T04:52:03Z

draft means not finished and not ready to merge

elanworld · 2025-01-14T09:34:06Z

what sleps are needed to complete the pull request?

DedeHai · 2025-01-14T11:02:30Z

fix the ones I marked

elanworld · 2025-01-15T02:58:34Z

What problems need to be fixed now? Do I need to fix them?

DedeHai · 2025-01-15T06:08:09Z

What problems need to be fixed now? Do I need to fix them?

see inline comments.

elanworld · 2025-01-15T07:22:31Z

After pushing the latest commit, I believe I have resolved all the issues you raised. If there's anything you're still not satisfied with, please let me know.

DedeHai · 2025-01-15T08:20:59Z

there is plenty of unresolved comments
https://github.com/Aircoookie/WLED/pull/4456/files

elanworld · 2025-01-15T09:27:43Z

I just noticed a marked conversation that I solved in this pull request files, but I can't see any comments. Could you provide a link or a screenshot? Thanks!

Or do you mean the conversations? I believe I have resolved all the issues you mentioned. If not, please let me know or show them to me again

DedeHai · 2025-01-15T09:30:50Z

elanworld · 2025-01-15T13:29:20Z

Why can't I see any comments like the ones in your screenshot?

DedeHai · 2025-01-15T14:09:57Z

I have no idea, maybe you disabled inline comments in github? that is beyond my knowledge I am afraid.

softhack007 · 2025-01-15T14:49:41Z

@DedeHai your comments are marked "pending" - it means you may need to finish your review for the comments to be published.

usermods/deep_sleep/usermod_deep_sleep.h

DedeHai · 2025-01-15T15:04:05Z

@softhack007 thanks, that may have been it. interesting, I never had to do that before...

elanworld · 2025-01-22T12:37:21Z

Added a commit for ESP32-C3 and ESP32-S2 development boards

DedeHai · 2025-01-22T13:22:27Z

thanks for the update, I will look at it in more detail soon.

DedeHai · 2025-01-22T13:16:05Z

usermods/deep_sleep/usermod_deep_sleep.h

  #endif
-
+      WiFi.disconnect();


this is not needed, it is shut down in deep sleep.

I read that it is best practice to do this to avoid timeouts on connected clients, may be useful if sleep time is short, so keep it.

DedeHai · 2025-01-27T06:19:48Z

wled00/pin_manager.h

@@ -66,7 +66,8 @@ enum struct PinOwner : uint8_t {
  UM_LDR_DUSK_DAWN     = USERMOD_ID_LDR_DUSK_DAWN,      // 0x2B // Usermod "usermod_LDR_Dusk_Dawn_v2.h"
  UM_MAX17048          = USERMOD_ID_MAX17048,           // 0x2F // Usermod "usermod_max17048.h"
  UM_BME68X            = USERMOD_ID_BME68X,             // 0x31 // Usermod "usermod_bme68x.h -- Uses "standard" HW_I2C pins
-  UM_PIXELS_DICE_TRAY  = USERMOD_ID_PIXELS_DICE_TRAY    // 0x35 // Usermod "pixels_dice_tray.h" -- Needs compile time specified 6 pins for display including SPI.
+  UM_PIXELS_DICE_TRAY  = USERMOD_ID_PIXELS_DICE_TRAY,   // 0x36 // Usermod "pixels_dice_tray.h" -- Needs compile time specified 6 pins for display including SPI.


please revert unnecessary changes

DedeHai · 2025-01-27T06:20:48Z

usermods/deep_sleep/usermod_deep_sleep.h

+      if (wakeupAfter) {
+        nextWakeupMin = nextWakeupMin < wakeupAfter / 60.0
+                            ? nextWakeupMin
+                            : wakeupAfter / 60.0;


a) dont use floats
b) use seconds and not minutes throughout the code: this breaks original code: if I set to 5s, it will turn to 0

DedeHai · 2025-01-27T06:21:09Z

usermods/deep_sleep/usermod_deep_sleep.h

+      if (nextWakeupMin > 0) {
+        esp_sleep_enable_timer_wakeup(nextWakeupMin * 60ULL *
+                                      (uint64_t)1e6);
+        DEBUG_PRINTF("wakeup after %d minites", nextWakeupMin);


DedeHai · 2025-01-27T06:30:10Z

usermods/deep_sleep/usermod_deep_sleep.h

-      esp_sleep_enable_timer_wakeup((uint64_t)wakeupAfter * (uint64_t)1e6); //sleep for x seconds
+      int nextWakeupMin = 0;
+      if (presetWake) {
+        nextWakeupMin = findNextTimerInterval() - 1; // wakeup before next preset


sleep is not that accurate, if you sleep for 2 days, this can miss the preset. not sure what a good way to handle it would be. maybe wake up offset should depend on length of sleep, use datasheet accuracy values to determine the amount (I think RTC accuracy is ±2% but did not check).
also when this wake-up is used, the lights should not turn on until the preset time is reached. could also check how far off the time still is and go back to deep-deep sleep.

DedeHai · 2025-01-27T06:31:37Z

usermods/deep_sleep/usermod_deep_sleep.h

+        esp_sleep_enable_timer_wakeup(nextWakeupMin * 60ULL *
+                                      (uint64_t)1e6);
+        DEBUG_PRINTF("wakeup after %d minites", nextWakeupMin);
+        DEBUG_PRINTLN("");


just make the new line in the print above this line and remove.

DedeHai · 2025-01-27T06:32:28Z

usermods/deep_sleep/usermod_deep_sleep.h

-
+      WiFi.disconnect();
+      WiFi.mode(WIFI_OFF);  // Completely shut down the Wi-Fi module
+      #ifndef CONFIG_IDF_TARGET_ESP32C3


why not move this in the ifdef above?

DedeHai · 2025-01-27T06:33:58Z

usermods/deep_sleep/usermod_deep_sleep.h

+    }
+
+    int findNextTimerInterval() {
+      int currentHour = hour(localTime), currentMinute = minute(localTime),


add a check here if the time is actually valid, not sure WLED already has a function for that but if time is not valid, this function will return bogus values.

add deepsleep module for battery application

2e3f42a

Merge branch 'main' of github.com:Aircoookie/WLED into deepsleep

b16c33c

softhack007 added the usermod usermod related label Jan 4, 2025

merge new Features to exists deep sleep usermod

444c1b8

elanworld force-pushed the deepsleep branch from 0c1899f to 444c1b8 Compare January 7, 2025 09:28

remove gpio configuretion; add touch pin option

994c7e1

DedeHai reviewed Jan 12, 2025

View reviewed changes

usermods/deep_sleep/usermod_deep_sleep.h Show resolved Hide resolved

config for esp32 c3

d3cf39b

remove wakeup reason from ui

464abad

DedeHai marked this pull request as draft January 13, 2025 08:11

elanworld marked this pull request as ready for review January 14, 2025 01:26

elanworld requested a review from DedeHai January 14, 2025 01:31

elanworld marked this pull request as draft January 14, 2025 01:32

add #ifdef in debug mode

cc29953

elanworld marked this pull request as ready for review January 15, 2025 07:22

DedeHai reviewed Jan 15, 2025

View reviewed changes

For ESP32-C3 and ESP32-S2 development boards

1767bc0

elanworld force-pushed the deepsleep branch from 6ef9272 to 1767bc0 Compare January 22, 2025 12:42

DedeHai reviewed Jan 27, 2025

View reviewed changes

add deepsleep module for battery application #4456

Are you sure you want to change the base?

add deepsleep module for battery application #4456

Conversation

elanworld commented Jan 4, 2025

DedeHai commented Jan 4, 2025 • edited Loading

elanworld commented Jan 7, 2025

DedeHai commented Jan 7, 2025 • edited Loading

elanworld commented Jan 7, 2025 • edited Loading

DedeHai commented Jan 7, 2025

elanworld commented Jan 8, 2025

DedeHai commented Jan 8, 2025

elanworld commented Jan 12, 2025

DedeHai commented Jan 12, 2025

elanworld commented Jan 12, 2025

elanworld commented Jan 13, 2025

DedeHai commented Jan 13, 2025

elanworld commented Jan 13, 2025

elanworld commented Jan 14, 2025

DedeHai commented Jan 14, 2025

elanworld commented Jan 14, 2025

DedeHai commented Jan 14, 2025

elanworld commented Jan 15, 2025

DedeHai commented Jan 15, 2025

elanworld commented Jan 15, 2025

DedeHai commented Jan 15, 2025

elanworld commented Jan 15, 2025

DedeHai commented Jan 15, 2025

elanworld commented Jan 15, 2025

DedeHai commented Jan 15, 2025

softhack007 commented Jan 15, 2025 • edited Loading

DedeHai commented Jan 15, 2025

elanworld commented Jan 22, 2025

DedeHai commented Jan 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DedeHai commented Jan 4, 2025 •

edited

Loading

DedeHai commented Jan 7, 2025 •

edited

Loading

elanworld commented Jan 7, 2025 •

edited

Loading

softhack007 commented Jan 15, 2025 •

edited

Loading