Skip to content

Add rosidl_runtime_py package - #34

Merged
jacobperron merged 2 commits into
masterfrom
rosidl_runtime_py
Mar 28, 2019
Merged

Add rosidl_runtime_py package#34
jacobperron merged 2 commits into
masterfrom
rosidl_runtime_py

Conversation

@jacobperron

@jacobperronjacobperron commented Mar 15, 2019

Copy link
Copy Markdown
Member

Migrated utility functions for working with Python ROS messages from the package ros2topic.

The intention is to reuse these functions in other CLI tools: ros2service and ros2action.

Replaces ros2/rclpy#285.

@jacobperronjacobperron added the in progress Actively being worked on (Kanban column) label Mar 15, 2019
This was referenced Mar 15, 2019
Closed
Merged
@jacobperron

jacobperron commented Mar 15, 2019

Copy link
Copy Markdown
MemberAuthor
  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron

jacobperron commented Mar 15, 2019

Copy link
Copy Markdown
MemberAuthor
  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

This was referenced Mar 15, 2019
Merged
Closed
@jacobperronjacobperron added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Mar 15, 2019
dirk-thomas
dirk-thomas reviewed Mar 27, 2019
View reviewed changes

@dirk-thomasdirk-thomas left a comment

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.

Some of the code seems to be copied from other repos. Please provide exact links to the copied snippets to allow evaluating that they are the same. I think some of them might also have changed since this PR was created and need to be updated.

Comment threadrosidl_runtime_py/package.xml Outdated
Comment threadrosidl_runtime_py/rosidl_runtime_py/set_message.py Outdated
Comment threadrosidl_runtime_py/rosidl_runtime_py/set_message.py Outdated
Comment threadrosidl_runtime_py/rosidl_runtime_py/convert.py Outdated
@jacobperron
Add rosidl_runtime_py package
0d687b9
Migrated utility functions for working with Python ROS messages from the package `ros2topic`.
Conversion implementation taken from:
https://github.com/ros2/ros2cli/blob/978fc8807cb1f70b5f9c2b085e7b6fa74d191d0e/ros2topic/ros2topic/verb/echo.py
Set message implementation taken from
https://github.com/ros2/ros2cli/blob/978fc8807cb1f70b5f9c2b085e7b6fa74d191d0e/ros2topic/ros2topic/api/__init__.py#L93-L119
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Refactor rosidl_runtime_py functions
4171c5a
* Add docstrings and annotations.
* Rename 'msg' -> 'message'
* Add more tests
* Remove default value from getattr in convert function
* Remove custom SetFieldError exception
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron

jacobperron commented Mar 28, 2019

Copy link
Copy Markdown
MemberAuthor

I've gone back and grabbed the latest code from ros2topic and put the initial migration in its own commit before my changes so the diff should be more clear.

Conversion implementation taken from:

https://github.com/ros2/ros2cli/blob/978fc8807cb1f70b5f9c2b085e7b6fa74d191d0e/ros2topic/ros2topic/verb/echo.py

Set message implementation taken from:

https://github.com/ros2/ros2cli/blob/978fc8807cb1f70b5f9c2b085e7b6fa74d191d0e/ros2topic/ros2topic/api/__init__.py#L93-L119

@jacobperron

jacobperron commented Mar 28, 2019

Copy link
Copy Markdown
MemberAuthor

The subsequent replacement in ros2cli: ros2/ros2cli#209

dirk-thomas
dirk-thomas approved these changes Mar 28, 2019
View reviewed changes

@dirk-thomasdirk-thomas left a comment

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.

I would suggest to merge without squashing to keep the two commits separate.

@jacobperron

jacobperron commented Mar 28, 2019

Copy link
Copy Markdown
MemberAuthor

Thanks for the review!

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

@jacobperron
jacobperron merged commit 4fe2166 into masterMar 28, 2019
@jacobperron
jacobperron deleted the rosidl_runtime_py branch March 28, 2019 21:36
@jacobperronjacobperron removed the in review Waiting for review (Kanban column) label Mar 28, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@dirk-thomasdirk-thomasdirk-thomas approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@dirk-thomasdirk-thomas

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jacobperron@dirk-thomas