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

Allow the user time-consuming cancel implementations #172

Merged
merged 2 commits into from
Feb 14, 2020

Conversation

Timple
Copy link
Contributor

@Timple Timple commented Feb 14, 2020

by setting the flag after function succeeds

Solves #171

@corot corot requested a review from spuetz February 14, 2020 07:16
@corot
Copy link
Collaborator

corot commented Feb 14, 2020

lgtm, will test during the weekend

@spuetz
Copy link
Collaborator

spuetz commented Feb 14, 2020

This is fine, logically it just moves the setting of the flag down, which will - as discussed - finish the cancel method first, and if the cancel method is not implemented, the controller execution will cancel the action after it returned from computeVel...

@spuetz spuetz merged commit c2afd0d into magazino:master Feb 14, 2020
@spuetz spuetz mentioned this pull request Feb 14, 2020
@Timple
Copy link
Contributor Author

Timple commented Feb 20, 2020

Would this feature be enough to justify a new debian release? 😇

@spuetz
Copy link
Collaborator

spuetz commented Feb 20, 2020

Sure, I will merge the other two open PR at the weekend and make a new release. ;) cheers.

@Timple
Copy link
Contributor Author

Timple commented Mar 18, 2020

ping :)

cxy2020 pushed a commit to cxy2020/move_base_flex that referenced this pull request Mar 24, 2020
* Allow the plugin developer time-consuming cancel implementations by setting the internal cancel flag after the plugin's cancel method succeeded.
cxy2020 pushed a commit to cxy2020/move_base_flex that referenced this pull request Mar 24, 2020
* Allow the plugin developer time-consuming cancel implementations by setting the internal cancel flag after the plugin's cancel method succeeded.
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.

None yet

3 participants