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

Put the action finished callback in a schedule instead of triggering immediately #273

Merged
merged 6 commits into from May 23, 2023

Conversation

mxgrey
Copy link
Contributor

@mxgrey mxgrey commented May 18, 2023

According to open-rmf/rmf#349 there might be a data race related to ActionExecution::finished. This PR should reduce the surface area for where a data race may be occurring.

…immediately

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
@mxgrey mxgrey requested a review from Yadunund May 23, 2023 08:31
Yadunund and others added 3 commits May 23, 2023 16:46
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Yadunund
Yadunund previously approved these changes May 23, 2023
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Copy link
Member

@Yadunund Yadunund left a comment

Choose a reason for hiding this comment

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

I moved the changelog entry for this PR into "Forthcoming" since 2.1.5 is already released.
In the future, we can just update the changelogs when doing a version bump using catkin_generate_changelogs

@Yadunund Yadunund merged commit 6ac5972 into main May 23, 2023
4 checks passed
@Yadunund Yadunund deleted the schedule_action_finish branch May 23, 2023 11:14
aaronchongth pushed a commit that referenced this pull request Jul 3, 2023
…immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Style

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update CHANGELOG

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Move changelog entry into forthcoming

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
aaronchongth pushed a commit that referenced this pull request Jul 3, 2023
…immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Style

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update CHANGELOG

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Move changelog entry into forthcoming

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
aaronchongth added a commit that referenced this pull request Jul 3, 2023
* Adding initiator and request time to booking

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use new booking and request API for EmergencyPullover and ResponsiveWait

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using new booking and request API for FleetUpdateHandle

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use requester instead of initiator, use new API

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* requester name for finishing task factories

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using reverted constructors with nullopt default parameters

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Fix build failures on build farm (#274)

* Fix style for rmf_fleet_adapter

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Make colcon test pass for rmf_traffic_ros2

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Add rmf_fleet_adapter_python to build ci

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update changelogs and bump patch (#275)

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Switch to rst changelogs (#276)

Signed-off-by: Yadunund <yadunund@gmail.com>

* Put the action finished callback in a schedule instead of triggering immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Style

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update CHANGELOG

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Move changelog entry into forthcoming

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>

* Revert changes to constructing finish request factories

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using overloaded TaskPlanner constructor to pass in name of fleet update handle

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using overloaded rmf_task make functions

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Update changelogs

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* 2.2.0

* Bump 2.3.0 (#282)

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Use new booking and request API, updated legacy FullControl fleet adapter

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Added node as parameter to pybinded set_task_planner_params, to pass planner_id and time functor to finishing task factory

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using system_clock instead of steady_clock

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Remove the need to pass a node into the set_task_planner_params python binding

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

---------

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Yadu <yadunund@gmail.com>
Co-authored-by: Grey <grey@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
aaronchongth added a commit that referenced this pull request Jul 3, 2023
* Adding initiator and request time to booking

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use new booking and request API for EmergencyPullover and ResponsiveWait

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using new booking and request API for FleetUpdateHandle

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use requester instead of initiator, use new API

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* requester name for finishing task factories

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using reverted constructors with nullopt default parameters

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Fix build failures on build farm (#274)

* Fix style for rmf_fleet_adapter

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Make colcon test pass for rmf_traffic_ros2

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Add rmf_fleet_adapter_python to build ci

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update changelogs and bump patch (#275)

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Switch to rst changelogs (#276)

Signed-off-by: Yadunund <yadunund@gmail.com>

* Put the action finished callback in a schedule instead of triggering immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Style

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update CHANGELOG

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Move changelog entry into forthcoming

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>

* Revert changes to constructing finish request factories

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using overloaded TaskPlanner constructor to pass in name of fleet update handle

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using overloaded rmf_task make functions

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Update changelogs

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* 2.2.0

* Bump 2.3.0 (#282)

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Use new booking and request API, updated legacy FullControl fleet adapter

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Added node as parameter to pybinded set_task_planner_params, to pass planner_id and time functor to finishing task factory

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using system_clock instead of steady_clock

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Remove the need to pass a node into the set_task_planner_params python binding

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

---------

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Yadu <yadunund@gmail.com>
Co-authored-by: Grey <grey@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
(cherry picked from commit dc740df)
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
aaronchongth added a commit that referenced this pull request Jul 3, 2023
* Adding initiator and request time to booking

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use new booking and request API for EmergencyPullover and ResponsiveWait

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using new booking and request API for FleetUpdateHandle

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Use requester instead of initiator, use new API

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* requester name for finishing task factories

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using reverted constructors with nullopt default parameters

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Fix build failures on build farm (#274)

* Fix style for rmf_fleet_adapter

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Make colcon test pass for rmf_traffic_ros2

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Add rmf_fleet_adapter_python to build ci

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update changelogs and bump patch (#275)

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Switch to rst changelogs (#276)

Signed-off-by: Yadunund <yadunund@gmail.com>

* Put the action finished callback in a schedule instead of triggering immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Style

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update CHANGELOG

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>

* Move changelog entry into forthcoming

Signed-off-by: Yadunund <yadunund@openrobotics.org>

---------

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>

* Revert changes to constructing finish request factories

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using overloaded TaskPlanner constructor to pass in name of fleet update handle

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using overloaded rmf_task make functions

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Update changelogs

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* 2.2.0

* Bump 2.3.0 (#282)

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Use new booking and request API, updated legacy FullControl fleet adapter

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Added node as parameter to pybinded set_task_planner_params, to pass planner_id and time functor to finishing task factory

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Using system_clock instead of steady_clock

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>

* Remove the need to pass a node into the set_task_planner_params python binding

Signed-off-by: Michael X. Grey <grey@openrobotics.org>

---------

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Yadu <yadunund@gmail.com>
Co-authored-by: Grey <grey@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
(cherry picked from commit dc740df)
Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Yadunund added a commit that referenced this pull request Jul 10, 2023
* Adding initiator and request time to booking



* Use new booking and request API for EmergencyPullover and ResponsiveWait



* Using new booking and request API for FleetUpdateHandle



* Use requester instead of initiator, use new API



* requester name for finishing task factories



* Using reverted constructors with nullopt default parameters



* Fix build failures on build farm (#274)

* Fix style for rmf_fleet_adapter



* Make colcon test pass for rmf_traffic_ros2



* Add rmf_fleet_adapter_python to build ci



---------



* Update changelogs and bump patch (#275)



* Switch to rst changelogs (#276)



* Put the action finished callback in a schedule instead of triggering immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately



* Style



* Update CHANGELOG



* Move changelog entry into forthcoming



---------





* Revert changes to constructing finish request factories



* Using overloaded TaskPlanner constructor to pass in name of fleet update handle



* Using overloaded rmf_task make functions



* Update changelogs



* 2.2.0

* Bump 2.3.0 (#282)



* Use new booking and request API, updated legacy FullControl fleet adapter



* Added node as parameter to pybinded set_task_planner_params, to pass planner_id and time functor to finishing task factory



* Using system_clock instead of steady_clock



* Remove the need to pass a node into the set_task_planner_params python binding



---------









(cherry picked from commit dc740df)

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Yadu <yadunund@gmail.com>
Co-authored-by: Grey <grey@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
Yadunund added a commit that referenced this pull request Jul 10, 2023
* Adding initiator and request time to booking



* Use new booking and request API for EmergencyPullover and ResponsiveWait



* Using new booking and request API for FleetUpdateHandle



* Use requester instead of initiator, use new API



* requester name for finishing task factories



* Using reverted constructors with nullopt default parameters



* Fix build failures on build farm (#274)

* Fix style for rmf_fleet_adapter



* Make colcon test pass for rmf_traffic_ros2



* Add rmf_fleet_adapter_python to build ci



---------



* Update changelogs and bump patch (#275)



* Switch to rst changelogs (#276)



* Put the action finished callback in a schedule instead of triggering immediately (#273)

* Put the action finished callback in a schedule instead of triggering immediately



* Style



* Update CHANGELOG



* Move changelog entry into forthcoming



---------





* Revert changes to constructing finish request factories



* Using overloaded TaskPlanner constructor to pass in name of fleet update handle



* Using overloaded rmf_task make functions



* Update changelogs



* 2.2.0

* Bump 2.3.0 (#282)



* Use new booking and request API, updated legacy FullControl fleet adapter



* Added node as parameter to pybinded set_task_planner_params, to pass planner_id and time functor to finishing task factory



* Using system_clock instead of steady_clock



* Remove the need to pass a node into the set_task_planner_params python binding



---------









(cherry picked from commit dc740df)

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Yadu <yadunund@gmail.com>
Co-authored-by: Grey <grey@openrobotics.org>
Co-authored-by: Yadunund <yadunund@openrobotics.org>
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

2 participants