Support non-absolute topic names. #57
Conversation
If the user passes "/topic_name" to the ros2 echo command, it works properly. If they pass "topic_name" to the ros2 echo command, it fails to match. This change just allows us to deal with non-absolute topic names. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
|
I think it would be better, since it would respect namespaces as well. |
|
Only because I didn't know about it :). Thanks, I'll give that a try. |
This will handle namespaces as well. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
clalancette
added
in review
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
|
I added in the validation as @wjwwood suggested. I'm also running a CI job, just on Linux, to make sure there aren't any lingering linting/pep8 errors: |
ros2topic/ros2topic/verb/echo.py
Outdated
| @@ -97,8 +99,10 @@ def represent_ordereddict(dumper, data): | |||
| def subscriber(node, topic_name, message_type, callback): | |||
| if message_type is None: | |||
| topic_names_and_types = get_topic_names_and_types(node=node) | |||
| expanded_name = expand_topic_name(topic_name, node.get_name(), node.get_namespace()) | |||
| validate_full_topic_name(expanded_name, is_service=False) | |||
There was a problem hiding this comment.
Nit: False being the default you shouldn't need to have to pass the is_service keyword
There was a problem hiding this comment.
Good call. Will fix that before merging.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
|
CI is green, I fixed the small problem, and it's approved. Merging. |
| @@ -97,8 +99,10 @@ def represent_ordereddict(dumper, data): | |||
| def subscriber(node, topic_name, message_type, callback): | |||
| if message_type is None: | |||
| topic_names_and_types = get_topic_names_and_types(node=node) | |||
| expanded_name = expand_topic_name(topic_name, node.get_name(), node.get_namespace()) | |||
| validate_full_topic_name(expanded_name) | |||
There was a problem hiding this comment.
This function might raise an InvalidTopicNameException which I assume results in printing a stacktrace which is not a good user experience. The possible exceptions should be catched and a "nicer" error message (without a stacktrace) should be printed instead.
There was a problem hiding this comment.
Yeah, when I made that suggestion I meant you could raise it here, catch it and do something nicer. If you just want the traceback, then that will happen when you try to create a pub or sub with the topic.
There was a problem hiding this comment.
Ah, OK. I'll open a new PR with something nicer.
* Added support for command line launch arguments Updated to support passing in command line arguments in the same format used by roslaunch (i.e., arg_name:=value). This supports zero or more arguments added after one or more launch files passed on the command line. * Updates to parsing/handling of command line args * Addressed review comments Fixed linter errors, updated copyrights, updated sphinx documentation, used different form of print. * pep257 fixup * use single quotes * Addressing comments from review Using single quotes instead of double quotes, fixed mistaken copyright year, allow the launch arg value to contain the arg separator, cleaned up main * Updated to support --args format * Fixing additional style errors Removed unnecessary code, fixed style/linting problems. * while passing the linter as is the extra indentation makes it a bit easier to read
* Add substitution for finding package share directory
Expose FindPackageShare as 'find-pkg-share' and FindPackagePrefix as 'find-pkg-prefix'.
This turns
<include file="$(find-pkg foo)/share/foo/launch/foo.launch.xml">
into
<include file="$(find-pkg-share foo)/launch/foo.launch.xml">
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Refactor to use abstract class
FindPackage is now an abstract class that requires subclasses to implement a 'find' method.
FindPackagePrefix is the subclass that finds the package prefix.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Update error message and describe() method
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
If the user passes "/topic_name" to the ros2 echo
command, it works properly. If they pass "topic_name"
to the ros2 echo command, it fails to match. This
change just allows us to deal with non-absolute topic
names.
This should fix #54 . I ran the tests against the ros2cli package locally,
with no errors. No CI since this is python code and it won't tell us very much.
Signed-off-by: Chris Lalancette clalancette@gmail.com