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
1.98k stars 638 forks source link

More printing fixes #399

Closed ropg closed 3 months ago

ropg commented 3 months ago
marcelstoer commented 3 months ago

It now does not take arguments anymore and does things internally

The the fewer the parameters, the better 👍

However, breaking public API is not something that we take lightly. We could keep the old function in the API for the time being. It could print a deprecation message to serial and delegate to the new function?

ropg commented 3 months ago

I'm not so sure I can think of any scenario where the user would want to interact with either drawLogBuffer or setLogBuffer. Maybe we should rename them to something that's private or protected and print deprecation msgs for both?

ropg commented 3 months ago

"deprecated: print functionality now handles buffer management automatically" ?

marcelstoer commented 3 months ago

Whatever we do, we can't remove functions from public API without a major version bump. Hence, I suggest something like

bool OLEDDisplay::setLogBuffer(uint16_t lines, uint16_t chars) {
  Serial.println("[deprecated] Print functionality now handles buffer management automatically. This is a no-op.");
}

setLogBuffer() can then be made private API I guess.

ropg commented 3 months ago

It occurred to me that it's maybe cleaner to have cls() and setFont() remove the buffer and then only the next print action re-set it as needed. That way users have the option to free the memory by just issuing cls(). Will make it all so later today.

marcelstoer commented 3 months ago

This looks sane, thanks a lot.