-
Notifications
You must be signed in to change notification settings - Fork 22
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
Allow GoToPlace to know about expected future destinations #61
Conversation
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## main #61 +/- ##
=======================================
Coverage 36.21% 36.21%
=======================================
Files 65 65
Lines 2491 2491
Branches 1366 1366
=======================================
Hits 902 902
Misses 552 552
Partials 1037 1037
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
LGTM!
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@aaronchongth sorry to dismiss your review, I just remembered to update the changelog. |
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.
No worries, thanks for remembering that 🙇
@@ -1,5 +1,9 @@ | |||
## Changelog for package rmf_task | |||
|
|||
2.1.0 (2022-XX-YY) |
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.
2.1.0 (2022-XX-YY) | |
2.1.0 (2022-05-05) |
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.
Apologies, I noticed a mistake in my review. I believe we will need this change log in rmf_task_sequence
instead of rmf_task
.
Just checked in with @marcoag, it sounds like we will need all packages in this repo to have mentioned this minor version, even if there were no changes to the packages.
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Whoops, you're right! I've moved the changes to |
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.
Ah! Gotcha, that makes sense. I'm gonna guess more features or fixes can be piled into this minor version before a release then. LGTM!
A potential issue with the
go_to_place
activity is the scheduling system might not work correctly if the robot needs to go to a location temporarily, and other robots also want to go to that same location, either temporarily or permanently. The two agents will have difficulty negotiating a smooth, consistent solution to their conflict.Adding in fields for the expected next destinations that the agent will go to after it arrives at the current destination will help the scheduling system smooth out these conflicts.