Add LoadMap service definition #152
+17
−1
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ add_service_files( | |
| FILES | ||
| GetMap.srv | ||
| GetPlan.srv | ||
| SetMap.srv | ||
| LoadMap.srv) | ||
|
|
||
| add_action_files( | ||
| FILES | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # URL of map resource | ||
| # Can be an absolute path to a file: file:///path/to/maps/floor1.yaml | ||
| # Or, relative to a ROS package: package://my_ros_package/maps/floor2.yaml | ||
| string map_url | ||
| --- | ||
| # Result code defintions | ||
| uint8 RESULT_SUCCESS=0 | ||
|
There was a problem hiding this comment. I'd prefer if these were just There was a problem hiding this comment. Actually, its not: Subtle difference. There was a problem hiding this comment. ah, yes sorry. Its not something I care deeply about, just a comment that it seems related enough with the namespace the additional wording just uses linespace. If no one else agrees, I'm certainly not going to push it. |
||
| uint8 RESULT_MAP_DOES_NOT_EXIST=1 | ||
| uint8 RESULT_INVALID_MAP_DATA=2 | ||
| uint8 RESULT_INVALID_MAP_METADATA=3 | ||
| uint8 RESULT_UNDEFINED_FAILURE=255 | ||
|
|
||
| # Returned map is only valid if result equals RESULT_SUCCESS | ||
| nav_msgs/OccupancyGrid map | ||
| uint8 result | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'd like to posit that how the map is loaded or what type of file the URL points to is up to the node implementing the service. E.g. it might point to a yaml file containing the metadata, or maybe an image file directly.
See ros-navigation/navigation2#1303 (comment) for a relevant discussion.
cc @SteveMacenski
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 this is also for ROS1, I'm not as concerned about it.
However you think we can change this to
map_uri? I typically think of URLs as links to a server rather than a URI which can be a local path.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.
A URL can also be a local path. It describes a way to locate a resource.
I'm okay with changing this to a more general URI, but would like to hear what others think. It might make the semantics more ambiguous.
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 know a URL 'is' that, but in vernacular we usually talk about servers. A URL is a subclass of URI. 100% agree its semantics. I just think it makes it more clear actually because of the vernacular uses and URI is what is used in SDF for getting models/packages/files in the same way this is attempting to so its consistent with other ROS-y tools.
I don't care that much one way or another since I know what it means, but I could see someone just looking at a message definition in
ros2 msg show ...and thinking it had to be a web link. If no one else agrees, I'm not going to push it.