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

support NO_GLOBAL_SERIAL #408

Closed craiglink closed 4 months ago

craiglink commented 5 months ago

add preprocessor #ifdefs for NO_GLOBAL_SERIAL which removes dependency to output a deprecated API message

marcelstoer commented 4 months ago

What is the underlying issue here? You don't want to see any console statements or you don't want to see this deprecation warning?

craiglink commented 4 months ago

we'd like to use your library, but we aren't using the Serial library ourselves. Additionally we are controlling when certain global objects from the esp32-arduinio library are instantiated ourselves instead of having them exist globally. That is why these preprocessor defines exist.

NO_GLOBAL_INSTANCES disables a number of global instances of classes across the SDK

NO_GLOBAL_SERIAL, and the various other NO_GLOBAL_xxx disable a specific global instance of a specific class.

We'd like to save the RAM and Flash memory by not having global instance of Serial in our code base by using your library

marcelstoer commented 4 months ago

we are controlling when certain global objects from the esp32-arduinio library are instantiated ourselves instead of having them exist globally.

Ok, I see. Considering that those are the only Serial invocations, I was wondering whether we shouldn't omit them altogether. I can definitely see its advantages, but it also has its price. @ropg, you added them in #399, what is your opinion?

craiglink commented 4 months ago

if you want to remove them all together, that works as well

ropg commented 4 months ago

No strongly held opinion. In general, it might make more sense to have some kind of macro or function to handle debug/error output, and then change what that does, instead of interfering at different places in the code. But given that this is essentially the only place where serial output is produced, it doesn't much matter either way.

One could do #define SERIAL_CONSOLE Serial and then have a debug / error function that only prints anything #ifdef SERIAL_CONSOLE, which would also allow one to set USBSerial or any other Stream instance. And then you could just #undef SERIAL_CONSOLE.