homieiot / homie-esp8266

💡 ESP8266 framework for Homie, a lightweight MQTT convention for the IoT
http://homieiot.github.io/homie-esp8266
MIT License
1.36k stars 308 forks source link

Added deep sleep support for ESP32 #600

Closed EinfachArne closed 4 years ago

EinfachArne commented 5 years ago

I added support for deep sleep with an ESP32.

Changes I made:

With this solution the ESP will never wake up, unless you define a way to wake it up for yourself. I can change this so that it automatically wakes up after a defined period of time, just like with the 8266. But I think the user should be able to choose his own wake up method as described here: https://docs.espressif.com/projects/esp-idf/en/latest/api-reference/system/sleep_modes.html

My question is: Should there be a way within the framework to choose the preferred way or should the user do it by himself, outside the framework?

kleini commented 5 years ago

I would try to keep the same API as homie-esp8266 currently has for ESP8266. ESP32 has support for sleeping a certain amount of time and therefore I would keep the same parameters on the doDeepSleep() method. This eases switching the hardware device without having the necessity to change the code. Later homie-esp8266 can add methods to support the other possible sleep variants of ESP32.

There are a lot of code formatting changes. Those make it hard to identify real code changes. My opinion is to put code formatting changes into completely separated pull requests. Btw I hate opening curly braces in a new line. This produces just one extra line for every code block and makes the code somewhat more unreadable as it already is.

kleini commented 5 years ago

Good, that the linter already told you, where you have to place the opening curly braces.

EinfachArne commented 5 years ago

Thanks for your input. Now I adapted the API to be almost the same as for the 8266.

Sorry for the formatting changes, I think I reverted all of them. At least the linter is happy with it :)

andreacioni commented 5 years ago

Hi guys! It will be awesome to allow ESP32 users to take advantage of the wake up capabilities that the hardware offers. Something like this should work:

void HomieClass::doDeepSleep(uint32_t time_us = 0, gpio_num_t gpio_pin=-1, int logic_level=-1) {
  Interface::get().getLogger() << F("💤 Device is deep sleeping...") << endl;
  Serial.flush();

  if(gpio_pin > 0 && logic_level > 0) {
    esp_sleep_enable_ext0_wakeup(gpio_pin,logic_level);
  }

  esp_sleep_enable_timer_wakeup(time_us);
  esp_deep_sleep_start();
}

What do you think?

EinfachArne commented 5 years ago

Perfectly fine for me but I am not the one to decide this. I added your suggestion into my pull request. Now let us see what the maintainers decide.

kleini commented 4 years ago

@EinfachArne you should rebase your branch on develop-v3 instead of merging develop-v3 changes into your branch. This will result in a much cleaner Git history.

stritti commented 4 years ago

@EinfachArne & @kleini could you rebase to branch develop please? Renaming the branches closed the PR. Sorry.