Skip to content

Extend CLI daemon's API#493

Merged
hidmic merged 6 commits intomasterfrom
hidmic/extend-daemon-api
Apr 27, 2020
Merged

Extend CLI daemon's API#493
hidmic merged 6 commits intomasterfrom
hidmic/extend-daemon-api

Conversation

@hidmic
Copy link
Copy Markdown

@hidmic hidmic commented Apr 22, 2020

Split from #421. This pull request extends the CLI daemon API to better match that of a regular rclpy.node.Node.

hidmic added 4 commits April 21, 2020 15:26
@hidmic
Extend CLI daemon's API further.
2889c28 
Support ROS graph API for actions plus basic node queries.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Fix NodeStrategy __getattr__ bug.
a074011 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Add xmlrpc marshalling for custom rclpy types.
66bc686 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Add daemon API tests.
385281b 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
dirk-thomas
dirk-thomas reviewed Apr 22, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

From reading the patch it is unclear to me why all the marshaling logic is needed / where it is being used?

This was referenced Apr 22, 2020
Merged
Closed
@hidmic
Copy link
Copy Markdown
Author

hidmic commented Apr 22, 2020 •
edited
Loading

From reading the patch it is unclear to me why all the marshaling logic is needed / where it is being used?

rclpy.node.Node API for graph introspection now includes some custom types that xmlrpc is unable to (un)marshal on its own e.g. TopicEndpointInfo and QoSProfile. This logic adds support for them. I have to say I'm not particularly fond of xmlrpc means to register such logic (it's actually quite limited), but that's how it is.

hidmic added 2 commits April 22, 2020 15:53
@hidmic
Address peer review comments.
7750f7c 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Minimize intermediate imports
e923320 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Copy Markdown
Author

hidmic commented Apr 24, 2020 •
edited
Loading

CI up to ros2cli:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • OSX Build Status
  • Windows Build Status

@hidmic hidmic merged commit fd48f11 into master Apr 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/extend-daemon-api branch April 27, 2020 18:47
@dirk-thomas dirk-thomas mentioned this pull request May 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jacobperron jacobperron Awaiting requested review from jacobperron

@ivanpauno ivanpauno Awaiting requested review from ivanpauno

1 more reviewer

@dirk-thomas dirk-thomas dirk-thomas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hidmic @dirk-thomas