nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.62k stars 3.12k forks source link

Don't error from node.sleep() when a sleep reject occurs #3560

Open tomsci opened 1 year ago

tomsci commented 1 year ago

In IDFv4 there's a new return value from esp_light_sleep_start which needs handling in node.sleep(). It's not really documented as far as I can see, but in the IDF source it's termed a "sleep reject" meaning (I think) that the conditions for wakeup already apply so the board doesn't actually ever go to sleep. You see it if you try to sleep with a GPIO low wakeup, and the GPIO is already low, for example. We really don't care about this nuance (compared to sleeping then immediately waking up), so the change is to just ignore the (undocumented) return value from esp_light_sleep_start and treat it the same as slept-then-woke-up, rather than erroring.

Now I think about it some more writing this PR, given it's not documented, maybe we should just return a boolean from node.sleep() indicating whether the sleep succeeded or not. Never error, don't make assumptions about the return value other than zero/nonzero, and get rid of the ESP_ERR_INVALID_STATE check which doesn't appear to actually be used any more?

jmattsson commented 5 months ago

While I like not having errors thrown needlessly, I don't see how we could find out the correct return value in the reject case?

pjsg commented 5 months ago

In some sense, the two error cases are 'system didn't sleep because the timeout happened or the wakeup condition was already satisfied'. In some senses this is the same as 'it timed out, or the wakeup condition was triggered'

jmattsson commented 5 months ago

We currently return the wakeup cause from this function, but I guess we could use ESP_SLEEP_WAKEUP_UNDEFINED for the sleep-rejected case? Not sure whether that's cleaner that the current behaviour though?