-
Notifications
You must be signed in to change notification settings - Fork 418
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
Asign node path name and time stamp to parameter event message #584
Conversation
This PR together with rcl_interfaces PR 51 works like a charm for my use-case (rclcpp-based monitoring of the system-wide configuration / node parameters). |
Generally this looks good. There appears to be a conflict with the recently added waitable interfaces. If you have a moment to check that it would be great. Then I can run CI on this and the python version. |
modify adding clock for rclcpp_lifestyle
ce040e9
to
45201e6
Compare
@tfoote I fixed the conflicts. It builds fine on my machine. |
node_graph_, | ||
node_services_, | ||
node_logging_ | ||
)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has trailing white spaces. @tfoote Please run the unit tests before merging PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 33f1e17.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dirk-thomas I'm not sure how that passed. I ran CI here: ros2/rcl_interfaces#51 (comment)
modify adding clock for rclcpp_lifestyle
Connects to ros2/rcl_interfaces#51
This PR follows my pull request in rcl_interfaces: ros2/rcl_interfaces#51 which adds a node name field and timestamp field to the ParameterEvents.msg. This pull request fills those fields.