-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ros2topic] Use import message logic provided by rosidl_runtime_py #415
Conversation
Connects to #218. Note that the action feedback logic in the echo verb was incorrect, resulting in a ModuleImportError. The new logic added in ros2/rosidl_runtime_py#9 should fix the error. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Rebuild with latest change for sanity: Edit: I'll wait for new linter errors from an update to be resolved in ros2/rclpy#480 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -128,7 +128,7 @@ def subscriber( | |||
raise RuntimeError( | |||
'Could not determine the type for the passed topic') | |||
|
|||
msg_module = import_message_type(topic_name, message_type) | |||
msg_module = get_message(message_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobperron meta^2: I wonder what'd happen if you used nargs='?', type=get_message
above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it would be fine if we change this line to msg_module = message_type
. But this seems out of scope of this PR. If you'd like to change it in a follow-up, go for it :)
Connects to #218.
Note that the action feedback logic in the echo verb was incorrect, resulting in a ModuleImportError.
The new logic added in ros2/rosidl_runtime_py#9 should fix the error.
Depends on ros2/rosidl_runtime_py#9