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

rclcpp "throttle" logging #797

Closed
IanTheEngineer opened this issue Jul 26, 2019 · 2 comments · Fixed by #879
Closed

rclcpp "throttle" logging #797

IanTheEngineer opened this issue Jul 26, 2019 · 2 comments · Fixed by #879
Assignees
Labels
enhancement New feature or request

Comments

@IanTheEngineer
Copy link

IanTheEngineer commented Jul 26, 2019

Feature request:

rclcpp logging client library support for throttle loggers.

I found a TODO in the source code, and figured it deserved to be called out in an issue for visibility:

# TODO(dhood): Implement the throttle macro using time sources available in rclcpp
excluded_features = ['named', 'throttle']
def is_supported_feature_combination(feature_combination):
    is_excluded = any([ef in feature_combination for ef in excluded_features])
    return not is_excluded
}@

This is also sort-of captured in a bullet point tucked away in a nearly-completed ticket in the ros2 repo: ros2/ros2#425

A description of the desired functionality can be found in the ROS1 wiki:

Throttle Logger

Throttle

    ROS_DEBUG_THROTTLE(period, ...)

    ROS_DEBUG_STREAM_THROTTLE(period, args)

    ROS_DEBUG_THROTTLE_NAMED(period, name, ...)

    ROS_DEBUG_STREAM_THROTTLE_NAMED(period, name, args) 

Throttled output will print a message at most once per "period".

Delayed throttle

    ROS_DEBUG_DELAYED_THROTTLE(period, ...)

    ROS_DEBUG_STREAM_DELAYED_THROTTLE(period, args)

    ROS_DEBUG_DELAYED_THROTTLE_NAMED(period, name, ...)

    ROS_DEBUG_STREAM_DELAYED_THROTTLE_NAMED(period, name, args) 

Delayed throttled output will print a message at most once per "period" and no message will be printed before one "period" has elapsed.

@BMarchi
Copy link
Contributor

BMarchi commented Sep 30, 2019

If we remove the throttle key from excluded_features, rclcpp macros are generated which use the rcutils implementation. We can see, from the generated files, the following:
#define RCUTILS_LOG_DEBUG_THROTTLE(time_source_type, duration, ...) . rcutils doesn't have the concept of a time source or a clock. It uses the system time clock for the throttling implementation.

I came up with the following ideas:

  1. Forget about the time source concept in rcutils and expect a time point instead (expressed in nanoseconds) and use that in all the throttle macros. We would need to force the user to pass a rcl_clock_t to the RCLCPP throttle macro definition and get a time point from that, or just pass the nanoseconds to it.
  2. Create a definition for a clock in rcutils that should be also used in rcl. This implies to change the API, so we can pass a clock to the macro.
  3. There's a comment saying that throttle should be enabled using the rclcpp time source, but I guess another option is to just use the system's clock and nothing else.

I personally think the first option is easier to implement and less error prone than the second one.
What do you think about it? @dirk-thomas @hidmic

@dirk-thomas
Copy link
Member

How about option 4: passing a function pointer with the same signature as rcutils_steady_time_now for the existing argument time_source_type? From rclcpp such a function can be created which wraps the C++ clock / time source calls.

nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
rcl_clock_t can't be trivially copied because adding or removing clock
jump callbacks to a copy will free the pointer held by the original
instance.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this issue Aug 5, 2022
ros2#797)

* Add spin_and_wait_for_matched to PublicationManager and update test codes

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Address review comments

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Wait without spin

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Remove unused codes and adjust default sleep time

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Fix wrong description of return value

Signed-off-by: Barry Xu <barry.xu@sony.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants