Skip to content

Add node path and timestamp fields to event msg#51

Merged
tfoote merged 2 commits intoros2:masterfrom
bpwilcox:source_node_events
Dec 4, 2018
Merged

Add node path and timestamp fields to event msg#51
tfoote merged 2 commits intoros2:masterfrom
bpwilcox:source_node_events

Conversation

@bpwilcox
Copy link
Copy Markdown
Contributor

Per the discussion in #27, this PR adds a timestamp and node path name field to the ParameterEvent.msg. This would allow for parameter events to be resolved by their source node.

Copy link
Copy Markdown
Contributor

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

This looks good to me. We should pair it with updates that fill these fields when published.

Update: Sorry I missed the related PR added "Connects to" at the top of the description to group it on Waffle.io. We still need a rclpy update too.

@bpwilcox
Copy link
Copy Markdown
Contributor Author

@tfoote I've just submitted a PR to add the update to ros2/rclpy to fill the ParameterEvent.msg. Can you update us on the status of this review and whether we could get these PRs merged for the Crystal release? A few of our issues in the navigation2 stack are held up on workarounds that this PR would resolve.

@norro
Copy link
Copy Markdown

norro commented Nov 30, 2018

This PR together with rclcpp PR 584 works like a charm for my use-case (rclcpp-based monitoring of the system-wide configuration / node parameters).

Copy link
Copy Markdown
Contributor

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

This looks good. It currently has @bpwilcox I rebased this on the upstream changes that already inserted the dependency, please check the merge is valid.

@bpwilcox
Copy link
Copy Markdown
Contributor Author

bpwilcox commented Dec 3, 2018

The merge looks valid to me.

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Dec 4, 2018

CI with all three repos rclcpp, rclpy and rcl_interfaces

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@tfoote tfoote merged commit b0bc045 into ros2:master Dec 4, 2018
@tfoote tfoote removed the in review Waiting for review (Kanban column) label Dec 4, 2018
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.

3 participants