Skip to content

Conversation

@craiglink
Copy link
Contributor

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

@marcelstoer
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

@ropg
Copy link
Contributor

ropg commented May 7, 2024

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.

@marcelstoer marcelstoer merged commit 8c03ef1 into ThingPulse:master May 8, 2024
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

Successfully merging this pull request may close these issues.

3 participants