Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added deep sleep support for ESP32 #600

Closed
wants to merge 27 commits into from
Closed

Added deep sleep support for ESP32 #600

wants to merge 27 commits into from

Conversation

EinfachArne
Copy link
Contributor

I added support for deep sleep with an ESP32.

Changes I made:

  • HomieClass::prepareToSleep() is now used for both platforms (ESP8266 & ESP32) but remains unchanged.

  • HomieClass::doDeepSleep() is new for the ESP32. The only change compared to the 8266 version is that the method for ESP32 is called: esp_deep_sleep_start();

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
Copy link
Collaborator

kleini commented May 2, 2019

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
Copy link
Collaborator

kleini commented May 2, 2019

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

@EinfachArne
Copy link
Contributor Author

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

@EinfachArne EinfachArne marked this pull request as ready for review May 3, 2019 00:19
@andreacioni
Copy link

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
Copy link
Contributor Author

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.

(cherry picked from commit 124992d)
stritti and others added 13 commits July 23, 2019 09:20
* Fixed device not reconnecting after MQTT connection loss

* formatting

(cherry picked from commit edb5d62)
* TLS support + fingerprint attribute in config file ( #399 )
Credit @adriancuzman

(cherry picked from commit 00843f3)
(cherry picked from commit c63aeb6)

* Refactor SSL to only compile when enabled (`#if ASYNC_TCP_SSL_ENABLED`)

(cherry picked from commit 2a8a8f1)
(cherry picked from commit 050cc8b)

* corrected string and char array length

(cherry picked from commit 107fbdf)

* documented how to use SSL encryption for MQTT

(cherry picked from commit ebac43a)

* replaced board esp01 with esp01_1m due to failing CI builds because of growing image size
When the configSize is MAX_JSON_CONFIG_FILE_SIZE the '\0' is written
after the buffers end. Don't load config files with a
size >= MAX_JSON_CONFIG_FILE_SIZE.

Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
* Migrate the code from ArduinoJson v5 to v6
* Resolve memory allocation issues resulting from the migration
* Reduce unnecessary copying of some JSON related strings
* Change to StaticJsonBuffer if possible
* Use C++ casts instead of C-style casts
* Remove unnecessary comments
* Serialize JsonDocument instead of JsonObject
* Fix wrong MQTT topic buffer size calulation which might lead to a buffer overflow
…topic is truncated (#526)

* Declare _mqttTopicCopy in BootNormal.hpp
* Using _mqttTopicCopy to manage subscribed topics
* Removed leading spaces
…#641) (#642)

Removed the lines copying the sketch MD5 at the BootNormal constructor
since that crashed the firmware. At that point the ESP object is not ready
to invoke getSketchMD5(). Also the code was duplicated since it's was
already invoked at the BootNormal::setup function with no issues.
Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
* Simplify JSON lookups
* Remove unnecessary lookups
* Add null checks for JSON strings before calling strlen()
mkfrey and others added 7 commits December 19, 2019 09:17
Only copy the topic on the first message chunk. Otherwise the old copy will be
destroyed on subsequent calls to `_onMqttMessage()` for the same message, while
the copy is still being referenced by `_mqttTopicLevels`. This leads to undefined
behaviour.
@kleini
Copy link
Collaborator

kleini commented Dec 27, 2019

@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 stritti closed this Jan 3, 2020
@stritti
Copy link
Collaborator

stritti commented Jan 3, 2020

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

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.