Skip to content

Conversation

@bugadani
Copy link
Contributor

@bugadani bugadani commented Jul 17, 2023

This PR adapts #574 for the ESP32-S3. The PR also includes some minor changes to the ESP32 implementation based on esp-idf.

cc #375

@bugadani bugadani force-pushed the deep_sleep branch 2 times, most recently from d79d4a1 to 59c548a Compare July 17, 2023 15:25
@bugadani
Copy link
Contributor Author

bugadani commented Jul 17, 2023

esp-idf's rtc_init, which is the basis of base_settings seems to have a bunch of code that should run on startup (LDO bias settings for example). I am unsure how to approach those. Currently, everything is part of base_settings but I'm fairly certain that applying a low LDO bias for sleep could actually not let the MCU wake up properly, in a stable manner.

This probably means we'll need to tie some of the sleep infra into clock initialization, for example.

Or, maybe I shouldn't complicate matters and we should mandate the user to call some initialization function when using sleep.

@bugadani
Copy link
Contributor Author

bugadani commented Jul 18, 2023

I have hijacked RtcSleepConfig a bit to actually implement MCU-specific sleep logic, partitioned somewhat similarly to esp-idf: by including a start_sleep and a finish_sleep function. I've moved the register access code from the parent module into RtcSleepConfig, hopefully this will help more (in terms of MCU support) than it hurts (by requiring duplication).

A current worry I have is that we leak RtcSleepConfig in place of esp-idf's sleep flags. This exposes more options than necessary and allows the users to set incompatible configurations.

@bugadani bugadani marked this pull request as ready for review July 20, 2023 12:49
@bugadani bugadani force-pushed the deep_sleep branch 2 times, most recently from 76784ac to ee35435 Compare July 21, 2023 09:49
Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, works for me so I see no reason not to merge this as is. Thanks for looking into this!

@MabezDev MabezDev merged commit 9f5e2b5 into esp-rs:main Jul 24, 2023
@bugadani bugadani deleted the deep_sleep branch July 24, 2023 19:21
@bugadani
Copy link
Contributor Author

works for me

oh wow I didn't actually test this because I need to implement GPIO wakeup for my stuff, but that's great to hear!

playfulFence pushed a commit to playfulFence/esp-hal that referenced this pull request Sep 26, 2023
* Add changelog entry

* Copy esp32 impl, update RtcSleepConfig

* implement apply

* extract rtc_sleep_pu

* Implement base_settings based on esp-idf rtc_init

* Hide CPU-specific sleep code

* Set base_settings when constructing Rtc

* Add s3 deep sleep defaults

* Implement finish_sleep

* Turn magic constant into enum

* Clear ext1 wakeup status

* Add wakeup source impls

* Add examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants