Skip to content

Asign node path name and time stamp to parameter event message#584

Merged
tfoote merged 1 commit intoros2:masterfrom
bpwilcox:add_node_info_events
Dec 4, 2018
Merged

Asign node path name and time stamp to parameter event message#584
tfoote merged 1 commit intoros2:masterfrom
bpwilcox:add_node_info_events

Conversation

@bpwilcox
Copy link
Copy Markdown
Contributor

@bpwilcox bpwilcox commented Nov 15, 2018

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.

@norro
Copy link
Copy Markdown

norro commented Nov 30, 2018

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).

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Dec 3, 2018

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
@bpwilcox bpwilcox force-pushed the add_node_info_events branch from ce040e9 to 45201e6 Compare December 3, 2018 21:46
@bpwilcox
Copy link
Copy Markdown
Contributor Author

bpwilcox commented Dec 4, 2018

@tfoote I fixed the conflicts. It builds fine on my machine.

@tfoote tfoote merged commit 9d7b50e into ros2:master Dec 4, 2018
@tfoote tfoote removed the in review Waiting for review (Kanban column) label Dec 4, 2018
node_graph_,
node_services_,
node_logging_
)),
Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed in 33f1e17.

Copy link
Copy Markdown
Contributor

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)

dirk-thomas added a commit that referenced this pull request Dec 5, 2018
cho3 pushed a commit to cho3/rclcpp that referenced this pull request Jun 3, 2019
cho3 pushed a commit to cho3/rclcpp that referenced this pull request Jun 3, 2019
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.

4 participants