Make throttle log macros accept a rclcpp::Duration
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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