ros2 / ros2/rclcpp

Make throttle log macros accept a rclcpp::Duration

Open
#1,929 5 comments 9 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

help wanted
Dominant language
C++
Stars
805
Forks
564
Avg merge
1d 17h
Merged PRs (30d)
27

Description

Feature request

Feature description

Currently RCLCPP_*_THROTTLE (and the RCUTILS_LOG_*_THROTTLE_NAMED which they cal) arbitrarily accept the duration as an integral value of milliseconds. This is highly confusing, especially since it is not documented anywhere but deep in the source, in RCUTILS_LOG_CONDITION_THROTTLE_BEFORE, where it calls:

static rcutils_duration_value_t __rcutils_logging_duration = RCUTILS_MS_TO_NS((rcutils_duration_value_t)duration); 

If it accepts a simple integral value, it at least should be in nanoseconds. Better yet, it would be nice to change the rclcpp side of the macro to accept an rclcpp::Duration, which would be very uniform, and then it can be converted to milliseconds to satisfy RCUTILS strange interface.

Implementation considerations

Being a macro, I'm not sure how to define this without breaking backward compatibility. However, that brings up the point: why do the rclcpp logging utils need to be macros? We could define a variadic function instead that wraps the RCUTILS C macros, right?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the RCLCPP_THROTTLE macros and the RCUTILS_LOG_THROTTLE_NAMED macros, then inspect RCUTILS_LOG_CONDITION_THROTTLE_BEFORE where the duration is converted. Determine the backward-compatibility implications of accepting rclcpp::Duration and whether replacing the rclcpp macros with a variadic function is viable. Done means the API behavior and compatibility path are clearly established.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
observability
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.