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

Mauro/add events executor #63

Open
wants to merge 99 commits into
base: irobot/add-events-executor
Choose a base branch
from

Conversation

mauropasse
Copy link
Collaborator

@mauropasse mauropasse commented Apr 14, 2021

Changes

  • Use rclcpp::GuardConditions 53cc25d
  • Todo: Use latest rclcpp::GuardConditions
  • Apply william's refactor to make callbacks type safe 56405fc
  • Use std::function as executor callbacks 1412e94
  • Add callback to rclcpp::GuardConditions 32aa3db
  • Add callback to rclcpp intra-process subscriptions 7cadae3
  • Limit intra-process subscription events to QoS depth size.
  • Limit nodes GuardConditions max events to one 41c8f8a
  • Todo: Fix tests

ivanpauno and others added 30 commits March 12, 2021 13:07
…1532)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
…ers (ros2#1568)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
…alse (ros2#1514)

Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Guard against overflow when converting from rclcpp::Duration to builtin_interfaces::msg::Duration,
which is a unsigned to signed conversion.

Use non-std int types for consistency

Handle large negative values

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* make rcl_com_interface optional

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>

* Update rclcpp_lifecycle/test/mocking_utils/patch.hpp

Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

* line break

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>

* use state_machine_options

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>

Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Clang is complaining about the looping variable being referenced as `const val` but should rather be `const ref`.

```
/Users/karsten/workspace/ros2/ros2_master/src/ros2/rclcpp/rclcpp/src/rclcpp/parameter_service.cpp:46:25: error: loop variable 'param' of type 'const rclcpp::Parameter' creates a copy from type 'const rclcpp::Parameter' [-Werror,-Wrange-loop-construct]
        for (const auto param : parameters) {
                        ^
/Users/karsten/workspace/ros2/ros2_master/src/ros2/rclcpp/rclcpp/src/rclcpp/parameter_service.cpp:46:14: note: use reference type 'const rclcpp::Parameter &' to prevent copying
        for (const auto param : parameters) {
             ^~~~~~~~~~~~~~~~~~
                        &
1 error generated.
```

Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* Document misuse of parameters callback

Related to ros2#1587

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove bad example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add API for checking QoS profile compatibility

Depends on ros2/rmw#299

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Refactor as free function

Returns a struct containing the compatibility enum value and string for the reason.

Updated tests to reflect behavior changes upstream.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Remove rmw_connext_cpp references.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* add automatically_add_executor_with_node option

Signed-off-by: Brice <brice.renaudeau@wyca.fr>

* Fix typo

Signed-off-by: Brice <brice.renaudeau@wyca.fr>

* add option usage in test

Signed-off-by: Brice <brice.renaudeau@wyca.fr>

* Document parameter

Signed-off-by: Brice <brice.renaudeau@wyca.fr>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Clock subscription callback group spins in its own thread

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* fix line length

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* fix code style divergence

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* fix code style divergence

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Initialize only once clock_callbackgroup

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Check if thread joinable before start a new thread

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Cancel executor and join thread in destroy_clock_sub

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Check if clock_thread_ is joinable before cancel executor and join

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Add use_clock_thread as an option

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Fix code style divergence

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Fix code style divergence

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Fix code style divergence

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* Add TimeSource tests

Signed-off-by: anaelle-sw <anaelle.sarazin@wyca.fr>

* update use_clock_thread value in function attachNode

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* join clock thread in function detachNode

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* TimeSource tests: fixes + comments + more tested cases

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* clean destroy_clock_sub()

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* flag to ensure clock_executor is cancelled

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* Always re-init clock_callback_group when creating a new clock sub

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* spin_until_future_complete() to cleanly cancel executor

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* fix tests warnings

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* fix test error: cancel clock executor

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* clean comments

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>

* fix precision loss

Signed-off-by: anaelle <anaelle.sarazin@wyca.fr>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
This reverts commit 3ab6571.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Apparently, the topics and services that LifecycleNode provides are not
available immediately after creating a node.

Fix flaky tests by accounting for some delay between the LifecycleNode
constructor and queries about its topics and services.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
…fined behavior (ros2#1609)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Alberto Soragna <asoragna@irobot.com>
Signed-off-by: Alberto Soragna <asoragna@irobot.com>
alsora and others added 29 commits April 23, 2021 15:18
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Complete support for rmw listener APIs
…in CancelCallback (ros2#1635)

* Fix deadlock issue that caused by other mutexes locked in CancelCallback

Signed-off-by: Kaven Yau <love29881460@qq.com>

* Add unit test for rclcpp action server deadlock

Signed-off-by: Kaven Yau <love29881460@qq.com>

* Update rclcpp_action/test/test_server.cpp

Co-authored-by: William Woodall <william+github@osrfoundation.org>

Co-authored-by: Kaven Yau <love29881460@qq.com>
Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Co-authored-by: William Woodall <william+github@osrfoundation.org>
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
…s manager (ros2#1643)

* use dynamic_pointer_cast to detect allocator mismatch in intra process manager

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

* add test case for mismatched allocators

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

* forward template arguments to avoid mismatched types in intra process manager

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

* style fixes

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

* refactor to test message address and count, more DRY

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

* update copyright

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

* fix typo

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

Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>

Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>
)

1. Add remove_on_shutdown_callback() in rclcpp::Context

Signed-off-by: Barry Xu <barry.xu@sony.com>

2. Add add_on_shutdown_callback(), which returns a handle that can be removed by remove_on_shutdown_callback().

Signed-off-by: Barry Xu <barry.xu@sony.com>
… CANCELING state (ros2#1641)

Signed-off-by: Kaven Yau <kavenyau@foxmail.com>
…os2#1648)

* Wait on graph guard condition for graph changes to propagate

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
…os2#1647)

* Keep custom allocator in publisher and subscription options alive.

Also, enforce an allocator bound to void is used to avoid surprises.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Avoid sizeof(void) in MyAllocator implementation.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Address peer review comment

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

* Use a lazely initialized private field when 'allocator' is not initialized

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Keep a rebound allocator for byte-sized memory blocks around
for publisher and subscription options.

Follow-up after 1fc2d58

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* initial version of type_adaptor.hpp

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

* initial version of rclcpp::get_message_type_support_handle()

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

* initial version of rclcpp::is_ros_compatible_type check

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

* fixup include statement order in publisher.hpp

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

* use new rclcpp::get_message_type_support_handle() and check in Publisher

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

* update adaptor->adapter, update TypeAdapter to use two arguments, add implicit default

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

* move away from shared_ptr<allocator> to just allocator, like the STL

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

* fixes to TypeAdapter and adding new publish function signatures

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

* bugfixes

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

* more bugfixes

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

* Add nullptr check

Signed-off-by: Audrow Nash <audrow@hey.com>

* Remove public from struct inheritance

Signed-off-by: Audrow Nash <audrow@hey.com>

* Add tests for publisher with type adapter

Signed-off-by: Audrow Nash <audrow@hey.com>

* Update packages to C++17

Signed-off-by: Audrow Nash <audrow@hey.com>

* Revert "Update packages to C++17"

This reverts commit 4585605.

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

* Begin updating AnySubscriptionCallback to use the TypeAdapter

Signed-off-by: Audrow Nash <audrow@hey.com>

* Use type adapter's custom type

Signed-off-by: Audrow Nash <audrow@hey.com>

* Correct which AnySubscriptionCallbackHelper is selected

Signed-off-by: Audrow Nash <audrow@hey.com>

* Setup dispatch function to work with adapted types

Signed-off-by: Audrow Nash <audrow@hey.com>

* Improve template logic on dispatch methods

Signed-off-by: Audrow Nash <audrow@hey.com>

* implement TypeAdapter for Subscription

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

* Add intraprocess tests with all supported message types

Signed-off-by: Audrow Nash <audrow@hey.com>

* Add intra process tests

Signed-off-by: Audrow Nash <audrow@hey.com>

* Add tests for subscription with type adapter

Signed-off-by: Audrow Nash <audrow@hey.com>

* Fix null allocator test

Signed-off-by: Audrow Nash <audrow@hey.com>

* Handle serialized message correctly

Signed-off-by: Audrow Nash <audrow@hey.com>

* Fix generic subscription

Signed-off-by: Audrow Nash <audrow@hey.com>

* Fix trailing space

Signed-off-by: Audrow Nash <audrow@hey.com>

* fix some issues found while testing type_adapter in demos

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

* add more tests, WIP

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

* Improve pub/sub tests

Signed-off-by: Audrow Nash <audrow@hey.com>

* Apply uncrustify formatting

Signed-off-by: Audrow Nash <audrow@hey.com>

* finish new tests for any subscription callback with type adapter

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

* fix adapt_type<...>::as<...> syntax

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

* fix explicit template instantiation of create_subscription() in new test

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

* cpplint fix

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

* Fix bug by aligning allocator types on both sides of ipm

Signed-off-by: Audrow Nash <audrow@hey.com>

* Fix intra process manager tests

Signed-off-by: Audrow Nash <audrow@hey.com>

Co-authored-by: Audrow Nash <audrow@hey.com>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Pausing and resuming the measurement inside the timing loop can cause
the initial run duration calculation to underestimate how long the
benchmark is taking to run, which results in the recorded run taking a
lot longer than it should. This is a known issue in libbenchmark.

This test is affected by that behavior, and typically takes a bit longer
than the others. The easiest thing to do right now is to just bump the
timeout. My tests show that 180 seconds is typically sufficient for this
test, so 240 should be a safe point to conclude that the test is
malfunctioning.

Signed-off-by: Scott K Logan <logans@cottsay.net>
* fix syntax issue with gcc

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

* uncrustify

Signed-off-by: William Woodall <william@osrfoundation.org>
Signed-off-by: Scott K Logan <logans@cottsay.net>
* Declare parameters uninitialized

Fixes ros2#1649

Allow declaring parameters without an initial value or override.
This was possible prior to Galactic, but was made impossible since we started enforcing the types of parameters in Galactic.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove assertion

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Throw NoParameterOverrideProvided exception if accessed before initialized

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test getting static parameter after it is set

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Do not throw on access of uninitialized dynamically typed parameter

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Rename exception type

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove unused exception type

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Uncrustify

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
)

* Fix occasionally missing goal result caused by race condition

Signed-off-by: Kaven Yau <kavenyau@foxmail.com>

* Take action_server_reentrant_mutex_ out of the sending result loop

Signed-off-by: Kaven Yau <kavenyau@foxmail.com>

* add note for explaining the current locking order in server.cpp

Signed-off-by: Kaven Yau <kavenyau@foxmail.com>
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
…s-merge-master

Merged master on irobot/add-rmw-listener-apis
…cutor

Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
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.