Skip to content
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

Create new service FindValidPose #316

Merged
merged 9 commits into from
Jul 3, 2023
Merged

Create new service FindValidPose #316

merged 9 commits into from
Jul 3, 2023

Conversation

corot
Copy link
Collaborator

@corot corot commented Mar 8, 2023

New MBF service to add to mbf_costmap_nav: it goes in the line of the suggestion at issue #8; but less ambitious by now.
But we can add other services that for example finds a valid pose within an area.

The implementation is on PR #317

mbf_msgs/srv/FindValidPose.srv Show resolved Hide resolved
# will be measured from the padded footprint
---
uint8 FREE = 0 # found pose is completely in traversable space
uint8 INSCRIBED = 1 # found pose is partially in inscribed space
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: I think the FREE / INSCRIBED distinction is only meaningful for costs related to the center point of the robot, but not the footprint? Maybe just return max_cost and average_cost in the result?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can be,,, we just provide the info and the user can decide to ignore it by checking state < LETHAL (most common case)

mbf_msgs/srv/FindValidPose.srv Show resolved Hide resolved
@RainerKuemmerle
Copy link
Contributor

Why would this be best implemented as a service? It might take a considerable amount of time to run (if tolerance is large?) maybe there are intermediate information to transmit in the future to allow the user to abort the search?

@corot
Copy link
Collaborator Author

corot commented Mar 28, 2023

Why would this be best implemented as a service? It might take a considerable amount of time to run (if tolerance is large?) maybe there are intermediate information to transmit in the future to allow the user to abort the search?

sorry, I answered on the wrong place 3 weeks ago!!! 🙈

good question; 2 reasons:

  • we intend this to be fast, so it can be called continuously to adapt to changing situations. From experience, I expect that this won't take more some tens of milliseconds
  • it's an extension of the other services provided at mbf_costmap_nav server, so makes sense to offer the same interface

mbf_msgs/srv/FindValidPose.srv Outdated Show resolved Hide resolved
mbf_msgs/srv/FindValidPose.srv Outdated Show resolved Hide resolved
@@ -21,6 +21,6 @@ uint8 UNKNOWN = 3 # path is partially in unknown spac
uint8 OUTSIDE = 4 # path is partially outside the map

uint32 last_checked # index of the first pose along the path with return_on state or worse
uint8 state # path worst state (until last_checked): FREE, INFLATED, LETHAL, UNKNOWN...
uint8 state # path worst state (until last_checked): FREE, INSCRIBED, LETHAL, UNKNOWN...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OOS: make comment match the constant name
This should not change checksum: http://wiki.ros.org/ROS/Technical%20Overview#Message_serialization_and_msg_MD5_sums

(2 more)

uint8 INSCRIBED = 1 # found pose is partially in inscribed space
uint8 LETHAL = 2 # found pose is partially in collision
uint8 UNKNOWN = 3 # found pose is partially in unknown space
uint8 OUTSIDE = 4 # found pose is partially outside the map
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd sort these error codes by precedence, i.e. FREE=0, INSCRIBED=1, UNKNOWN=2, OUTSIDE=3, LETHAL=4 (+same ordering in the comments). Then the user can just do a <= check with whatever scenario is still acceptable for them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes.... I know would be better,,, but the other services already have this ordering; so I wanted to avoid confusion giving different values to the same constant
we can change all services, thought. Hopefully nobody is using the numeric value!!!

@corot corot marked this pull request as ready for review June 28, 2023 09:53
@corot corot requested a review from ChristofDubs June 28, 2023 09:55
@RainerKuemmerle
Copy link
Contributor

@corot Do you have a benchmark how long the service call will take with the search implementation of #317 ?
Otherwise looks good to me.

@corot
Copy link
Collaborator Author

corot commented Jun 30, 2023

@corot Do you have a benchmark how long the service call will take with the search implementation of #317 ? Otherwise looks good to me.

Yes, it's very fast; we call it at high frequency w/o any problem

mbf_msgs/srv/FindValidPose.srv Outdated Show resolved Hide resolved
mbf_msgs/srv/FindValidPose.srv Outdated Show resolved Hide resolved
mbf_msgs/srv/FindValidPose.srv Outdated Show resolved Hide resolved
mbf_msgs/srv/FindValidPose.srv Outdated Show resolved Hide resolved
corot and others added 4 commits July 3, 2023 15:31
Co-authored-by: ChristofDubs <christof.dubs@gmx.ch>
Co-authored-by: ChristofDubs <christof.dubs@gmx.ch>
Co-authored-by: ChristofDubs <christof.dubs@gmx.ch>
Co-authored-by: ChristofDubs <christof.dubs@gmx.ch>
@corot corot merged commit e9519ed into master Jul 3, 2023
@corot corot deleted the js/FindValidPose.srv branch July 3, 2023 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants