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

[DRAFT] Backports EventsExecutor from rolling/upstream #145

Open
wants to merge 11 commits into
base: irobot/humble
Choose a base branch
from

Conversation

apojomovsky
Copy link

This PR merges the EventsExecutor from the official repository.

@apojomovsky apojomovsky force-pushed the apojomovsky/upstream-events-executor branch from 1f80ed2 to 43ca625 Compare April 16, 2024 19:22
@apojomovsky apojomovsky force-pushed the apojomovsky/upstream-events-executor branch 3 times, most recently from e860ad2 to f52e445 Compare May 10, 2024 18:17
mjcarroll and others added 5 commits May 16, 2024 15:04
* Deprecate callback_group call taking context

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Add base executor objects that can be used by implementors

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Template common operations

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Address reviewer feedback:

* Add callback to EntitiesCollector constructor
* Make function to check automatically added callback groups take a list

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lint

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Address reviewer feedback and fix templates

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lint and docs

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Make executor own the notify waitable

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Add pending queue to collector, remove from waitable

Also change node's get_guard_condition to return shared_ptr

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Change interrupt guard condition to shared_ptr

Check if guard condition is valid before adding it to the waitable

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lint and docs

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Utilize rclcpp::WaitSet as part of the executors

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Don't exchange atomic twice

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Fix add_node and add more tests

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Make get_notify_guard_condition follow API tick-tock

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Improve callback group tick-tocking

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Don't lock twice

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Address reviewer feedback

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Add thread safety annotations and make locks consistent

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* @wip

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Reset callback groups for multithreaded executor

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Avoid many small function calls when building executables

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Re-trigger guard condition if buffer has data

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Address reviewer feedback

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Trace points

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Remove tracepoints

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Reducing diff

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Reduce diff

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Uncrustify

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Restore tests

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Back to weak_ptr and reduce test time

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* reduce diff and lint

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Restore static single threaded tests that weren't working before

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Restore more tests

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Fix multithreaded test

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Fix assert

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Fix constructor test

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Change ready_executables signature back

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Don't enforce removing callback groups before nodes

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Remove the "add_valid_node" API

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Only notify if the trigger condition is valid

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Only trigger if valid and needed

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Fix spin_some/spin_all implementation

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Restore single threaded executor

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Picking ABI-incompatible executor changes

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Add PIMPL

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Additional waitset prune

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Fix bad merge

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Expand test timeout

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Introduce method to clear expired entities from a collection

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Make sure to call remove_expired_entities().

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

* Prune queued work when callback group is removed

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Prune subscriptions from dynamic storage

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Styles fixes.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

* Re-trigger guard conditions

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Condense to just use watiable.take_data

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lint

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Address reviewer comments (nits)

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lock mutex when copying

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Refactors to static single threaded based on reviewers

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* More small refactoring

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lint

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Lint

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Add ready executable accessors to WaitResult

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Make use of accessors from wait_set

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Fix tests

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Fix more tests

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Tidy up single threaded executor implementation

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* Don't null out timer, rely on call

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

* change how timers are checked from wait result in executors

Signed-off-by: William Woodall <william@osrfoundation.org>

* peak -> peek

Signed-off-by: William Woodall <william@osrfoundation.org>

* fix bug in next_waitable logic

Signed-off-by: William Woodall <william@osrfoundation.org>

* fix bug in StaticSTE that broke the add callback groups to executor tests

Signed-off-by: William Woodall <william@osrfoundation.org>

* style

Signed-off-by: William Woodall <william@osrfoundation.org>

---------

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: William Woodall <william@osrfoundation.org>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: William Woodall <william@osrfoundation.org>
The commit also includes a few test fixes and temporary comments
allowing the unit-tests, and the full package, to build.
Get the upstream executor_notify_waitable.cpp/.hpp and the
test_executors_timer_cancel_behavior unit-test
Signed-off-by: Alexis Tsogias <a.tsogias@cellumation.com>
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Alexis Tsogias <a.tsogias@cellumation.com>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>
@apojomovsky apojomovsky force-pushed the apojomovsky/upstream-events-executor branch from f52e445 to 443b777 Compare May 16, 2024 19:49
Alexis Pojomovsky and others added 5 commits May 17, 2024 16:26
* trigger guard condition waitset regardless of whether a trigger callback is present

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

* restore mutex in guard_condition.cpp

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

* remove whitespace

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

* add unit-test

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

* add documentation for trigger and set_on_trigger_callback

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
@apojomovsky apojomovsky force-pushed the apojomovsky/upstream-events-executor branch from 2b32d25 to 03163bf Compare May 29, 2024 18:01
This reverts commit a09f6f8.
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.

4 participants