Skip to content

Make RegisterEventHandler describe its sub-entities#386

Merged
ivanpauno merged 2 commits intoros2:masterfrom
pbaughman:issue_385_register_event_handler_describe_entities
Feb 20, 2020
Merged

Make RegisterEventHandler describe its sub-entities#386
ivanpauno merged 2 commits intoros2:masterfrom
pbaughman:issue_385_register_event_handler_describe_entities

Conversation

@pbaughman
Copy link
Copy Markdown
Contributor

@pbaughman pbaughman commented Feb 18, 2020

  • Make the RegisterEventHandler action return a description of its conditional sub-entities
  • Also wire up the correct description method for the launch_testing
    stdout ready listener event handler

This is needed so that launch_testing can find a ReadyToTest action that's conditional on seeing some stdout from one of the processes under test

Signed-off-by: Pete Baughman peter.c.baughman@gmail.com

  • Closes issue 385

@pbaughman
Make RegisterEventHandler describe its sub-entities
edfe5f7 
  - Also wire up the correct description method for the launch_testing
stdout ready listener event handler

Signed-off-by: Pete Baughman <peter.c.baughman@gmail.com>
@pbaughman pbaughman requested a review from wjwwood February 18, 2020 20:45
@pbaughman pbaughman mentioned this pull request Feb 18, 2020
Merged
ivanpauno
ivanpauno approved these changes Feb 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM!
Can you add a test case?

@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Feb 18, 2020

@ivanpauno

Can you add a test case?

Yes! let me put a little thought into it first though.

@pbaughman
Add test cases
4ca68c6 
Signed-off-by: Pete Baughman <peter.c.baughman@gmail.com>
@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Feb 18, 2020

@ivanpauno Ok - I didn't do any assertions about the text part of the description - that's not meant to be a thing you use programmatically. I did check that the actions you give to the constructor come out the other side.

@pbaughman pbaughman requested a review from ivanpauno February 19, 2020 18:38
@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Feb 20, 2020

@ivanpauno poke

@ivanpauno
Copy link
Copy Markdown
Member

ivanpauno commented Feb 20, 2020 •
edited
Loading

CI:

  • Linux Build Status (unrelated failures)

@ivanpauno poke

I was OoO yesterday.

@ivanpauno ivanpauno added the in review Waiting for review (Kanban column) label Feb 20, 2020
@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Feb 20, 2020

@ivanpauno This kind of looks like an issue with CI:

[ERROR] [launch]: Caught exception in launch (see debug for traceback): package 'demo_nodes_py' found at '/home/jenkins-agent/workspace/ci_linux/ws/install/demo_nodes_py', but libexec directory '/home/jenkins-agent/workspace/ci_linux/ws/install/demo_nodes_py/lib/demo_nodes_py' does not exist

Have you seen something like that before?

@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Feb 20, 2020

I get the same errors when I build and test launch_ros master in osrf/ros2:nightly - I think something is broken upstream

[ERROR] [launch]: Caught exception in launch (see debug for traceback): package 'demo_nodes_py' found at '/opt/ros/foxy', but libexec directory '/opt/ros/foxy/lib/demo_nodes_py' does not exist

@pbaughman
Copy link
Copy Markdown
Contributor Author

pbaughman commented Feb 20, 2020

Opened an issue here to try to get this fixed

@ivanpauno ivanpauno merged commit 96191f2 into ros2:master Feb 20, 2020
@ivanpauno
Copy link
Copy Markdown
Member

ivanpauno commented Feb 20, 2020

Thanks for the fix @pbaughman !
The test failures are unrelated, see ros2/build_farmer#266.

@pbaughman pbaughman deleted the issue_385_register_event_handler_describe_entities branch February 20, 2020 16:19
This was referenced Feb 20, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ivanpauno ivanpauno ivanpauno approved these changes

@wjwwood wjwwood Awaiting requested review from wjwwood wjwwood is a code owner

Assignees

No one assigned

Labels

in review Waiting for review (Kanban column)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pbaughman @ivanpauno