Skip to content

Conversation

@ipa-rmb
Copy link
Contributor

@ipa-rmb ipa-rmb commented Feb 21, 2018

I did not encounter any differences in usage with my applications, please test if your applications run likewise as before.

@ipa-rmb
Copy link
Contributor Author

ipa-rmb commented Feb 21, 2018

The change with the namespaces is a proposal, by the way. I prefer it like that, but if the majority does not like it, we can stay with what we have.

Copy link
Member

@fmessmer fmessmer left a comment

Choose a reason for hiding this comment

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

the duplicate /ns/node_name is quite ugly

also - again - do not provide features from indigo_dev...use a proper feature branch

std::string polygon_service_name = "/map_accessibility_analysis/map_polygon_accessibility_check";
std::string points_service_name = "/map_accessibility_analysis/map_accessibility_analysis/map_points_accessibility_check";
std::string perimeter_service_name = "/map_accessibility_analysis/map_accessibility_analysis/map_perimeter_accessibility_check";
std::string polygon_service_name = "/map_accessibility_analysis/map_accessibility_analysis/map_polygon_accessibility_check";
Copy link
Member

Choose a reason for hiding this comment

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

you could simply remove the ns="map_accessibility_analysis" in the launch file to achieve backwards-compatibility wrt the resulting namespace...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it was with double namespace before. But that's fine, I will change that.

Copy link
Member

@fmessmer fmessmer Feb 22, 2018

Choose a reason for hiding this comment

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

advertising the topic under the - now private - nodehandle changed the topic/service names advertised by the server (because the nodename itself is now prepended to the topic)...see also that you had to add the extra namespace to the client...!

so for compatibility with external applications you need to keep topic/service namespace as they were before....
if you like the parameters in the private namespace you have to remove the external ns of the node...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants