Conversation
fix topics/srvices prefixes
mikaelarguedas
added
in review
The double <tag> surprised me at first
dhood
left a comment
dhood
left a comment
There was a problem hiding this comment.
I made a couple of small changes so the code is easier to read. I think it's reasonable now that it doesn't need to deal with the two tag types, but feel free to revert them if the changes weren't appropriate
sros2/api/__init__.py
Outdated
| """ % (tag, 'rt', topic_name, tag) | ||
| """ % (tag, 'rt/' + topic_name, tag) | ||
| # TODO(mikaelarguedas) remove this hardcoded handling for default parameter topics | ||
| # TODO(mikaelarguedas) remove the need for empty partition |
sros2/api/__init__.py
Outdated
| for key, value in service_topic_prefixes.items(): | ||
| if key == 'Request': | ||
| for service_direction, topic_prefix in service_topic_prefixes.items(): | ||
| if service_direction == 'Request': |
There was a problem hiding this comment.
Thanks for looking into that. I couldnt find a "good" name when I refactored so I didnt change it.
I don't think that "service_direction" is what we mean here. For example, a service server would subscribe to a Request topic.
I made another attempt at renaming the variable in 70bc265 trying to match the "topic_prefix" variable introduced here. Feedback welcome
There was a problem hiding this comment.
yep, good point, that name looks good!
|
merging this as all dependeant PRs have been merged. Access control is currently not working since we introduced the parameter servers starting automatically with the node. As this is out of the scope of this PR I will address it in a follow-up |
3 participants
This moves the hack for default created topics and services from partition to topic names to match the code changes from ros2/ros2#476.
Note that this now enforces that all created topics have an empty partition
Connects to ros2/ros2#476