Skip to content

[deepSleep] Make deepSleep work again with 2.5.0 and refactor delay var #2217

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

Merged
merged 5 commits into from
Jan 7, 2019
Merged

[deepSleep] Make deepSleep work again with 2.5.0 and refactor delay var #2217

merged 5 commits into from
Jan 7, 2019

Conversation

clumsy-stefan
Copy link
Contributor

Make deepSleep work again with Core 2.5.0. Changed ESP.deepSleep() to ESP.deepSleepInstant().

Refactor delay variable to dsdelay to avoid conflicts with the delay() function.

Added a delay(100) before going to sleep to give the node time to send the last log messages.

Make deepSleep work again with Core 2.5.0. Changed ESP.deepSleep() to ESP.deepSleepInstant().

Refactor delay variable to dsdelay to avoid conflicts with the delay() function.

Added a delay(100) before going to sleep to give the node time to send the last log messages.
added #ifdefs to make build happy also for versions <2.5.0
@TD-er
Copy link
Member

TD-er commented Jan 6, 2019

Maybe you can also add a check for the deepSleepMax() ?
See ThingPulse - Max deep sleep for ESP8266 and these discussions:

Then we can also try to extend the max. deepsleep in ESPeasy's settings to allow longer deepsleep.

N.B. in the code we have some kind of flush built in before entering deep sleep:

ESPEasy/src/ESPEasy.ino

Lines 547 to 560 in 4d2a76a

if (readyForSleep()){
if (Settings.UseRules)
{
String event = F("System#Sleep");
rulesProcessing(event);
}
// Flush outstanding MQTT messages
runPeriodicalMQTT();
flushAndDisconnectAllClients();
deepSleep(Settings.Delay);
//deepsleep will never return, its a special kind of reboot
}
}

@clumsy-stefan
Copy link
Contributor Author

clumsy-stefan commented Jan 6, 2019

I'll have a look at the deepSleepMax() value/settings...

The flush does not seem to work for the log which is on line 177
addLog(LOG_LEVEL_INFO, F("SLEEP: Powering down to deepsleep..."));
in syslog it shows up only after I inserted the delay() after that line.

@TD-er
Copy link
Member

TD-er commented Jan 6, 2019

OK, so maybe we should "fix" that in flushAndDisconnectAllClients() ?
Do you read the log from serial? That is now buffered, so we may also check for the buffer of the serial output and the HW buffer.
No need to keep waiting for worst-case scenario, when we can determine when it is done.

@TD-er TD-er added Category: Stabiliy Things that work, but not as long as desired Category: Energy Related to reduce power as much as possible. labels Jan 6, 2019
Allow to specify sleep times up deepSleepMax() on core 2.5.0
@TD-er
Copy link
Member

TD-er commented Jan 6, 2019

Hmm, the core 2.5.0 builds fail with this:

/home/travis/build/letscontrolit/ESPEasy/src/Misc.ino: In function 'void deepSleepStart(int)':
/home/travis/build/letscontrolit/ESPEasy/src/Misc.ino:397:38: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
if (dsdelay > ESP.deepSleepMax() || dsdelay < 0)
^
Compiling .pioenvs/hard_core_250_beta_Shelly_1_2M256/FrameworkArduino/FS.cpp.o
cc1plus: some warnings being treated as errors
*** [.pioenvs/hard_core_250_beta_Shelly_1_2M256/src/ESPEasy.ino.cpp.o] Error 1

It's a bit harsh, but in this case I can understand it, since the resulting value of uint64_t deepSleepMax() (and it will overflow 32-bit unsigned int)

@clumsy-stefan
Copy link
Contributor Author

Of course . Right, sorry...

It was too late I guess,, I did not yet review it t thoroughly... Sorry for that..

Will amend it tomorrow!

Fix type errors of #999e43a and (hopefully) cast all types correctly now....

Keep deep sleep delay below INT_MAX for parameter storage compatibility.
@clumsy-stefan
Copy link
Contributor Author

@TD-er wrote:

OK, so maybe we should "fix" that in flushAndDisconnectAllClients() ?
Do you read the log from serial? That is now buffered, so we may also check for the buffer of the serial output and the HW buffer.
No need to keep waiting for worst-case scenario, when we can determine when it is done.

Basically I agree, but when I understand the code correctly, the flushAndDisconnectAllClients() is not called anymore once the deepSleepStart()function is called. It goes immediately to sleep after the addLog() in there.

So basically if you want to have that in flushAndDisconnectAllClients() you would need to call it instead of just a delay(100) and I'm not sure if it's guaranteed that this function returns in a reasonable time, when it tries to flush everything...

@TD-er
Copy link
Member

TD-er commented Jan 7, 2019

So basically if you want to have that in flushAndDisconnectAllClients() you would need to call it instead of just a delay(100) and I'm not sure if it's guaranteed that this function returns in a reasonable time, when it tries to flush everything...

Then we should include a timeout there.
Also this function is one of the few actually allowed to run as many delay calls as needed to finish the job.
My point is, it is better to get stuff flushed and shutdown early as possible, instead of waiting N msec to go to sleep even if it isn't needed.

@clumsy-stefan
Copy link
Contributor Author

My point is, it is better to get stuff flushed and shutdown early as possible, instead of waiting N msec to go to sleep even if it isn't needed.

Sure I think it depends on the point of view... If you want to have deepSleep units because they run on battery and save energy you probably don't want to wait too long before shutdown, even if not all logs are done or all clients disconnected. On the other habnd you want to make sure, that at least the values are sent to the controller... seems to be kind of a trade-off here...

The 'old' limit of 4294 seconds was 2^32 usec.
By casting it to an uint32, you may get unexpected results when multiplying it by 10^6 again.
So better to use uint64 as variable and work on that one, now the deepsleep functions support uint64.
@TD-er
Copy link
Member

TD-er commented Jan 7, 2019

I just pushed a new commit to it.
Maybe you can test it?

About the 'trade-off', I am not saying we shouldn't wait. Only try to wait as much as needed to get all buffers flushed and no longer.

@clumsy-stefan
Copy link
Contributor Author

I just pushed a new commit to it.
Maybe you can test it?

sure, thanks!! I'm jsut compiling and installing now.. will report if it works..

looks more or less the same for me, done a bit differently... except that you allocate a uint64_t and multiplying the int instead of allocating "only" an ìntand dividing the uint64_t`into it... thaugt it takes less memory the other way round...

the adding of the max possible value to the web interface I also changed but did not yet commit/push it ;) same thought though...

@TD-er
Copy link
Member

TD-er commented Jan 7, 2019

The memory usage isn't an issue here, since it is just very local and on the stack too. (allocation on the heap takes time, on the stack doesn't)

The main issue why I changed it to do all on the 64-bit value was because of this:

ESP.deepSleepInstant((uint32_t)dsdelay * 1000000, WAKE_RF_DEFAULT);

In this call, any value of dsdelay of over 4294 would result in something unexpected, since it would overflow. (and you would not notice it)
So when working on a 64 bit int, you know for sure it will not overflow inbetween.

@clumsy-stefan
Copy link
Contributor Author

great!

up to now it seems to run without issues...at least this part...

@TD-er
Copy link
Member

TD-er commented Jan 7, 2019

OK, so I can merge it?

@clumsy-stefan
Copy link
Contributor Author

from my point of view yes, It compiles also in the Arduino framework and runs on my nodes for about 2 hours now, including deepSleep nodes..

@TD-er TD-er merged commit 91a24fd into letscontrolit:mega Jan 7, 2019
@TD-er
Copy link
Member

TD-er commented Jan 7, 2019

@Barracuda09 I was almost certain I did change that one too, but apparently not.
Also I agree with your remarks about adding curly braces to indicate the scope.
Usually I try to change those when I see them.

@pieman64
Copy link

@TD-er I understand the 32 bit overflow of microseconds that gives uint32_t a maximum of 4294 seconds (or approx 71 minutes). Previously I was wrongly thinking we were able to go to a 64 bit value but I guess it's actually just 33 bits. Does this mean the new maximum sleep is around 8590 seconds (approx 143 minutes)? I am using the new code set at 18000 seconds (5 hours) with the expectation of the ESP8266 waking after 143 minutes. Two hours to wait before I have the results.

@TD-er
Copy link
Member

TD-er commented Jan 14, 2019

@pieman64 See the links I posted in this reply: #2217 (comment)

@pieman64
Copy link

Thanks @TD-er
From https://thingpulse.com/max-deep-sleep-for-esp8266/ it states:
13612089337μs ≙ 13612s ≙ 3.78h ≙ 3h 46min
That rings true with the 3 to 4 hours I had in mind some months ago when I looked into it.

@pieman64
Copy link

pieman64 commented Jan 14, 2019

I got 3 hours 33 minutes from my first run as shown from the blurb below:

/*
14:20:30.585 ->     ___  __          __
14:20:30.632 ->    / _ )/ /_ _____  / /__
14:20:30.632 ->   / _  / / // / _ \/  '_/
14:20:30.678 ->  /____/_/\_, /_//_/_/\_\
14:20:30.725 ->         /___/ v0.5.4 on Arduino
14:20:30.772 -> 
14:20:32.162 -> [1706] Connecting to mysecretserver.com:80
14:20:32.407 -> [1928] Ready (ping: 83ms).
14:20:33.588 -> Uptime: 3s
14:20:34.476 -> Sleep
14:20:35.034 -> Long sleep                       ****** 3hours 33 minutes
17:53:12.153 ->     ___  __          __
17:53:12.187 ->    / _ )/ /_ _____  / /__
17:53:12.187 ->   / _  / / // / _ \/  '_/
17:53:12.223 ->  /____/_/\_, /_//_/_/\_\
17:53:12.257 ->         /___/ v0.5.4 on Arduino
17:53:12.291 -> 
17:53:13.727 -> [1708] Connecting to mysecretserver.com:80
17:53:14.135 -> [2091] Ready (ping: 83ms).
17:53:15.315 -> Uptime: 3s
17:53:16.035 -> Sleep
17:53:16.586 -> Long sleep

*/

#define ESP8266
#define CORE_2_5_0 

void sleep()
{
    //ESP.deepSleep(60000000 * 15 * (900 / 872)); // old format 15 minutes sleep 900 / 872 is tolerance 
    // details below taken from: https://github.com/clumsy-stefan/ESPEasy/blob/cf95d0ca37726f1c3e188987960ac99b08a5d7f0/src/Misc.ino
    // 1 million microseconds in a second
    unsigned long dsdelay = 18000; // 4294 seconds max approx 71 minutes before core 2.5.0
                                   // try 5 hours (18000 seconds) for core 2.5.0
    // Max is now approx 13612089337μs ≙ 13612s ≙ 3.78h ≙ 3h 46min from https://thingpulse.com/max-deep-sleep-for-esp8266/                               
    myTerminal.println("Sleep");
    myConsole.println("Sleep");
    delay(500);                   // give the node time before going to sleep
    display.clearDisplay();
    display.display();     
    #if defined(ESP8266)
      #if defined(CORE_2_5_0)
        uint64_t deepSleep_usec = dsdelay * 1000000ULL;    // 4294 seconds * 1000000
        Serial.println(F("Long sleep"));         // debugging
        if ((deepSleep_usec > ESP.deepSleepMax()) || dsdelay < 0) {
          deepSleep_usec = ESP.deepSleepMax();
        }
        ESP.deepSleepInstant(deepSleep_usec, WAKE_RF_DEFAULT);
      #else
        if (dsdelay > 4294 || dsdelay < 0)
          dsdelay = 4294;   //max sleep time ~71 minutes
          Serial.println(F("Short sleep"));         // debugging
        ESP.deepSleep((uint32_t)dsdelay * 1000000, WAKE_RF_DEFAULT);
      #endif
    #endif    
}

@pieman64
Copy link

The second cycle was broadly similar to the first but slightly shorter at about 3.5 hours. Unfortunately the third cycle failed and after 7 hours the ESP8266 was still sleeping.

I have dropped the 18,000 seconds to 12,000, which in theory is 3 hours 20 minutes. Due to the poor time keeping of these devices the first cycle with these settings was timed at 3 hours and 8 minutes. The second cycle was 3 hours and 14 minutes. It has just started the third cycle and I will see if it wakes this time.

@TD-er
Copy link
Member

TD-er commented Jan 15, 2019

The internal clock is not that accurate.
It depends on a number of factors like temperature and more likely with these cheap modules, the quality of the crystal.

@fluppie
Copy link

fluppie commented Jan 18, 2019

Maybe we can extend the time by working with loops like the guys from LowPowerLab do?
See send_loops code in https://github.com/LowPowerLab/RFM69/blob/master/Examples/WeatherNode/WeatherNode.ino

@clumsy-stefan clumsy-stefan deleted the deepSleep branch January 21, 2019 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Energy Related to reduce power as much as possible. Category: Stabiliy Things that work, but not as long as desired
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants