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

WiFi fpm sleep type checks in mode() #7975

Open
6 tasks done
mcspr opened this issue Apr 12, 2021 · 4 comments
Open
6 tasks done

WiFi fpm sleep type checks in mode() #7975

mcspr opened this issue Apr 12, 2021 · 4 comments

Comments

@mcspr
Copy link
Collaborator

mcspr commented Apr 12, 2021

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Problem Description

After #7902

if (m != WIFI_OFF && wifi_fpm_get_sleep_type() != NONE_SLEEP_T) {
// wifi starts asleep by default
wifi_fpm_do_wakeup();
wifi_fpm_close();
}

In case enableWiFiAtBootTime(); is used, wifi_fpm_get_sleep_type() != NONE_SLEEP_T is always true b/c SDK default is MODEM and it seems to be a setting for the wifi_fpm_open(), not the actual active mode check. Should it be set by the opposite __disableWiFiAtBootTime() to NONE since a lot of functions seem to skip WiFi.forceSleepBegin/Wake and directly use SDK API?

MCVE Sketch

#include <Arduino.h>
#include <ESP8266WiFi.h>

void setup() {
    Serial.begin(115200);
#if 0
    enableWifiAtBootTime();
#endif
    Serial.println(wifi_fpm_get_sleep_type());
}

void loop() {
}

Debug Messages

#if 0 & #if 1

2
@dok-net
Copy link
Contributor

dok-net commented Apr 16, 2021

@mcspr I am addressing things like this in PR #7979. Help with review and testing is appreciated. @Tech-TX is extremely helpful with testing, the actual measurable effects of the API additions should be documented, please help with that.

@dok-net
Copy link
Contributor

dok-net commented Apr 16, 2021

@mcspr Perhaps it's a misunderstanding as to what NONE really means in NONE_SLEEP_T:

Should it be set by the opposite __disableWiFiAtBootTime() to NONE

I understand that NONE means no sleep mode at all, not no Wifi.

You are right that the code is in violation of Espressif documentation:

This API can only be called when MODEM_SLEEP_T force sleep function is enabled

and should be fixed like so

--- a/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp
+++ b/libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp
@@ -426,9 +426,10 @@ bool ESP8266WiFiGenericClass::mode(WiFiMode_t m, WiFiState* state) {
         return true;
     }
 
-    if (m != WIFI_OFF && wifi_fpm_get_sleep_type() != NONE_SLEEP_T) {
+    const sleep_type_t sleepType = wifi_fpm_get_sleep_type();
+    if (m != WIFI_OFF && sleepType != NONE_SLEEP_T) {
         // wifi starts asleep by default
-        wifi_fpm_do_wakeup();
+        if (sleepType == MODEM_SLEEP_T) wifi_fpm_do_wakeup();
         wifi_fpm_close();
     }
 
@@ -550,8 +551,9 @@ bool ESP8266WiFiGenericClass::forceSleepBegin(uint32 sleepUs) {
  * @return ok
  */
 bool ESP8266WiFiGenericClass::forceSleepWake() {
-    if (wifi_fpm_get_sleep_type() != NONE_SLEEP_T) {
-        wifi_fpm_do_wakeup();
+    const sleep_type_t sleepType = wifi_fpm_get_sleep_type();
+    if (sleepType != NONE_SLEEP_T) {
+        if (sleepType == MODEM_SLEEP_T) wifi_fpm_do_wakeup();
         wifi_fpm_close();
     }
 
@@ -789,8 +791,9 @@ bool ESP8266WiFiGenericClass::shutdown (uint32 sleepUs, WiFiState* state)
 
 bool ESP8266WiFiGenericClass::resumeFromShutdown (WiFiState* state)
 {
-    if (wifi_fpm_get_sleep_type() != NONE_SLEEP_T) {
-        wifi_fpm_do_wakeup();
+    const sleep_type_t sleepType = wifi_fpm_get_sleep_type();
+    if (sleepType != NONE_SLEEP_T) {
+        if (sleepType == MODEM_SLEEP_T) wifi_fpm_do_wakeup();
         wifi_fpm_close();
     }
 

@dok-net
Copy link
Contributor

dok-net commented Apr 16, 2021

PR #7979 would gladly accept control of the sleep code at that level, and add a cached flag for auto vs. forced sleep modes. Currently without that, it unfortunately has no way of discovering which of auto or forced is in effect when the newly added sleep functions are called, which makes full switching back impossible and always results in auto sleep mode due to

wifi_fpm_close();

when turning off its various new sleep modes.

@dok-net
Copy link
Contributor

dok-net commented Apr 16, 2021

Changes suggested above are now incorporated in #7979, plus ESPWiFiGeneric.cpp has been refactored to use the new ESP class-based sleep functions.
@mcspr Please consider if you can close this issue?

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

No branches or pull requests

2 participants