WLED icon indicating copy to clipboard operation
WLED copied to clipboard

Improvements to deepsleep UM

Open elanworld opened this issue 1 year ago • 37 comments

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

    • Touch-sensor wakeup on compatible ESP32 boards and an option to wake before the next preset timer event; UI/config exposes touch-pin where supported.
    • Added next-timer calculation to allow preset-based wake scheduling.
  • Bug Fixes / Improvements

    • Improved wakeup timing to pick the nearest valid interval and ensure Wi‑Fi is turned off before sleep.
    • Board-specific handling: touch-pin option omitted where unsupported; wake-pin inputs validated.
  • Documentation

    • Deep Sleep docs updated to clarify wakeup methods and new settings.

✏️ Tip: You can customize this high-level summary in your review settings.

elanworld avatar Jan 04 '25 06:01 elanworld

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?

DedeHai avatar Jan 04 '25 06:01 DedeHai

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

elanworld avatar Jan 07 '25 09:01 elanworld

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.

DedeHai avatar Jan 07 '25 10:01 DedeHai

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?

elanworld avatar Jan 07 '25 12:01 elanworld

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.

DedeHai avatar Jan 07 '25 12:01 DedeHai

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?

elanworld avatar Jan 08 '25 02:01 elanworld

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

DedeHai avatar Jan 08 '25 05:01 DedeHai

Add only the touch option and preset wakeup timer

elanworld avatar Jan 12 '25 03:01 elanworld

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

DedeHai avatar Jan 12 '25 10:01 DedeHai

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?

elanworld avatar Jan 12 '25 15:01 elanworld

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 */

elanworld avatar Jan 13 '25 03:01 elanworld

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.

DedeHai avatar Jan 13 '25 06:01 DedeHai

done

elanworld avatar Jan 13 '25 07:01 elanworld

What does draft status mean in this repository?

elanworld avatar Jan 14 '25 01:01 elanworld

draft means not finished and not ready to merge

DedeHai avatar Jan 14 '25 04:01 DedeHai

what sleps are needed to complete the pull request?

elanworld avatar Jan 14 '25 09:01 elanworld

fix the ones I marked

DedeHai avatar Jan 14 '25 11:01 DedeHai

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

elanworld avatar Jan 15 '25 02:01 elanworld

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

see inline comments.

DedeHai avatar Jan 15 '25 06:01 DedeHai

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.

elanworld avatar Jan 15 '25 07:01 elanworld

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

DedeHai avatar Jan 15 '25 08:01 DedeHai

image 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

elanworld avatar Jan 15 '25 09:01 elanworld

image

DedeHai avatar Jan 15 '25 09:01 DedeHai

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

elanworld avatar Jan 15 '25 13:01 elanworld

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

DedeHai avatar Jan 15 '25 14:01 DedeHai

image

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


Screenshot_20250115-155324_Samsung Internet

softhack007 avatar Jan 15 '25 14:01 softhack007

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

DedeHai avatar Jan 15 '25 15:01 DedeHai

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

elanworld avatar Jan 22 '25 12:01 elanworld

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

DedeHai avatar Jan 22 '25 13:01 DedeHai

Walkthrough

The changes introduce enhancements to the Deep Sleep usermod, adding support for touch sensor-based wakeup and enabling wakeup based on the next scheduled preset timer event. The configuration and documentation are updated to reflect these new features, including new options for specifying a touch wakeup pin and enabling timer-based preset wakeup. The pin manager is updated with a new enumerator for deep sleep usermod pin ownership. Additional debug information and validation are incorporated, and the configuration schema and UI are extended to support the new options.

Changes

Files/Paths Change Summary
usermods/deep_sleep/readme.md Updated documentation to clarify wakeup sources (including preset timer) and added information about the new DEEPSLEEP_WAKEUP_TOUCH_PIN configuration option.
wled00/pin_manager.h Added UM_DEEP_SLEEP enumerator to PinOwner enum for deep sleep usermod pin management; fixed missing comma in enum.
usermods/deep_sleep/deep_sleep.cpp Enhanced Deep Sleep usermod with touch sensor wakeup (except ESP32C3), timer-based preset wakeup, new configuration options and UI, new helper methods for timer calculations, updated wakeup logic, debug outputs, and validation improvements.

[!TIP]

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.
✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Apr 17 '25 06:04 coderabbitai[bot]