Skip to content

Add SUCCESS severity level between INFO and WARN#573

Closed
sutharsan-311 wants to merge 1 commit into
ros2:rollingfrom
sutharsan-311:feature/add-success-log-level
Closed

Add SUCCESS severity level between INFO and WARN#573
sutharsan-311 wants to merge 1 commit into
ros2:rollingfrom
sutharsan-311:feature/add-success-log-level

Conversation

@sutharsan-311

Copy link
Copy Markdown

Description

This adds a SUCCESS severity level (value 26) between INFO (20) and WARN (30).

In robotics systems, logs tend to have a lot of INFO noise: velocity publishing, costmap updates, parameter reads. When a meaningful operation completes (navigation goal reached, sensor initialized, hardware ready), there is no way to distinguish that from general info without parsing message strings.

SUCCESS fills that gap. It means "a task completed correctly", which is different from "here is some information".

Changes:

  • RCUTILS_LOG_SEVERITY_SUCCESS = 26 added to the severity enum in logging.h
  • "SUCCESS" entry added to the severity names array in logging.c
  • Full RCUTILS_LOG_SUCCESS_* macro family added to logging_macros.h, matching the existing pattern for INFO/WARN/ERROR
  • RCUTILS_LOG_MIN_SEVERITY_SUCCESS = 2 added for compile-time filtering

Value 26 satisfies the existing LSB=0 constraint noted above the enum. No existing values are modified.

Tools that do not handle severity 26 will degrade gracefully. The MIN_SEVERITY constants shift by one (WARN: 2->3, ERROR: 3->4, FATAL: 4->5, NONE: 5->6). Code using named defines is unaffected.

Happy to close this if the preference is to keep severity levels fixed.

Is this user-facing behavior change?

Yes. Users can use RCUTILS_LOG_SUCCESS(...) and related macros.

Did you use Generative AI?

Yes, for assistance with research and drafting. The implementation was reviewed and understood before submission.

Additional Information

Related: #526 (color formatting for existing levels, no conflict)

@sutharsan-311 sutharsan-311 force-pushed the feature/add-success-log-level branch from f9dce96 to 4e4183b Compare June 4, 2026 11:45

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks for creating issue and discussion.

but, IMO, this PR does not make sense at all.

severity is a monotonic scale of urgency or badness, DEBUG < INFO < WARN < ERROR < FATAL. "SUCCESS" isn't a point on that axis at all. the result from API/interface already tells us, so a SUCCESS level is redundant; for observability, INFO already covers it; and either way, "succeeded" isn't a severity in semantic.

#define RCUTILS_LOG_MIN_SEVERITY_ERROR 3
#define RCUTILS_LOG_MIN_SEVERITY_FATAL 4
#define RCUTILS_LOG_MIN_SEVERITY_NONE 5
#define RCUTILS_LOG_MIN_SEVERITY_SUCCESS 2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I will not do this because those are a packed sequential index (DEBUG=0, INFO=1, WARN=2, …), not the enum, and they're compile-time definitions used for compile-time log stripping. inserting SUCCESS renumbers WARN 2→3, ERROR 3→4, etc. that's a silent break across translation units or packages built at different times which is arguably worse than a clean ABI break because it fails quietly.

@sutharsan-311

Copy link
Copy Markdown
Author

Makes sense, thanks for explaining the reasoning. Closing this.

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.

2 participants