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

logging: log: add LOG_WRN_ONCE #70282

Merged

Conversation

JordanYates
Copy link
Collaborator

Add an equivalent to the linux WARN_ONCE macro. This is intended for
use when the developer should be notified of an event, but may occur a
multitude of times in quick succession.

Using LOG_WRN could result in either cluttered logs or recursive
logging (i.e. in serial drivers).

Proposed in the discussion of #65112

Jordan Yates added 3 commits March 15, 2024 21:26
Add an equivalent to the linux `WARN_ONCE` macro. This is intended for
use when the developer should be notified of an event, but may occur a
multitude of times in quick succession.

Using `LOG_WRN` could result in either cluttered logs or recursive
logging (i.e. in serial drivers).

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Add a test for the `LOG_WRN_ONCE` wrapper around `LOG_WRN`.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Document the existence of `LOG_WRN_ONCE`.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I tossed around a similar rate-limit debug variant at one point.

@nordic-krch
Copy link
Contributor

Long time ago there was #41783 but author dropped it and it never get in. There we had LOG_<lvl>_N(n, ...) which was more flexible and supported all severity levels maybe it would be good to resurrect that approach instead?

@JordanYates
Copy link
Collaborator Author

Long time ago there was #41783 but author dropped it and it never get in. There we had LOG_<lvl>_N(n, ...) which was more flexible and supported all severity levels maybe it would be good to resurrect that approach instead?

I don't mind if that is the approach you want to go with, but I won't be resurrecting it.

IMO only LOG_LEVEL_WRN really makes sense for this macro anyway. DBG should always print, ERR is fatal, so likely won't happen again anyway, and INF is normal operation so shouldn't be spamming the logs in any case. WRN is uniquely "user should be notified, the event is unusual, but because its only a warning, may occur many times in quick succession".

@fabiobaltieri fabiobaltieri merged commit ca36fee into zephyrproject-rtos:main Mar 21, 2024
22 checks passed
@JordanYates JordanYates deleted the 240315_log_wrn_once branch March 21, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants