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

Use variable move mission #16

Merged
merged 7 commits into from
Nov 21, 2022
Merged

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented Sep 1, 2022

Fixes: #7

  • Move missions should be a predefined variable mission, so no additional missions are posted to the MiR server whenever commanded.
  • Docking uses a predefined mission too.
  • A charge finishing_request, executes the dock command successfully, tested on hardware, however once docked and charging starts, it was found that the fleet adapter is unable to interrupt it and send new commands even though the robot is fully charged, charge finishing task can't be interrupted #17

Copy link
Collaborator

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

I left a few comments, but I wouldn't consider them to be blockers.

fleet_adapter_mir/fleet_adapter_mir.py Outdated Show resolved Hide resolved
fleet_adapter_mir/MiRCommandHandle.py Show resolved Hide resolved
@mxgrey
Copy link
Collaborator

mxgrey commented Sep 6, 2022

I have one general concern related to the implementation of this fleet adapter.

On this line and this line we're looking at the state reported by MiR, and if it gives us a READY result then we assume that it has reached the last waypoint that we commanded to it.

However, what happens if the system falls out of sync? For example, what if the following sequence happens:

  • fleet adapter sends mission request,
  • fleet adapter requests task state
  • MiR receives task state request and issues response that the robot is Ready
  • MiR receives mission request
  • fleet adapter receives confirmation of mission request
  • fleet adapter receives response that robot is Ready, which is taken to mean that the last mission request has already succeeded

This sequence may be improbable, but unless there's something in the REST API library to guarantee that the REST request/response ordering is FIFO, we have to consider that this sequence is possible. Packets dropping over wifi with TCP resending the requests + responses could allow this sequence to happen. I've certainly witnessed this kind of behavior in async systems that don't have message ordering guarantees.

…pdated all use cases as well, added additional documentation

Signed-off-by: Aaron Chong <[email protected]>
@aaronchongth
Copy link
Member Author

Yeah I can see that happening in rare scenarios, #20.

I'll try to get it addressed in the next overhaul.

@aaronchongth aaronchongth merged commit 09c0fea into perform-action Nov 21, 2022
@aaronchongth aaronchongth deleted the use-variable-move-mission branch November 21, 2022 04:16
aaronchongth added a commit that referenced this pull request Nov 21, 2022
* Adding task planner params, waypoint and lane merge distance, various fixes in accessing dict

Signed-off-by: Aaron Chong <[email protected]>

* Using member function to insert update rmf_updater instead of lambda

Signed-off-by: Aaron Chong <[email protected]>

* Basic prototype for perform action working, needs refactoring

Signed-off-by: Aaron Chong <[email protected]>

* Reading actions from config file to be accepted

Signed-off-by: Aaron Chong <[email protected]>

* Add example for perform action, and added in max merge waypoint and lane distance in example config

Signed-off-by: Aaron Chong <[email protected]>

* Changing rouge warn to info

Signed-off-by: Aaron Chong <[email protected]>

* Reporting error to execution

Signed-off-by: Aaron Chong <[email protected]>

* Fix missing assignment to None

Signed-off-by: Aaron Chong <[email protected]>

* Using a Action class to encompass all action related members

Signed-off-by: Aaron Chong <[email protected]>

* Use variable move mission (#16)

* First implementation of calling predefined variable move mission

Signed-off-by: Aaron Chong <[email protected]>

* added variable_move_mission field in example config, fixed mir_config

Signed-off-by: Aaron Chong <[email protected]>

* Add assert that start sets are not empty, and fix API usage for start.lane

Signed-off-by: Aaron Chong <[email protected]>

* Removed unused create move missions, fix use before declare

Signed-off-by: Aaron Chong <[email protected]>

* Docking using pre-defined mission, adding to example config

Signed-off-by: Aaron Chong <[email protected]>

* Changed variable_move_mission to more descriptive rmf_move_mission, updated all use cases as well, added additional documentation

Signed-off-by: Aaron Chong <[email protected]>

Signed-off-by: Aaron Chong <[email protected]>

Signed-off-by: Aaron Chong <[email protected]>
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