ThingPulse / esp8266-oled-ssd1306

Driver for the SSD1306 and SH1106 based 128x64, 128x32, 64x48 pixel OLED display running on ESP8266/ESP32
https://thingpulse.com
Other
2k stars 638 forks source link

Add support for resuming from deep sleep #259

Closed ThorstenBr closed 4 years ago

ThorstenBr commented 4 years ago

I've updated @ccantill's patch to match the latest head revision. Actual patch content is otherwise identical to ccantil's patch #215 from about a year ago. Any chance to get this included into master? I'd also like to separate the module's init code from the hardware device's init code, to enable smooth display updates across deep sleeps. It's actually a fairly trivial change... ;-)

ccantill commented 4 years ago

I agree something like allocate would make more sense here. I picked resume because it made the most sense from a user perspective, but it does indeed make the code harder to understand.

helmut64 commented 4 years ago

Just a little note on deep sleep, most MCUs I know (all mbed.os MCUs, Atmel, MIPS) preserve the entire memory on deepsleep, and they continue on wakeup with the MCU, flash and memory normally. For this MCUs there is no need to develop anything extra as they just continue with the wakeup.

Only the ESP/ESP32 offers a shutdown mode (which they call irritating deep sleep). In the shutdown mode the MCU basically is doing a cold boot and everything is lost. The ESP RTC memory offers an external storage to keep variables wish can be accessed after the boot from suspend (deep sleep).

Usually a program goes into deep sleep, wakes up periodically on timer or GPIO and it decides if a screen update needs to be done. Does it makes sense to allocate the the SSD1303 Class just when something needs to be drawn. e.g. “SSD1306Wire *display = new SSD1306Wire(0x3c, SDA, SCL)”

This would allow to do it smart without any overhead and init the OLED lib only when an update is needed.

ccantill commented 4 years ago

Does it makes sense to allocate the the SSD1303 Class just when something needs to be drawn. e.g. “SSD1306Wire *display = new SSD1306Wire(0x3c, SDA, SCL)”

Sure, but I don't see how this changes the original problem which is that the init call also clears the display resulting in a (briefly) blank screen between initialization and updating.

helmut64 commented 4 years ago

The init clearing the display should be no problem because in the same millisecond you do a redraw of the new content. On display.display(); the entire new content (frame buffer) gets drawn to the OLED. PS: You don't call the init, neither the class constructor. See my new SSD1306Wire above.

ThorstenBr commented 4 years ago

With the Arduino framework, people like to get all "init" code done within the setup hook - while screen updates are usually done at run-time within "loop" only. Real-time people also like to separate all one-time "init" stuff from the run-time code, to reduce any "hickups" while looping. Sure, it's not proper real-time and it may not be a big deal in this case...

Another suggestion: how about changing the API, avoiding the extra "resume" method, and just add an optional parameter to "init" instead. With the default value for the parameter, existing sources do not need to be changed. The API would remain compatible - while providing the flexibility to just skip the display reset.

Something like:

bool OLEDDisplay::init(bool resetDisplay=true) { ... malloc stuff ... if (resetDisplay) { sendInitCommands(); ... } }

helmut64 commented 4 years ago

Arduino ist a standard C/C++ environment pretty much like Mbed-os, Keil and others. Yes Arduino has also little libraries like Strings, Hardware API, etc., however the huge part is the Compiler and Runtime environment supplied with the C/C++ standard. The init can done in the setup anyways, however only if the display is needed.

Every little String operation is allocating and release memory, it makes no difference if the class constructor runs in setup or in loop. For a deep sleep case it is more likely that no loop runs at all, the system wakes up, checks something and enters deep sleep again to save energy.

Anyways my opinion is that the deep sleep use case can be easily implemented without a SSD1306 library update.

PS: For my deep sleep use case it turn off the display which brings the display down to 1µA.

ccantill commented 4 years ago

The API would remain compatible - while providing the flexibility to just skip the display reset.

The current code change also retains backwards compatibility; the init function still does all the allocation stuff to get the library working, but now it's also possible to call only the allocation stuff.

The init clearing the display should be no problem because in the same millisecond you do a redraw of the new content.

It is a problem; there is a noticeable flash on the first redraw after a resume from deep sleep if you do the full initialization (which currently includes writing a blank framebuffer to the display).

ThorstenBr commented 4 years ago

The current code change also retains backwards compatibility;

Yes, of course. My suggestion with the other API approach was just meant as an idea, since the result might be a little easier to understand than having separate init/resume methods. I meant the other approach would also be backwards compatible. But I don't really mind which way to go. Either approach would allow me to get rid of my local fork... ;-)

marcelstoer commented 4 years ago

For now I'll be happy if just rename resume to allocateBuffer so I can merge this 😉

Further changes can potentially go into separate PRs.

ThorstenBr commented 4 years ago

Done. :)