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

AMRNAV-6708 AMR7 placing pallet #76

Conversation

VladyslavHrynchak200204
Copy link

@VladyslavHrynchak200204 VladyslavHrynchak200204 commented Aug 5, 2024


Basic Info

Info Please fill out this column
Ticket(s) this addresses https://lvserv01.logivations.com/browse/AMRNAV-6708
Primary OS tested on (Ubuntu)
Robotic platform tested on (gazebo simulation)

Description of contribution in a few bullet points

Reason for change:

AMR7 placed a pallet not quite next to the pallet inside the bin leave between them too much space. We call DriveOnHeading, after 2s we fail because of and cancel it.

image

After that we run another DriveOnHeading which fails because we don`t wait until previous call DriveOnHeading stops. This cause this issue.

image

image

Changes in this PR:

  • added postponed_handle_ field in SimpleActionServer
  • added void handle_postponed() function in SimpleActionServer
  • added calls of handle_postponed() function in TimedBehavior to process postponed goal after terminate_all()

Receive terminate_all for first DriveOnHeading -> accept new goal for second DriveOnHeading -> add new goal to postponed_handle_ -> terminate_all for first DriveOnHeading -> call handle_postponed() after we terminate goals successful -> handle postponed_handle_ goal

image

@VladyslavHrynchak200204 VladyslavHrynchak200204 added bug Something isn't working and removed bug Something isn't working labels Aug 5, 2024
@VladyslavHrynchak200204 VladyslavHrynchak200204 marked this pull request as draft August 5, 2024 13:09
if(is_active(current_handle_) && is_cancel_requested()){
postponed_handle_ = handle;
info_msg("Postpone goal");
return;

Choose a reason for hiding this comment

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

If current_handle_ is active and requested to cancel the goal, we store it in postponed_handle_ to handle that after we successfully cancel our previous goal.

@@ -242,6 +243,7 @@ class TimedBehavior : public nav2_core::Behavior
result->total_elapsed_time = elasped_time_;
action_server_->terminate_all(result);
onActionCompletion();
action_server_->handle_postponed();
return;
}

Choose a reason for hiding this comment

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

After we cancel our goal, we call handle_postponed() to handle goals, which we received during cancelling previous goal

if(is_active(current_handle_) && is_cancel_requested()){
postponed_handle_ = handle;
info_msg("Postpone goal");
return;

Choose a reason for hiding this comment

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

I can make postponed_handle_ as queue of postponed goals, or we can postpone only one goal as we have now.

Copy link

@jplapp jplapp left a comment

Choose a reason for hiding this comment

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

looks like this works but I want to avoid duplicating the concept of "preemption". This is sending a new goal, while the action server already has a goal.

In which category do we fall here? Do we cancel the old goal first, or do we already send a new one? The latter would be preemption so you could try to implement that instead ? (see comment // TODO(orduno) #868 Enable preempting a Behavior on-the-fly without stopping

@VladyslavHrynchak200204
Copy link
Author

VladyslavHrynchak200204 commented Aug 5, 2024

looks like this works but I want to avoid duplicating the concept of "preemption". This is sending a new goal, while the action server already has a goal.

In which category do we fall here? Do we cancel the old goal first, or do we already send a new one? The latter would be preemption so you could try to implement that instead ? (see comment // TODO(orduno) #868 Enable preempting a Behavior on-the-fly without stopping

In concept of "preemption" we receive new goal -> save in pending_handle_ -> set preempt_requested_ = true -> if (action_server_->is_preempt_requested()) we aborting and stopping our current goal -> send preempting goal .

But when i implemented my concept of "postponed goal" i never received a message "Received a preemption request for %s,
however feature is currently not implemented. Aborting and stopping." because i wait until we cancel previous goal.

we have two ways to implement that:

1 - avoid preemption concept and use my - when we have goal to cancel, we wait until it cancels and postpone new goal, then start to handle it.

2 - use concept of preemption but improve it -> if current goal is active -> we received new goal -> new goal store in queue -> run new goal from queue if we cancel, abort, finished successful previous goal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants