Skip to content
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

Namespace Log Level Defines #23

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Namespace Log Level Defines #23

wants to merge 8 commits into from

Conversation

thorrak
Copy link

@thorrak thorrak commented Jan 11, 2022

I'm using Arduino Log in a project with other libraries that already have defines set up for LOG_LEVEL_ERROR amongst other things. This causes a number of warnings at compile time (and some strange behavior in the logs).

This PR renames the #defines to prepend ARDUINO_LOG_ thereby effectively namespacing them.

@joscha
Copy link
Collaborator

joscha commented Jan 11, 2022

This renaming game is kind of open ended if we assume that each library is compatible with all other libraries at all time.
Whilst I agree that the currently chosen name is quite wide, this change is also backwards incompatible.
Is there a better way to rename these during a preprocessing phase in your project potentially?
Or can we change the pull request so you can specify a prefix for these and leave it empty by default to ensure backwards compatibility?

@thorrak
Copy link
Author

thorrak commented Jan 11, 2022

Can you specify a prefix for preprocessor items?

@joscha
Copy link
Collaborator

joscha commented Jan 11, 2022 via email

ldenisey and others added 7 commits January 15, 2023 15:31
CR and LF definition were inverted
Removed "register" as it is obsolete after C++11
Bring up to date with upstream
Merge Beirdo's change pending upstream
@mattbaker-digital
Copy link

mattbaker-digital commented Feb 21, 2024

Same for CR LF etc. They are far too generic.

@egnor
Copy link

egnor commented Apr 16, 2024

I quite agree that polluting the main namespace is annoying, but ARDUINO_LOG_LOG_LEVEL_SILENT and such are long, cumbersome and redundant?

ARDUINO_LOG_SILENT etc would be fine, OR just change them to be constexpr definitions instead of macros so they can go into a C++ namespace, which can then be pulled into the global namespace by default, but if you define -DARDUINO_LOG_NAMESPACE or something like that it will avoid doing so, thus allowing backward compatibility by default but new code can be more specific.

(Even then, arduino_log::Log.notice(...) would be a lot. Better would be something like alog::notice(...)?)

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.

6 participants