* Add disable(enable)_callbacks() API to the subscriptions
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Add test coverage for the disable(enable)_calbacks()
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Address code review comments. Add missing includes in tests
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Don't trace callback in dispatch methods if callbacks are disabled
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Protect enable/disable callbacks and callbacks execution with mutex
Reasoning:
To avoid possible UB when callbacks disabled during callback
execution and callback handler object deleted while execution hasn't
been finished yet.
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Renames for shadowed callback_mutex_ from the EventHandlerBase
Note: We can't reuse `on_new_event_callback_mutex_` from the
EventHandlerBase because those mutex used to protect another type of
callbacks.
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Temporary removes the on_new_[message]event on disable_callbacks()
Note: While we are temporary removes the on new message callback and all
on new event callbacks from the underlying rmw layer to prevent them
from being called. We can't guarantee that the currently executing
on_new_[message]event callbacks are finished before we exit from the
disable_callbacks().
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Use recursive_mutex for callback lock in any_subscription_callback
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* Address CI warnings about usage of deprecated rclcpp::spin_some(node)
- Use spin_some() from the rclcpp::executors::SingleThreadedExecutor
directly.
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
---------
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
* add note indicating that spin_until_future_complete should be used only with futures generated by ROS entities
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* fix indentation and improve explanation of consequences
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
---------
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* change misleading warning message, making it more correct and informative
Signed-off-by: Peter Mitrano (AR) <peter.mitrano@agile-robots.com>
* Fix compile error. Needed to also build rcl from source.
Signed-off-by: Peter Mitrano (AR) <peter.mitrano@agile-robots.com>
* explicitely initialize pointer as null, to adhere to best practice
Signed-off-by: Peter Mitrano (AR) <peter.mitrano@agile-robots.com>
---------
Signed-off-by: Peter Mitrano (AR) <peter.mitrano@agile-robots.com>
The most important change in here is the changes to the package.xml
and the CMakeLists.txt, which now properly export the dependencies
to downstream packages as required. On those two fronts:
1. Make sure to add a dependency on rmw, which this package does depend
on for rmw_request_id_t
2. Make sure to add a dependency on rcl_interfaces, which this package
also depends on.
3. Export depend class_loader, composition_interfaces, rclcpp,
rcpputils, and rmw, all of which are exported in the header files.
4. Remove the unnecessary test dependencies on launch_testing and
std_msgs, neither of which is used.
The rest of the change here is to cleanup the header files to include
what you use everywhere.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Fix `start_type_description_service` param handling
Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>
* Add test
Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>
* Demonstrate different exceptions depending on node options
Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>
* Same exact exception and `what()` message in both cases
Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>
* Uncrustify
Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>
---------
Signed-off-by: Patrick Roncagliolo <ronca.pat@gmail.com>
* add parameter_overrides_with_parameter test.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* no need to template the function signature of append_parameter_override().
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
---------
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
With the similar rewrite coming in rcutils logging macros,
this is now required (since the old machinery relied on the
removed code in rcutils). Regardless, I think this is more
readable and maintainable.
Note that this does *not* remove the python3-empy dependency,
since that is still needed for other things.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
clang does not like :
__attribute__ ((visibility("default")))
[[deprecated("Don't like'")]]
while
[[deprecated("Don't like'")]]
__attribute__ ((visibility("default")))
is fine...
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* Add new interfaces to enable intropsection for action
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Correct some comments
Signed-off-by: Barry Xu <barry.xu@sony.com>
---------
Signed-off-by: Barry Xu <barry.xu@sony.com>
https://github.com/ros2/rclcpp/issues/2678 reported that the executor
was holding strong references to entities during execution. This commit
adds a regression test for this.
* fix(Executor): Don't hold strong references to entities during spin
This fixes a bug, were dropping an entity during a callback would
not prevent further callbacks. Note, there might still be a race
in the case of the mutithreaded executor.
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>
This fixes a potential race leading to a segfault. The segfault would
happen, if the tear down of the test would occur before the timer thread
stopped the spinning of the executor.
Also simplifies the test code, as the cancel of the executor is now in
the TearDown().
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* Fix transient local publish when inter and intra process communications are both present.
* Apply the fix to TypeAdapted signature
* Add an executor to intra_process_inter_process_mix_transient_local test case to enable inter process publishing test
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
For reasons I admit I do not understand, the deprecation
warnings for StaticSingleThreadedExecutor on Windows
happen when we construct a shared_ptr for it in the tests.
If we construct a regular object, then it is fine. Luckily
this test does not require a shared_ptr, so just make it
a regular object here, which rixes the warning.
While we are in here, make all of the tests camel case to
be consistent.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Make ament_cmake a buildtool dependency
The `ament_cmake` package isn't needed at runtime and so should only be
listed as a `buildtool_dependency`, as is done in most other packages.
* Remove the ament_cmake dependency entirely
Since there's already a buildtool dependency on ament_cmake_ros, having
ament_cmake as well is redundant.
Signed-off-by: Nathan Wiebe Neufeldt <nwiebeneufeldt@clearpath.ai>
* Omnibus fixes for running tests with Connext.
When running the tests with RTI Connext as the default
RMW, some of the tests are failing. There are three
different failures fixed here:
1. Setting the liveliness duration to a value smaller than
a microsecond causes Connext to throw an error. Set it to
a millisecond.
2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1.
Connext is somewhat slow in this regard, so it can be the case
that we are overwriting a previous service introspection event
with the next one. Switch to the ServicesDefaultQoS in the test,
which ensures we will not lose events.
3. Connext is slow to match publishers and subscriptions. Thus,
when creating a subscription "on-the-fly", we should wait for the
publisher to match it before expecting the subscription to actually
receive data from it.
With these fixes in place, the test_client_common, test_generic_service,
test_service_introspection, and test_executors tests all pass for
me with rmw_connextdds.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Fixes for executors.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* One more fix for services.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* More fixes for service_introspection.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* More fixes for introspection.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
---------
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
They are both legacy from times past, and are both
no longer serving their intended purposes. Just remove them.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Fix NodeOptions assignment operator
Also copy the enable_logger_service_ member during the assignment operation
* Add more checks for NodeOptions copy test
* Set non default values by avoiding the copy-assignement
Signed-off-by: Romain DESILLE <r.desille@gmail.com>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
* Cleanup of test_intra_process_manager.cpp
In particular, make every pub and sub have to
pass in both a topic name and a QoS when they
are constructing mock pubs and subs for the
intra-process manager test. This just makes it
easier to tell whether the pubs and subs should be
matched or not.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Make sure to check QoS compatibility in the intra_process_manager tests.
It turns out that the intra_process_manager will attempt to match
QoS between publishers and subscriptions as they are added to the IPM.
This is exactly correct, but the tests were not following the same
pattern.
Thus, when running these tests under Zenoh, the tests would fail
because more things would match than the tests were expecting.
Update the test to take the differences in QoS into account,
which fixes the test under rmw_zenoh_cpp (and keeps it working
for the existing DDS RMWs).
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Added feedback
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
---------
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
The function that may not complete successfully is
`rcl_lifecycle_state_fini()`, not `rcl_lifecycle_transition_fini()`.
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
The default is 300 seconds, but on Windows this is taking
between 250 and 300 seconds (I'm seeing it timeout sometimes).
Up the timeout to 600 seconds, which should be more than enough.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* LifecycleNode base class resource needs to be reset via dtor.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* add debug notice that prints LifecycleNode is not shutdown in dtor.
Currently it is user application responsibility to manage the all state control.
See more details for https://github.com/ros2/rclcpp/issues/2520.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* add test cases to call shutdown from each primary state.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* address review comments.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
---------
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* Implement generic service
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Add the required header files
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Fix compiling errors on Windows
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Fix compiling errors on Windows
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Fix compiling errors on Windows
Signed-off-by: Barry Xu <barry.xu@sony.com>
---------
Signed-off-by: Barry Xu <barry.xu@sony.com>
* add unit-test to verify that spin-all doesn't need warm-up
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* improve comment and add callback group test
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* move executor tests to a new file
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* do not require warm up iteration with events executor spin_some
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* add spin_some tests and cleanup
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* add missing include directives
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
---------
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* deprecate the static single threded executor
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* suppress deprecation warning for static executor and remove benchmark tests
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* fix uncrustify linter errors
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* fix windows deprecation warnings
i created an alias to give the deprecated executor a new name; this works when the class is directly used but it doesn't work in combination with templates (like std::make_shared<DeprecatedAlias>) because the compiler needs to resolved the type behind the alias triggering the warning
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* update test_reinitialized_timers.cpp to use the executor test utilities
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* do not use executor pointer
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
---------
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Co-authored-by: Alberto Soragna <asoragna@irobot.com>
The current
`TestLifecycleServiceClient.get_service_names_and_types_by_node` test
was using `LifecycleServiceClient`, which is just a normal
`rclcpp::Node` with some `rclcpp::Client`s, to test
`NodeGraph::get_service_names_and_types_by_node()`. However,
`NodeGraph::get_service_names_and_types_by_node()` is for service
servers, not clients. The test just ended up checking the built-in
services of an `rclcpp::Node`, since it wasn't actually checking the
names of the services, and not checking the clients of the
`LifecycleServiceClient` or the built-in services of a
`rclcpp_lifecycle::LifecycleNode`. I believe this was probably not the
intention, since this is an `rclcpp_lifecycle` test.
Instead, use an actual `rclcpp_lifecycle::LifecycleNode` and check its
service servers by calling
`NodeGraph::get_service_names_and_types_by_node()` through
`LifecycleNode::get_service_names_and_types_by_node()`.
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
We already do this clean-up if some other tasks below fail.
Before:
[ RUN ] TestUtilities.test_context_init_shutdown_fails
[ERROR] [1722555370.075637014] [rclcpp]: rcl context unexpectedly not shutdown during cleanup
[WARN] [1722555370.077175569] [rclcpp]: logging was initialized more than once
[ OK ] TestUtilities.test_context_init_shutdown_fails (3 ms)
After:
[ RUN ] TestUtilities.test_context_init_shutdown_fails
[WARN] [1722555108.693207861] [rclcpp]: logging was initialized more than once
[ OK ] TestUtilities.test_context_init_shutdown_fails (3 ms)
Also, remove an unnecessary line in `test_utilities`, and expect context
to not be valid if init fails.
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
* Make Waitable::{is_ready,add_to_wait_set,execute} abstract APIs.
I initially started looking at this because clang was complaining
that "all paths through this function will call itself". And it
is correct; if an implementation does not override these methods,
and they are every called, they will go into an infinite loop
calling themselves.
However, while looking at it it seemed to me that these were really
part of the API that a concrete implementation of Waitable needed
to implement. It is fine if an implementation doesn't want to do
anything (like the tests), but all of the "real" users in the codebase
override these.
Thus, remove the implementations here and make these pure virtual
functions that all subclasses must implement.
* Remove some more "empty" implementations.
In particular, these are implementations in the Waitable
class that only throw exceptions. Rather than do this,
make these APIs pure virtual so all downstream classes
have to implement them. All of the current users (except
for tests) do anyway, and it makes the API much cleaner.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Even when we want to run them on multiple RMWs, we can
do that by compiling once, then setting the environment
variable appropriately.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Make the subscriber_triggered_to_receive_message test more reliable.
In the current code, inside of the timer we create the subscription
and the publisher, publish immediately, and expect the subscription
to get it immediately. But it may be the case that discovery
hasn't even happened between the publisher and the subscription
by the time the publish call happens.
To make this more reliable, create the subscription and publish *before*
we ever create and spin on the timer. This at least gives 100
milliseconds for discovery to happen. That may not be quite enough
to make this reliable on all platforms, but in my local testing this
helps a lot. Prior to this change I can make this fail one out of 10
times, and after the change I've run 100 times with no failures.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* move notify waitable setup to its own function
* move mutex lock to retrieve_entity utility
* use entities_need_rebuild_ atomic bool in events-executors
* remove duplicated set_on_ready_callback for notify_waitable
* use mutex from base class rather than a new recursive mutex
* use current_collection_ member in events-executor
* delay adding notify waitable to collection
* postpone clearing the current collection
* commonize notify waitable and collection
* commonize add/remove node/cbg methods
* fix linter errors
---------
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* Fix the lifecycle tests on RHEL-9.
The full explanation is in the comment, but basically since
RHEL doesn't support mocking_utils::inject_on_return, we have
to split out certain tests to make sure resources within a
process don't collide.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
* Release ownership of entities after spinning cancelled
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Move release action to every exit point in different spin functions
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Move wait_result_.reset() before setting spinning to false
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Update test code
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Move test code to test_executors.cpp
Signed-off-by: Barry Xu <barry.xu@sony.com>
---------
Signed-off-by: Barry Xu <barry.xu@sony.com>
That's because it is too large for Windows Debug to compile,
so split into smaller bits.
Even with this split, the file is too big; that's likely
because we are using TYPED_TEST here, which generates multiple
symbols per test case. To deal with this, without further
breaking up the file, also add in the /bigobj flag when
compiling on Windows Debug.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Revert "LifecycleNode shutdown on dtor only with valid context. (#2545)"
This reverts commit d8d83a0ee6.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* Revert "call shutdown in LifecycleNode dtor to avoid leaving the device in unknown state (2nd) (#2528)"
This reverts commit 3bc364a10b.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
---------
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* Revert "Revert "call shutdown in LifecycleNode dtor to avoid leaving the device in un… (#2450)" (#2522)"
This reverts commit 42b0b5775b.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* lifecycle node dtor shutdown should be called only in primary state.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* adjust warning message if the node is still in transition state in dtor.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
---------
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* test(Executors): Added tests for busy waiting
Checks if executors are busy waiting while they should block
in spin_some or spin_all.
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* fix: Reworked spinAll test
This test was strange. It looked like, it assumed that spin_all did
not return instantly. Also it was racy, as the thread could terminate
instantly.
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* fix(Executor): Fixed spin_all not returning instantly is no work was available
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* Update rclcpp/test/rclcpp/executors/test_executors.cpp
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: jmachowinski <jmachowinski@users.noreply.github.com>
* test(executors): Added test for busy waiting while calling spin
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* fix(executor): Reset wait_result on every call to spin_some_impl
Before, the method would not recollect available work in case of
spin_some, spin_all. This would lead to the method behaving differently
than to what the documentation states.
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
* restore previous test logic for now
Signed-off-by: William Woodall <william@osrfoundation.org>
* refactor spin_some_impl's logic and improve busy wait tests
Signed-off-by: William Woodall <william@osrfoundation.org>
* added some more comments about the implementation
Signed-off-by: William Woodall <william@osrfoundation.org>
---------
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: jmachowinski <jmachowinski@users.noreply.github.com>
Signed-off-by: William Woodall <william@osrfoundation.org>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: jmachowinski <jmachowinski@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* feat(Client): Added function to stop callbacks of a goal handle
This function allows us to drop the handle in a locked context.
If we do not do this within a lock, there will be a race condition between
the deletion of the shared_ptr of the handle and the result / feedback
callbacks.
* fix: make Client goal handle recursive
This fixes deadlocks due to release of goal handles in callbacks etc.
* fix(ActionGoalClient): Fixed memory leak for nominal case
This fixes a memory leak due to a self reference in the ClientGoalHandle.
Note, this fix will only work, if the ClientGoalHandle ever receives
a result callback.
* doc: Updated documentation of rclcpp_action::Client::async_send_goal
* docs: Made the async_send_goal documentation more explicit
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Janosch Machowinski <J.Machowinski@cellumation.com>
When I added in the tests for large messages, I made a mistake and reserved space in the strings, but didn't actually expand it. Thus, we were writing into uninitialized memory. Fix this by just using the correct constructor for string, which will allocate and initialize the memory properly.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Signed-off-by: William Woodall <william@osrfoundation.org>
Signed-off-by: Janosch Machowinski <j.machowinski@nospam.org>
Co-authored-by: Janosch Machowinski <j.machowinski@nospam.org>
* call shutdown in LifecycleNode dtor to avoid leaving the device in unknown state.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* add test to verify LifecycleNode::shutdown is called on destructor.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
---------
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
the previous version of the test was relying on the assumption that a timer with 1ms period gets called at least 6 times if the main thread waits 15ms. this is true most of the times, but it's not guaranteed, especially when running the test on windows CI servers. the new version of the test makes no assumptions on how much time it takes for the timers manager to invoke the timers, but rather focuses on ensuring that they are called the right amount of times, which is what's important for the purpose of the test
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* refactor and improve the spin_some parameterized tests for executors
Signed-off-by: William Woodall <william@osrfoundation.org>
* disable spin_some_max_duration for the StaticSingleThreadedExecutor and EventsExecutor
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup and clarify the docstring for Executor::spin_some()
Signed-off-by: William Woodall <william@osrfoundation.org>
* style
Signed-off-by: William Woodall <william@osrfoundation.org>
* review comments
Signed-off-by: William Woodall <william@osrfoundation.org>
---------
Signed-off-by: William Woodall <william@osrfoundation.org>
* Add unit tests for hashing rclcpp_action::GoalUUID's
* Use the FNV-1a hash algorithm for Goal UUID
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Mostly by ensuring we aren't attempting to store
large messages on the stack. Also add in tests.
I verified that before these changes, the tests failed,
while after them they succeed.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Implement generic client
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Fix the incorrect parameter declaration
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Deleted copy constructor and assignment for FutureAndRequestId
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Update codes after rebase
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Address review comments
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Address review comments from iuhilnehc-ynos
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Correct an error in a description
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Fix window build errors
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Address review comments from William
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Add doc strings to create_generic_client
Signed-off-by: Barry Xu <barry.xu@sony.com>
---------
Signed-off-by: Barry Xu <barry.xu@sony.com>
The original reason is that on Windows Debug, we were
failing to compile because the compilation unit was
too large. But also this file was way too large (1200
lines), so it makes sense to split it up.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* fix
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* add timer cancel tests
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* cleanup header include
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* reverting change to timer_greater function
Signed-off-by: Gus Brigantino <gusbrig97@gmail.com>
* use std::optional, and handle edgecase of 1 cancelled timer
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* clean up run_timers func
Signed-off-by: Gus Brigantino <gusbrig97@gmail.com>
* some fixes and added tests for cancel then reset of timers.
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* refactor and clean up. remove cancelled timer tracking.
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* remove unused method for size()
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* linting
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* relax timing constraints in tests
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* further relax timing constraints to ensure windows tests are not flaky.
Signed-off-by: Matt Condino <mwcondino@gmail.com>
* use sim clock for tests, pub clock at .25 realtime rate.
Signed-off-by: Matt Condino <mwcondino@gmail.com>
---------
Signed-off-by: Matt Condino <mwcondino@gmail.com>
Signed-off-by: Gus Brigantino <gusbrig97@gmail.com>
Co-authored-by: Gus Brigantino <gusbrig97@gmail.com>
* Add intra process transient local durability support to publisher and subscription
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Remove durability_is_transient_local_ from publisher_base
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Design changes that move most transient local publish functionalities out of
intra process manager into intra process manager
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Move transient local publish to a separate function
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Remove publisher buffer weak ptr from intra process manager when it associated publisher is removed.
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Remove incorrectly placed RCLCPP_PUBLIC
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Add missing RCLCPP_PUBLIC
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Expand RingBufferImplementation beyond shared_ptr and unique_ptr
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Comment and format fix
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
---------
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Stop storing the context in the guard condition.
This was creating a circular reference between GuardCondition
and Context, so that Context would never be cleaned up.
Since we never really need the GuardCondition to know
about its own Context, remove that part of the circular
reference.
While we are in here, we also change the get_context()
lambda to a straight weak_ptr; there is no reason for the
indirection since the context for the guard condition
cannot change at runtime.
We also remove the deprecated version of the
get_notify_guard_condition(). That's because there is
no way to properly implement it in the new scheme, and
it seems to be unused outside of rclcpp.
Finally, we add in a test that guarantees the use_count
is what we expect when inside and leaving a scope, ensuring
that contexts will properly be destroyed.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* make type support helper supported for service and action as well
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* not to use template and only add the necessary service type currently
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update comment
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add deprecated cycle for `get_typesupport_handle`
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
---------
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* aligh with rcl
* keep same behavior with rclpy
1. to not throw exception durning rcl_logging_rosout_remove_sublogger
2. reset error message for RCL_RET_NOT_FOUND
* test for a node with rosout disabled
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Both the EventHandler and its associated pubs/subs share
the same underlying rmw event listener.
When a pub/sub is destroyed, the listener is destroyed.
There is a data race when the ~EventHandlerBase wants
to access the listener after it has been destroyed.
The EventHandler stores a shared_ptr of its associated pub/sub.
But since we were clearing the listener event callbacks on the
base class destructor ~EventHandlerBase, the pub/sub was
already destroyed, which means the rmw event listener was also
destroyed, thus causing a segfault when trying to obtain it
to clear the callbacks.
Clearing the callbacks on ~EventHandler instead of
~EventHandlerBase avoids the race, since the pub/sub are still valid.
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Support users holding onto shared pointers in the message memory pool
Before this commit, the MessageMemoryPool would actually
reuse messages in the pool, even if the user had taken
additional shared_ptr copies.
This commit fixes things so that we properly handle that situation. In particular,
we allocate memory during class initialization, and delete
it during destruction. We then run the constructor when
we hand the pointer out, and the destructor (only) when
we return it to the pool. This keeps things consistent.
We also add in locks, since in a multi-threaded scenario we need
to protect against multiple threads accessing the pool
at the same time.
With this in place, things work as expected when users hold
shared_ptr copies. We also add in a test for this situation.
One note about performance: this update preserves the
"no-allocations-at-runtime" aspect of the MessagePool. However,
there are some tradeoffs with CPU time here, particularly with
very large message pools. This could probably be optimized
further to do less work when trying to add items back to the
free_list, but I view that as a further enhancement.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Add a custom deleter when constructing rcl_service_t
In the type description service construction, we were previously passing
the shared_ptr to the rcl_service_t with the assumption that
rclcpp::Service would do the clean up. This was an incorrect
assumption, and so I have added a custom deleter to fini the service and
delete when the shared_ptr is cleaned up.
Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
* Disable the loaned messages inside the executor.
They are currently unsafe to use; see the comment in the
commit for more information.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Use message_info in SubscriptionTopicStatistics instead of typed message
- Untemplatize the rclcpp::topic_statistics::SubscriptionTopicStatistics
class. Now we will be using message_info instead of typed deserialized
messages in the handle_message callbacks.
* Fix test_receive_stats_include_window_reset by using publisher emulator
- Emulate publishing messages by directly calling
rclcpp::Subscription::handle_message(msg_shared_ptr, message_info) and
settling up needed message_info.source_timestamp
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
1. Remove the default Logger copy constructor without copy
assignment (rule of three -> rule of zero).
2. Remove an unnecessary capture in a lambda.
3. Mark a variable unused.
4. Mark a method as override.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
The comment in the commit explains this clearly, but
on Windows ERROR is a macro. The reuse of it, even
as an enum, causes compilation errors on downstream
users. Push the macro and undefine it so downstream
consumers can freely include it.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
We need this because it is possible for one thread to
be handling the on_parameter_event callback while another
one is detaching the node. This lock will protect that
from happening.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
In a recent commit (bc435776a2),
we reworked how the Rate class worked so it could be
used with ROS time as well. Unfortunately, we also
accidentally broke the API of it by changing the return
type of Rate::period to a Duration instead of a
std::chrono::nanoseconds . Put this back to a std::chrono::nanoseconds;
if we want to change it to a Duration, we'll have to
add a new method and deprecate this one.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* fix(ClientGoalHandle): Made mutex recursive to prevent deadlocks
This prevents deadlocks in cases, were e.g. get_status() would
be called on the handle in a callback of the handle.
* test(rclcpp_action): Added test for deadlocks during access of a goal handle
This test checks, if the code deadlocks, if methods on the goal handle are
called from the callbacks.
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* Cleanup flaky timers_manager tests.
The timers_manager tests were relying too heavily on
specific timings; this caused them to be flaky on the
buildfarm, particularly on Windows.
Here, we increase the timeouts, and remove one test which
just relies too heavily on specific timeouts. This should
make this test much less flaky on the buildfarm.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Make Rate to select the clock to work with
Add ROSRate respective with ROS time
* Make GenericRate class to be deprecated
* Adjust test cases for new rates
is_steady() to be deprecated
Signed-off-by: Alexey Merzlyakov <alexey.merzlyakov@samsung.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
* Remove unnecessary lambda captures in the tests.
This was pointed out by compiling with clang.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
It is depended on by rclcpp/src/rclcpp/logger.cpp, but
the dependency was not explicitly declared (it was
being inherited from rcl, I believe). Fix that here.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
1. Use constref for the loop variable.
2. Do more work outside of the loop.
3. Skip doing unnecessary work where we can inside the loop.
With this in place, I measured about a 7% performance
improvement over the previous implementation.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Do not crash Executor when send_response fails due to client failure.
Related to https://github.com/ros2/ros2/issues/1253
It is not sane that a faulty client can crash our service Executor, as
discussed in the referred issue, if the client is not setup properly,
send_response may return RCL_RET_TIMEOUT, we should not throw an error
in this case.
Signed-off-by: Zang MingJie <zealot0630@gmail.com>
* Update rclcpp/include/rclcpp/service.hpp
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Zang MingJie <zealot0630@gmail.com>
* address review comments.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
---------
Signed-off-by: Zang MingJie <zealot0630@gmail.com>
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Zang MingJie <zealot0630@gmail.com>
* Stop using constref signature of benchmark DoNotOptimize.
Newer versions of google benchmark (1.8.2 in my case) warn
that the compiler may optimize away the DoNotOptimize calls
when using the constref version. Get away from that here
by explicitly *not* calling the constref version, casting
where necessary.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* TypeDescriptions interface with readonly param configuration
* Add parameter descriptor, to make read only
* example of spinning in thread for get_type_description service
* Add a basic test for the new interface
* Fix tests with new parameter
* Add comments about builtin parameters
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: William Woodall <william@osrfoundation.org>
Since this is a common idiom, especially under this name, we should
define the `always_false_v` template within a namespace to avoid
conflict with other libraries and user code. This could either be
`rclcpp::detail` if it's intended only for internal use or just `rclcpp`
if it's intended as a public helper. In this PR, I've initially chosen
the former.
Signed-off-by: Nathan Wiebe Neufeldt <nwiebeneufeldt@clearpath.ai>
The original motiviation to do this was a crash during
teardown when using a newer version of gtest. But while
I was in here, I did a small overall cleanup, including:
1. Moving code closer to where it is actually used.
2. Getting rid of unused 'using' statements.
3. Adding in missing includes.
4. Properly tearing down and recreating the rclcpp
context during test teardown (this fixed the actual
bug).
5. Making class members private where possible.
6. Renaming class methods to our usual conventions.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Modifies timers API to select autostart state
* Removes unnecessary variables
* Adds autostart documentation and expands some timer test
Signed-off-by: Voldivh <eloyabmfcv@gmail.com>
* Enable callback group tests for connextdds
* Enable executors and event executor tests for connextdds
* Enable qos events tests for connextdds
* Less flaky qos_event tests
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
* added available_capacity to get the lowest number of free capacity for intra-process communication for a publisher
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* added unit tests for available_capacity
Signed-off-by: Joshua Hampp <j.hampp@eu.denso.com>
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* fixed typos in comments
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* Updated warning
Co-authored-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* returning 0 if ipm is disabled in lowest_available_ipm_capacity
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* return 0 if no subscribers are present in lowest_available_capacity
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* updated unit test
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* update unit test
Signed-off-by: Joshua Hampp <j.hampp@eu.denso.com>
* moved available_capacity to a lambda function to be able to handle subscriptions which went out of scope
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
* updated unit test to check subscriptions which went out of scope
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
---------
Signed-off-by: Joshua Hampp <j.hampp@denso-adas.de>
Signed-off-by: Joshua Hampp <j.hampp@eu.denso.com>
Co-authored-by: Joshua Hampp <j.hampp@denso-adas.de>
Co-authored-by: Joshua Hampp <j.hampp@eu.denso.com>
Co-authored-by: Alberto Soragna <alberto.soragna@gmail.com>
* add mutex to protect events_executor current entity collection and unit-test
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* be more precise with mutex locks; make stress test less stressfull
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* fix uncrustify error
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
---------
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
This is to ensure callbacks are destroyed last
on entities destruction, avoiding the gap in time
in which rmw entities hold a reference to a
destroyed function.
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Co-authored-by: Mauro Passerino <mpasserino@irobot.com>
The initial implementation of the events-executor contained a bug where the executor
would end up in an inconsistent state and stop processing interrupt/shutdown notifications.
Manually adding a node to the executor results in a) producing a notify waitable event
and b) refreshing the executor collections.
The inconsistent state would happen if the event was processed before the collections were
finished to be refreshed: the executor would pick up the event but be unable to process it.
This would leave the `notify_waitable_event_pushed_` flag to true, preventing additional
notify waitable events to be pushed.
The behavior is observable only under heavy load.
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
In particular, you should never have a "bare" string in a
printf-like call; that could potentially access uninitialized
memory. Instead, make sure to format the string with %s.
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
* Add support for logging service.
* Update to not modify interfaces and not change time_source
* Use unique_ptr for NodeBuiltinExecutorImpl
* Use user thread to run logger service
* Update code for lifecycle_node
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Lei Liu <Lei.Liu.AP@sony.com>
* add events-executor and timers-manager in rclcpp
fix check for execute_timers_separate_thread; ignore on_ready_callback if asked to execute a timer; reduce usage of void pointers
* have the executor notify waitable fiddle with guard condition callbacks only if necessary
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* Deprecate callback_group call taking context
* Add base executor objects that can be used by implementors
* Template common operations
* Add callback to EntitiesCollector constructor
* Make function to check automatically added callback groups take a list
* Make executor own the notify waitable
* Add pending queue to collector, remove from waitable
Also change node's get_guard_condition to return shared_ptr
* Change interrupt guard condition to shared_ptr
Check if guard condition is valid before adding it to the waitable
* Make get_notify_guard_condition follow API tick-tock
* Improve callback group tick-tocking
* Add thread safety annotations and make locks consistent
* Remove the "add_valid_node" API
* Only notify if the trigger condition is valid
* Only trigger if valid and needed
Signed-off-by: Michael Carroll <michael@openrobotics.org>
applied tracepoints for intra_publish
add tracepoints for linking buffer and subscription
Signed-off-by: Kodai Yamasaki <114902604+ymski@users.noreply.github.com>
If the intraprocess buffer still has data after taking, re-trigger the
guard condition to ensure that the executor will continue to service it,
even if incoming publications stop.
Signed-off-by: Michael Carroll <michael@openrobotics.org>
This does 2 separate things:
* Adds (void)unused_variable things where needed.
* Stops doing some checks on moved-from items in tests.
With this in place, most of the remaining clang static analysis
warnings are gone.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Rename QOSEventHandler* to EventHandler.
We are going to be using it for more than just QOS events, so
rename it to just EventHandler and EventHandlerBase for now.
Leave the old names in place but deprecated.
* Rename qos_event.hpp -> event_handler.hpp
* Revamp incompatible_qos callback setting.
* Add in incompatible type implementation.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
The main reason to do this is so that we can compile rclcpp
with the clang static analyzer. As of clang++-14 (what is in
Ubuntu 22.04), the default still seems to be C++14, so we need
to specify C++17 so that new things in the rclcpp headers work
properly.
Further, due to reasons I don't fully understand, I needed to
set CMAKE_CXX_STANDARD_REQUIRED in order for clang to really use
that version. So set this as well.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Implementation of service introspection.
To do this, we add a new method on the Client and
Service classes that allows the user to change the
introspection method at runtime. These end up calling
into the rcl layer to do the actual configuration,
at which point service introspection messages will be
sent as configured.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Brian Chen <brian.chen@openrobotics.org>
That is, in cache_last_msg(), we were just taking a shared_ptr
reference. While this is pretty fast, it also means that
any changes to that message would be reflected internally.
Instead, make a new shared pointer out of that message when
it comes in, which essentially causes this to be a copy.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
This eliminates a potential hang when the isolated container is being
shutdown via the rclcpp SignalHandler.
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
* Fix -Wmaybe-uninitialized warning
gcc 12 warned about `callback_list_ptr` potentially being uninitialized
when compiling in release mode (i.e., with `-O2`). Since `shutdown_type`
is a compile-time parameter, we fix the warning by enforcing the
decision at compile time.
Signed-off-by: Alexander Hans <ahans@users.noreply.github.com>
If the user creates SystemDefaultsQoS setting, they are
explicitly asking for SystemDefaults. In that case, we should
*not* warn, and just let the underlying RMW choose what it
wants. Implement that here by passing a boolean parameter
through that decides when we should print out the warning,
and then skip printing that warning when SystemDefaultsQoS
is created.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Add in a fix for older compilers.
The addition of the NodeInterfaces class made it stop compiling
with older compilers (such as gcc 9.4.0 on Ubuntu 20.04).
The error has to do with calling the copy constructor on
rclcpp::Node, which is deleted. Work around this by removing
the NodeInterfaces shared_ptr constructor, which isn't technically
needed.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* add clock type to node_options and change node/lifecycle_node constructor accordingly
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Modify TimeSource::NodeState class to work with different clock types. Add test cases.
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Change on_parameter_event to output RCLCPP_ERROR and check
ros_time_active_ in ClocksState::attachClock()
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Remove a redundant include
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Disallow setting use_sim_time to true if a clock_type can't support it.
Following the reasoning in c54a6f1cd2, on_set_parameters doesn't try to enforce use_sim_time to be of boolean type explicitly.
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* minior style change
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* remove reason string for successful result
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
Signed-off-by: Jeffery Hsu <jefferyyjhsu@gmail.com>
* Add in a warning for a KeepLast depth of 0.
It really doesn't make much sense to have a KeepLast depth of 0;
no data could possibly be stored. Indeed, the underlying DDS
implementations don't actually support this. It currently "works"
because a KeepLast depth of 0 is assumed to be system default,
so the RMW layer typically chooses "1" instead. But this isn't
something we should be encouraging users to do, so add a warning.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* fix a case that not throw ParameterUninitializedException
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* catch ParameterUninitializedException exception while calling get_parameters in service callback
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update doc
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add one more test
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* explicitly use this to call a method inside the class itself
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* protect state_machine_ with mutex lock.
protect state_handle_ with mutex lock.
reconsider mutex lock scope.
remove mutex lock from constructors.
lock just once during initialization of LifecycleNodeInterfaceImpl.
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
* Move updating of current_state to right after initialization.
This is slightly more correct in the case that registering one
of the services fails.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
* Add a test case
a subscriber on a new executor with a callback group triggered to receive a message
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* fix flaky and not to use spin_some
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update comment
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update for not using anti-pattern source code
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add a notify guard condition for callback group
Co-authored-by: William Woodall <william@osrfoundation.org>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* Notify guard condition of Node not to be used in Executor
it is only for the waitset of GraphListener
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* put code in the try catch
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* defer to create guard condition
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* use context directly for the create function
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* cpplint
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* fix that some case might call add_node after shutdown
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* nitpick and fix some potential bug
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add sanity check as some case might not create notify guard condition after shutdown
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* Cleanup includes.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* remove destroy method
make a callback group can only create one guard condition
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* remove limitation that guard condition can not be re-created in callback group
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Co-authored-by: William Woodall <william@osrfoundation.org>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
We should only need to update the current state when it changes,
so we do that in the change_state method (which is not const).
Then we can just return the current_state_ object in
get_current_state, and then mark it as const.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Split lifecycle_node_interface_impl into header and implementation.
There is no reason it should all be in the header file. No
functional change.
* Mark LifecycleNodeInterfaceImpl as final.
* Update documentation about return codes.
* Mark a bunch of LifecycleNodeInterfaceImpl methods as const.
* Make most of LifecycleNodeInterfaceImpl private.
* Mark some LifecycleNode methods as const.
* Disable copies on LifecycleNodeInterfaceImpl.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
That is, make sure they are all listed in package.xml, found
in the CMakeLists.txt, and properly included where they are
used.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Do not clear entities callbacks on destruction
Removing these clearings since they were not necessary,
since the objects are being destroyed anyway.
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Fix CI
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Restore clear_on_ready_callback on ~QOSEventHandlerBase
Needed since QOSEventHandlerBase does not own
the pub/sub listeners. So the QOSEventHandler
can be destroyed while the corresponding listeners
are still alive, so we need to clear these callbacks.
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Add coment on clearing callback for QoS event
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Co-authored-by: Mauro Passerino <mpasserino@irobot.com>
* Make ParameterService and Sync/AsyncParameterClient accept rclcpp::QoS
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Avoid deprecated function using another deprecated function
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
clarify documentation to show that not capturing returned handles
will result in callbacks immediately being unregistered
Signed-off-by: Jason Beach <jason.m.beach@gmail.com>
* Support add_pre_set_parameter and add_post_set_parameter callbacks in addition to add_on_set_parameter_callback in Node API
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
That is, it returns false if shutdown has been called, and
true in all other circumstances.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Fix subscription.is_serialized() for callbacks with message info argument
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Add tests + please linters
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
In the pull request https://github.com/ros2/rclcpp/pull/1442 the ability
to use `std::string` as the first argument to the logging functions was
(rightfully) removed.
This commit just corrects the documentation of the macro, since it
confused me a bit ;)
Signed-off-by: Daniel Reuter <daniel.robin.reuter@googlemail.com>
* 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>
* to support a feature of content filtered topic
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* update for checking memory allocated
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* set expression parameter only if filter is valid
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add test
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* use a default empty value for expresion parameters
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* remove std_msgs dependency
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* use new rcl interface
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* remove unused include header files and fix unscrutify
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update comment
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update comment
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* rename
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* refactor test
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* relate to `rcutils_string_array_t expression_parameters` changed in rmw
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* remove the implementation temporary, add them with fallback in the feature
1. add DEFINE_CONTENT_FILTER with default(ON) to build content filter interfaces
2. user need a compile option(CONTENT_FILTER_ENABLE) to turn on the feature
to set the filter or call the interfaces
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update comments and check the option to build content filter test
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add DDS content filter implementation
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* address review
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* rcl api changed
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* increase the filter propagation time
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* add rclcpp::ContentFilterOptions and use it as the return type of get_content_filter
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* increase the maximun time to wait message event and content filter propagation
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* cft member should be defined in the structure.
to check the macro for interfaces is enough.
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* set test timeout to 120
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update doc
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* [NodeParameters] Set name if user didn't populate
If the user provided parameter descriptor already contains a name, then
we should not override said name, under the good faith assumption that
the user knows what they are doing.
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
This package uses rcl_interfaces directly, and we've been getting away
with the missing dependency because `rcl` has the entirety of
`rcl_interfaces` as part of its link interface even tough it doesn't
need to.
If (when) `rcl` scopes its dependency to only the c generator and drops
the cpp generator from its link interface, this package will fail to
find the cpp symbols at link time. This change remedies that.
Signed-off-by: Scott K Logan <logans@cottsay.net>
* mark some single-argument constructors explicit that should have been
Signed-off-by: William Woodall <william@osrfoundation.org>
* suppress clang-tidy [google-explicit-constructor] in some cases where it is a false-positive
Signed-off-by: William Woodall <william@osrfoundation.org>
* Revert "suppress clang-tidy [google-explicit-constructor] in some cases where it is a false-positive"
This reverts commit eb6bf7e2df0dd27c945e97584ba9902ef9f61305.
Signed-off-by: William Woodall <william@osrfoundation.org>
* Fix a bunch more rosdoc2 issues in rclcpp.
There are a smattering of problems in here:
1. We weren't properly using PREDEFINE for all of our macros.
2. The Doxyfiles were referencing tag files that may not exist
(this will be handled by rosdoc2 automatically).
3. The C++ parser in doxygen can't handle "friend classname",
but can handle "friend class classname"; it shouldn't matter
one way or the other to the compiler.
4. There were a couple of parameters that were not documented.
5. The doxygen C++ parser can't handle decltype in all situations.
6. There was a structure using a C-style declaration.
This patch fixes all of the above issues. We still aren't totally
clean on a rosdoc2 build, but we are a lot closer.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Use a single call to collect all CallbackGroup pointers.
Believe it or not, the taking and releasing of mutex_ within
the CallbackGroup class shows up in profiles. However, when
collecting entities we really don't need to take it and drop
it between each of subscriptions, services, clients, timers,
and waitables. Just take it once at the beginning, collect
everything, and then return, which removes a lot of unnecessary
work with that mutex.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Pass shared_ptr as constref in the MemoryStrategy class.
That way, in the case that the callee doesn't need to take
a reference, we avoid creating and destroying a shared_ptr
class. This reduces the overhead in using these functions,
and seems to work fine in the default case. If a user wants
to subclass the MemoryStrategy class, then they can explicitly
create a shared_ptr copy by using the copy constructor.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Use constref shared_ptr when iterating.
Believe it or not, the creation and destruction of the
shared_ptr class itself shows up in profiles in these loops.
Since we don't need to hold onto a reference while iterating
(the class is already doing that), just use constref wherever
we can which drastically reduces the overhead.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* replace spin_until_future_complete with spin in component_manager_isolated.hpp
spin_until_future_complete() is more inefficient as it needs to check the state of the future and the timeout after every work iteration
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* fix uncrustify error
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* Add return value version of get_parameter_or
* Add test case
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
Co-authored-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
* Small cleanups in TimeSource.
Simplify some code, and also make sure to throw an exception
when use_sim_time is not of type bool. Also add a test for
the latter.
* Remove the sim_time_cb_handler.
It was originally used to make sure that someone didn't change
the 'use_sim_time' type from boolean to something else. But
since the parameters interface now does that automatically for
us, we don't need the explicit check here.
I can think of one situation that this allows that wasn't allowed
before. If the user defined 'use_sim_time' as a parameter override
when constructing a node, and the type is bool, and they also
defined the parameters as 'dynamic_typing', then this callback
will still have effect. But presumably if the user went out of
their way to change the parameter to dynamic_typing, they are
trying to do something esoteric and so we should just let them.
* ClocksState does not need to be a pointer.
Instead, make it a regular member variable. That lets us get
rid of all of the special handling for when it is a weak_ptr
or not. It's lifetime is now just that of NodeState.
* Stop using NodeState as a weak_ptr in the captures.
This ended up causing the lifetime of the object to be weird.
Instead, just capture 'this' which is sufficient.
* Make sure destroy_clock_sub is first.
* Switch to using just a regular object.
Windows objects to trying to do "make_shared" in the RCLCPP
macro, so just switch to a normal object here.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* add use_global_arguments for node options of component nodes
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
keep use_global_arguments false default
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
update warning message
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
update warnning message
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
add test
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
* use forward_global_arguments instead of use_global_arguments
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
* Automatically transition LifecyclePublisher(s) between activated and inactive
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Fix: Add created publishers to the managed entities vector
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* enabled_ -> activated_
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Continue setting should_log_ as before
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Fix visibility attributes so it works on Windows
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Cleanup test_publisher_with_type_adapter.
It had a lot of infrastructure that it didn't need, so remove
most of that. Also move the creation of the node to open-coded,
since we weren't really saving anything. Finally make sure
to reset the node pointers as appropriate, which cleans up
errant error messages.
* Cleanup test_subscription_with_type_adapter.
It had a lot of infrastructure that it didn't need, so remove
most of that. Also move the creation of the node to open-coded,
since we weren't really saving anything. Finally make sure
to reset the node pointers as appropriate, which cleans up
errant error messages.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Remove ones that aren't needed, and add in ones that are
actually needed in the respective files.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
That is, make it so that if both a publisher and a subscriber using intra-process
and using TypeAdaptation are the same type, we don't need to convert to
a ROS Message type on the publisher and from a ROS Message type before
delivering to the subscriber. Instead, we store the original message type
in the subscription buffer, and deliver it straight through to the subscriber.
This has two main benefits:
1. It is better for performance; we are eliding unnecessary work.
2. We can use TypeAdaptation to pass custom handles (like hardware accelerator
handles) between a publisher and subscriber. That means we can avoid
unnecessary overhead when using a hardware accelerator, like synchronizing
between the hardware accelerator and the main CPU.
The main way this is accomplished is by causing the SubscriptionIntraProcessBuffer
to store messages of the Subscribed Type (which in the case of a ROS message type
subscriber is a ROS message type, but in the case of a TypeAdapter type is the
custom type). When a publish with a Type Adapted type happens, it delays conversion
of the message, and passes the message type from the user down into the IntraProcessManager.
The IntraProcessManager checks to see if it can cast the SubscriptionBuffer
to the custom type being published first. If it can, then it knows it can deliver the message directly
into the SubscriptionBuffer with no conversion necessary. If it can't, it then sees if
the SubscriptionBuffer is of a ROS message type. If it is, then it performs the conversion
as necessary, and then stores it. In all cases, the Subscription is then triggered underneath
so that the message will eventually be delivered by an executor.
We also add tests to ensure that in the case where the publisher and subscriber
and using the same type adapter, no conversion happens when delivering the
message.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
* Update Doxyfile
* Update docblocks with warnings
* Use leading * instead of trailing [] for pointer types
* Help Doxygen parse std::enable_if<> condition
* Add documentation-only SFINAE functions' declarations
* Avoid function pointer type syntax in function arguments.
* Add documentation-only SFINAE function traits.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Add wait_for_all_acked support
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Update the description of wait_for_all_acked
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Use rcpputils::convert_to_nanoseconds() for time conversion
Signed-off-by: Barry Xu <barry.xu@sony.com>
* Use rclcpp::GuardCondition
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Pass GuardCondition ptr instead of ref to remove_guard_condition
Before the api was taking a reference to a guard condition,
then getting the address of it. But if a node had expired,
we can't get the orig gc dereferencing a pointer,
nor can we get an address of an out-of-scope guard condition.
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Address PR comments
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Co-authored-by: Mauro Passerino <mpasserino@irobot.com>
* Use parantheses around logging macro parameter
This allows the macro to expand "correctly", i.e. macro argument
expression is fully evaluated before use.
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
* Remove redundant parantheses around macro param
`decltype(X)` already provides sufficient "scoping" to the macro
parameter `X`.
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
* Add test case for expressions as logging param
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
* Revert "Revert "Add Clock::sleep_until method (#1748)" (#1793)"
This reverts commit d04319a438.
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Context, Shutdown Callback, Condition Var per call
The `Clock` doesn't have enough information to know which Context should
wake it on shutdown, so this adds a Context as an argument to
sleep_until().
Since the context is per call, the shutdown callback is also registered
per call and cannot be stored on impl_.
The condition_variable is also unique per call to reduce spurious
wakeups when multiple threads sleep on the same clock.
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Throw if until has wrong clock type
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* unnecessary newline
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Shorten line
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Fix time jump thresholds and add ROS time test
Use -1 and 1 thresholds because 0 and 0 is supposed to disable the
callbacks
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Shorten line
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* rclcpp::ok() -> context->is_valid()
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* No pre-jump handler instead of noop handler
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* If ros_time_is_active errors, let it throw
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Get time source change from callback to avoid race if ROS time toggled quickly
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Fix threshold and no pre-jump callback
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Use RCUTILS_MS_TO_NS macro
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Explicit cast for duration to system time
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Throws at the end of docblock
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Add tests for invalid and non-global contexts
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Fix lifetime of context so it remains alive while its dependent node handles are still in use
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Explicitly delete moving and assigning
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Satisfy uncrustify
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Fix more spacing complaints
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Fixing style
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Provide a safe move constructor to avoid double-allocation
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Fix uncrustify
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
* Take message ownership from moved LoanedMessage
Otherwise, since the rvalue reference passed into the move
constructor of `LoanedMessage` was not set to `nullptr`, the
underlying message would be double-free'd.
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
* Add unit test for LoanedMessage move construction
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
* Include <utility> header to satisfy cpplint
`std::move` was being used without including said header,
which made cpplint unhappy.
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
clang static analysis complains that there are dead stores in
most of the benchmark tests, which is technically correct.
We use an idiom like:
for (auto _ : state) {
}
And never access _. Silence clang here by doing (void)_;
all of the places this is seen.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Fix#1775
Connext takes significantly longer for discovery to happened compared to the other RMWs.
So, waiting an arbitrary amount of time for a message to be received is brittle.
By instead waiting for the publisher and subscription to match, the tests become more robust.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Convert internal state into shareable structs
* Add documentation
* PIMPL the class
* Use a weak_ptr to avoid a potential dangling reference
* Comply with the rule of 5
Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>
* fixup some small things that cppcheck noticed in my editor
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix implementation too
Signed-off-by: William Woodall <william@osrfoundation.org>
* Add Clock::sleep_until method
Handle all 3 clock types, and edge cases for shutdown and enabling/disabling ROS time
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
The documentation for std::string::front() says that calling it
on an empty string is undefined behavior. It actually seems to
work on all platforms except Windows Debug, where it throws an
error. Make sure to check for empty first.
We also notice that there is no reason to check the
existing_sub_namespace for empty; the rest of the code handles
that fine. So remove that check.
Finally, we add an anonymous namespace so that these functions do
not pollute the global namespace.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Deprecated `shared_ptr<MessageT>` sub callbacks
Addresses #1619.
Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
* Resolve deprecated subscription callbacks in tests
Specifically, `void shared_ptr<MessageT>` subscription callbacks have
been migrated to `void shared_ptr<const MessageT>` subscription
callbacks.
This change has been performed only on the test files that do
not actually house unit tests for the `AnySubscriptionCallback` class.
For unit tests that actually target the deprecated `set` functions,
the deprecation warnings have to be avoided. This patch will be
introduced in a separate commit.
Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
* Suppress deprecation warnings in unit tests
This commit specifically introduces suppression of the deprecation
warnings produced while compiling unit tests for the
`AnySubscriptionCallback` class.
The macro mechanics to conditionally include the `deprecated` attribute
is not ideal, but the diagnostic pragma solution (`# pragma GCC
diagnostic ignored`) did not work for these unit tests, possibly because
of the way gtest is initializing the necessary `InstanceContext`
objects.
A `TODO` directive has been left to figure out a better way to address
this warning suppression.
Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
* Fix shared ptr callback in wait_for_message
Moving away from deprecated signatures.
Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
* `rclcpp_action`: Fix deprecated subscr. callbacks
Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
* `rclcpp_lifecycle`: Fix deprecated sub callbacks
Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
* Revert "Revert "Updating client API to be able to remove pending requests (#1728)" (#1733)"
This reverts commit d5f3d35fbe.
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Address peer review comments
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Fix tests in rclcpp_components, rclcpp_lifecycle
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Change log level for lifecycle_publisher
De-activating a lifecycle publisher while the function that was invoking `publish` is still running floods the log of useless warning messages.
This requires to add a boolean check around the publish call, thus making useless the choice of a lifecycle publisher
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* change lifecycle publisher to log warning only once per transition
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* rework lifecycle publisher log mechanism to use an helper function
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
* change doxygen format to use implicit brief
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Co-authored-by: Alberto Soragna <asoragna@irobot.com>
* Use FindPython3 and make python3 dependency explicit
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Need CMake 3.12 for FindPython3
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Create valid effective namespace when sub-namespace is empty
Fix#1656.
Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
* Add regression test for effective namespace and empty sub-namespace
Adds regression test for #1656.
Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
Callers should change to using for_each_callback_group(), or
store the callback groups they need internally.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
The main reason to add this method in is to make accesses to the
callback_groups_ vector thread-safe. By having a
callback_groups_for_each that accepts a std::function, we can
just have the callers give us the callback they are interested
in, and we can take care of the locking.
The rest of this fairly large PR is cleaning up all of the places
that use get_callback_groups() to instead use
callback_groups_for_each().
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* to support declare parameter with int vector
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* update for float vector
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* adding node name to the executor runtime_error
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
* changing get_name() to get_fully_qualified_name() + linting
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
* changing print format for tests
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
* 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>
* Declare parameters uninitialized
Fixes#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>
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>
* 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 4585605223639bbd9d18053e5ef39725638512b4.
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>
Keep a rebound allocator for byte-sized memory blocks around
for publisher and subscription options.
Follow-up after 1fc2d58799
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* 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>
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>
* spin_some/spin_all/spin_once support: static executor
Signed-off-by: Mauro <mpasserino@irobot.com>
* Use spin_once instead of spin_once_impl
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Fix memory leak
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* revert spinSome change
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
* Override spin_once_impl only
This way the StaticSingleThreadedExecutor uses spin_once and
spin_until_future_complete APIs from the base executor class,
but uses its own overrided version of spin_once_impl.
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Co-authored-by: Mauro <mpasserino@irobot.com>
* refactor AnySubscriptionCallback to...
use std::variant and make the dispatch functions constexpr,
avoiding runtime dispatching.
Also, deprecate the std::function<void (std::shared_ptr<MessageT>)> signature,
as it is unsafe to share one shared_ptr when multiple subscriptions take it,
because they could mutate MessageT while others are using it. So you'd have to
make a copy for each subscription, which is no different than the
std::unique_ptr<MessageT> signature or the user making their own copy in a
shared_ptr from the const MessageT & signature or the
std::shared_ptr<const MessageT> signature.
Added a std::function<void (const std::shared_ptr<const MessageT> &)> signature
to go along side the existing
std::function<void (std::shared_ptr<const MessageT>)> signature.
Removed redundant 'const' before pass-by-value signatures, e.g.
std::function<void (const shared_ptr<const MessageT>)> became
std::function<void (shared_ptr<const MessageT>)>.
This will not affect API or any users using the old style.
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix use of std::bind, free functions, etc. using new function_traits::as_std_function<>
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix use of const MessageT & callbacks by fixing subscriptin_traits
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix deprecation warnings
Signed-off-by: William Woodall <william@osrfoundation.org>
* use target_compile_features to require c++17 for downstream users of rclcpp
Signed-off-by: William Woodall <william@osrfoundation.org>
* uncrustify
Signed-off-by: William Woodall <william@osrfoundation.org>
* cpplint
Signed-off-by: William Woodall <william@osrfoundation.org>
* use target_compile_features(... cxx_std_17)
Signed-off-by: William Woodall <william@osrfoundation.org>
* Keep both std::shared_ptr<const MessageT> and const std::shared_ptr<const MessageT> & signatures.
The const std::shared_ptr<const MessageT> & signature is being kept because it
can be more flexible and efficient than std::shared_ptr<const MessageT>, but
costs realtively little to support.
The std::shared_ptr<const MessageT> signature is being kept because we want to
avoid deprecating it and causing disruption, and because it is convenient to
write, and in most cases will not materially impact the performance.
Signed-off-by: William Woodall <william@osrfoundation.org>
* defer deprecation of the shared_ptr<MessageT> sub callbacks
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix unused variable warning
Signed-off-by: William Woodall <william@osrfoundation.org>
* small fixups to AnySubscriptionCallback
Signed-off-by: William Woodall <william@osrfoundation.org>
* add check for unset AnySubscriptionCallback in dispatch methods
Signed-off-by: William Woodall <william@osrfoundation.org>
* update dispatch methods to handle all scenarios
Signed-off-by: William Woodall <william@osrfoundation.org>
* updated tests for AnySubscriptionCallback, include full parameterized i/o matrix
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup test with changed assumption
Signed-off-by: William Woodall <william@osrfoundation.org>
* remove use of std::unary_function, which was removed in c++17
Signed-off-by: William Woodall <william@osrfoundation.org>
* silence c++17 warnings on windows for now
Signed-off-by: William Woodall <william@osrfoundation.org>
* Copying files from rosbag2
The generic_* files are from rosbag2_transport
typesupport_helpers incl. test is from rosbag2_cpp
memory_management.hpp is from rosbag2_test_common
test_pubsub.cpp was renamed from test_rosbag2_node.cpp from rosbag2_transport
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Rebrand into rclcpp_generic
Add package.xml, CMakeLists.txt, Doxyfile, README.md and CHANGELOG.rst
Rename namespaces
Make GenericPublisher and GenericSubscription self-contained by storing shared library
New create() methods that return shared pointers
Add docstrings
Include only what is needed
Make linters & tests pass
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Review feedback
* Delete CHANGELOG.rst
* Enable cppcheck
* Remove all references to rosbag2/ros2bag
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Move rclpp_generic into rclcpp
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Rename namespace rclcpp_generic to rclcpp::generic
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Free 'create' functions instead of static functions in class
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Remove 'generic' subdirectory and namespace hierarchy
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Order includes according to style guide
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Remove extra README.md
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Also add brief to class docs
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Make ament_index_cpp a build_depend
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Add to rclcpp.hpp
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Remove memory_management, use rclcpp::SerializedMessage in GenericPublisher::publish
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Clean up the typesupport_helpers
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Use make_shared, add UnimplementedError
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Add more comments, make member variable private, remove unnecessary include
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Apply suggestions from code review
Co-authored-by: William Woodall <william+github@osrfoundation.org>
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Rename test
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Update copyright and remove ament_target_dependencies for test
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Accept PublisherOptions and SubscriptionOptions
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Remove target_include_directories
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Add explanatory comment to SubscriptionBase
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Use kSolibPrefix and kSolibExtension from rcpputils
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Fix downstream build failure by making ament_index_cpp a build_export_depend
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Use path_for_library(), fix documentation nitpicks
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Improve error handling in get_typesupport_handle
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Accept SubscriptionOptions in GenericSubscription
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Make use of PublisherOptions in GenericPublisher
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Document typesupport_helpers
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Improve documentation
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Use std::function instead of function pointer
Co-authored-by: William Woodall <william+github@osrfoundation.org>
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Minimize vertical whitespace
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Add TODO for callback with message info
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Link issue in TODO
Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
* Add missing include for functional
Signed-off-by: nnmm <nnmmgit@gmail.com>
* Fix compilation
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Fix lint
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Address review comments (#1)
* fix redefinition of default template arguments
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* address review comments
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* rename test executable
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* add functionality to lifecycle nodes
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* Refactor typesupport helpers
* Make extract_type_identifier function private
* Remove unused extract_type_and_package function
* Update unit tests
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Remove note about ament from classes
This comment only applies to the free functions.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Fix formatting
Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com>
* Fix warning
Possible loss of data from double to rcutils_duration_value_t
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add missing visibility macros
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Co-authored-by: William Woodall <william+github@osrfoundation.org>
Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com>
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>
* Add API for checking QoS profile compatibility
Depends on https://github.com/ros2/rmw/pull/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>
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>
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>
* Replace stale reference to Connext
* Restore exceptions for ros2/rmw_connext to ease transition to rticommunity/rmw_connextdds
Signed-off-by: Andrea Sorbini <asorbini@rti.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Also add the following fixes for CI:
* Fix symbol visibility error on Windows
* Remove an unused parameter to quiet a clang-tidy warning on MacOS
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
When building rclcpp under clang static analysis, it complains
that the "DoNotOptimize" function from Google benchmark can
cause a memory leak. I can't see how this is possible, so I
can only assume that the inline assembly that is used to implement
"DoNotOptimize" is causing a false positive. Just quite the
warning here when building under clang static analysis.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* add ParameterEventsSubscriber class and tests
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* test improvements, path name fixes, and more documentation
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* fix lint and uncrustify issues
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* add try-catch and warning around getting parameter value
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* pass rclcpp::Parameter object to callback, rename functions, get param from event
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* use unordered map with string pair, add test for different nodes same parameter, address feedback
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* address feedback (wjwwood) part 1
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
use const string & for node name
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* add RCLCPP_PUBLIC macro
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* address feedback part 1: static get_parameter method, remove register_parameter_update, mutex for thread-safety
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* map parameters to list of callbacks
functions to remove parameter callbacks
add functions to remove event callbacks, remove subscriptions, allow subscribing event callback to many namespaces, additional test coverage
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* use absolute parameter event topic for parameter event subscriber
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* support multiple parameter event callbacks
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* use get_child for parameter event subscriber logger
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* update utility function description
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* address feedback: move HandleCompare, test exceptions, reference code source
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* address feedback: replace ParameterEventsFilter dependency, fix copyright, add get_node_logging_interface, modify constructor
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
* Get rid of a few compiler warnings; add test to build
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Remove some unneeded code
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Remove a stray debug trace
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Add a function to get all parameter values from a parameter event
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Address code review feedback
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Another name change; using Handler instead of the more passive term, Monitor
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Pass SharedPtrs callback remove functions instead of bare pointers
Per William's review feedback.
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Add a comment block describing usage
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Address review feedback
* Remove unused interfaces
* Document LIFO order for invoking callbacks
* Add test cases to verify LIFO order for callbacks
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* A couple more doc fixes from review comments
Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
Co-authored-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
* Throw an exception when declaring a parameter of a specific type and passing a descriptor with dynamic typing enabled.
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* added tear-down of node sub-interfaces in reverse order of their creation (#1469)
Signed-off-by: Colin MacKenzie <colin@flyingeinstein.com>
* added name of service to log message for leak detection. Previously it gave no indication of what node is causing the memory leak (#1469)
Signed-off-by: Colin MacKenzie <colin@flyingeinstein.com>
When running the rclcpp tests under UBSAN, it complained that
the AnySubscriptionCallback tests were using an uninitialized
allocator.
Reviewing the code there, it also looks like the AnySubscriptionCallback
constructor wasn't checking for a null allocator.
Fix this by doing 3 things:
1. Add a check for a null allocator in the AnySubscriptionCallback
constructor.
2. Add a new test to test_any_subscription_callback that tests the new
nullptr handling.
3. Fix the test fixture to initialize the allocator before trying to
call the AnySubscriptionCallback constructor.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
The CMake 'install' command should never be used to copy content from
the source tree to the build directory like this. The destination
directory is affected by the 'DESTDIR' variable on UNIX platforms, which
will get prefixed onto the destination directory and we'll end up
installing the test resources into the target install space.
This change will eliminate the excess test collateral present in the
rclcpp deb packages being installed to '/tmp/binarydeb'.
Signed-off-by: Scott K Logan <logans@cottsay.net>
If we just EXPECT it, then we continue running tests below
and eventually dereference an empty list.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
According to the documentation at
https://en.cppreference.com/w/cpp/utility/move,
the "moved-from" objects are "in a valid but unspecified
state" after the move. Thus, we shouldn't assume anything
about them, since it could be implementation-defined behavior.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Destroy msg extracted from LoanedMessage.
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* Remove the release method of LoadedMessage and a related test case
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* Revert "Remove the release method of LoadedMessage and a related test case"
This reverts commit b9825251d148198cb63dc841139e88e77ac02aff.
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* Use unique_ptr as return type for release method of LoanedMessage
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* Update based on review.
Co-authored-by: William Woodall <william@osrfoundation.org>
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* Update description for the release method
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* update based on suggestion and revert some changes.
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
* Use explicit capture
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
* Use unique_ptr as argument type and update exist test
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Co-authored-by: William Woodall <william@osrfoundation.org>
Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Fix#1526.
As action_server_ depends on clock_, we declare clock_ first.
Then the order of deletion becomes action_server_, clock_.
Signed-off-by: y-okumura-isp <y-okumura@isp.co.jp>
* Add comments about declaration order (#1526)
Signed-off-by: y-okumura-isp <y-okumura@isp.co.jp>
* Change to using unique_ptrs for DummyExecutor.
clang static analysis suggested that there was a possible memory
leak here, and it is right. The test is expecting to throw during
the constructor, in which case the memory would be automatically
freed. However, if the test did *not* throw for some reason,
we would leak the memory. Switch to using a unique_ptr here
which will free the memory in all cases at the end of the scope.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* unlock action_server_reentrant_mutex_ before calling user callback functions
add an additional lock to keep previous behavior broken by deadlock fix
Also add a test case to reproduce deadlock situation in rclcpp_action
Signed-off-by: Daisuke Sato <daisukes@cmu.edu>
The graph_listener thread is started in the background in some of the tests and
this thread is killed by Windows prior to executing global destructors if it is
still running when leaving main(). This then corrupts state because the RMW
layer is blocking in a waitset and causes Cyclone to hang trying to destroy the
waitset.
Signed-off-by: Erik Boasson <eb@ilities.com>
In particular, we don't know for sure that the graph will only
have 1 user; we just know that it should have *at least* one
user. So change that check to be more robust. This also
lets us get rid of the slightly strange call to 'notify_graph_change'
that was being used to cleanup graph users.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Make sure to wait for graph change events in test_node_graph.
While debugging something else, I came across this fairly rare
flake in the test_node_graph tests. Essentially, it may take
some time for node changes to propagate into the graph. If
the NodeGraph client asks for data before the changes are
in the graph, they may get something they don't expect. The
fix is to wait for an event on the NodeGraph to happen, and
then look for the data you are interested in.
Before this change, I could get the flake to happen on a loaded
aarch64 system fairly regularly. After this change, I can no
longer make the flake happen on a loaded system.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* add timeout to SyncParametersClient methods
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
* update parameter client test with timeout.
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
* use template thin interface to keep the implementations in library.
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
* test duration should be long enough not to detect unnecessary timeout
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
* Change nullptr checks to use ASSERT_TRUE.
clang static analysis gets a bit confused going through the
gtest macros, so switch from ASSERT_NE to ASSERT_TRUE as
we've done elsewhere.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* add LifecycleNode::get_transition_graph along with service.
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
* add concrete test cases for get_available_transitions with each primary state.
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Note that these tests are written without using
performance_test_fixture. Because the parameter server is running in the
same process, any allocations happening in the spin thread for the
server get picked up by the allocation statistics even though those
functions aren't invoked in the tests.
If we can find a way to turn off the memory tracking on a per-thread
basis, we can enable memory tracking. Until then, leaving the memory
statistics enabled could be misleading.
Signed-off-by: Scott K Logan <logans@cottsay.net>
* Reserve vector capacities and use emplace_back for constructing vectors
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Use resize instead of reserve
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove push_back
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Change uint8_t iterator variables to size_t
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Change to unsigned int
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Refactor graph listener tests to work on CentOS.
inject_on_return doesn't work on CentOS. To fix this, we
do two separate things:
1. Where applicable, replace calls to inject_on_return with
patch_and_return (which does work).
2. We were sort of abusing inject_on_return to do partial
initialization for us for some of the tests. Instead, make
the class under test (GraphListener) have a protected method
that we can call to do initialization. With this in place,
we can now get rid of the problematic inject_on_return.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Older versions of MSVC 2019 can't figure out the correct namespace.
Just to keep them happy, add a fully-qualified namespace.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Deprecate Duration(rcl_duration_value_t) in favor of static Duration::from_nanoseconds(rcl_duration_value_t)
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Add service and client benchmarks
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Style
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Uncrustify
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Set CMakeLists to only use default rmw for benchmarks
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add comment
Signed-off-by: Stephen Brawner <brawner@gmail.com>
This patch actually does 4 related things:
1. Renames the recursive mutex in the ServerBaseImpl class
to action_server_reentrant_mutex_, which makes it a lot
clearer what it is meant to lock.
2. Adds some additional error checking where checks were missed.
3. Adds a lock to publish_status so that the action_server
structure is protected.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Increase test timeouts of slow running tests with rmw_connext_cpp
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fix other issues with connext
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add unit tests for qos and qos_event files
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fix windows CI
Signed-off-by: Stephen Brawner <brawner@gmail.com>
In particular, add API coverage for spin_node_until_future_complete,
spin_until_future_complete, and spin_node_once.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Add test for ParameterService
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Change value used as max representation
* Add coverage tests time
* Add call to detach clock
* Add tests time
* Add duration construction tests
* Add const qualifier to constants
* Add check clock stays the same
* Make operator RCLCPP_PUBLIC
* Add tests exceptions duration
* Fix division by 0 on windows
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
* Test the remaining node public API
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add comment
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Complete coverage of Parameter and ParameterValue API
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Adding comments
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add tests for node_options API
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove c-style casts
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Make sure that spin_until_future_complete throws an exception
if we are already spinning.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Add coverage for client API
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
We need to determine if this is a new node *before* adding
it to the list; otherwise, it will always be treated as an old
node.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
While we are here, add in another test for the stream operator for future_return_code.cpp
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Increase coverage of publisher/subscription API
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add coverage for missing API (except executors
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Do not check state
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Increase coverage of node_interfaces, including with mocking rcl errors
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Increase coverage rclcpp_action to 95%
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Rebase onto #1311
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* rcutils test depend
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Cleaning up
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Pass goal handle to goal response callback instead of a future
This resolves an issue where `promise->set_value` is called before a potential call to `promise->set_exception`.
If there is an issue sending a result request, set the exception on the future to the result in the goal handle, instead of the future to the goal handle itself.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Do not remove goal handle from list if result request fails
This way the user can still interact with the goal (e.g. send a cancel request).
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Set the result awareness to false if goal handle is invalidated
This will cause an exception when trying to get the future to result, in addition to the exception when trying to access values for existing references to the future.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Revert "Set the result awareness to false if goal handle is invalidated"
This reverts commit d444e09131c42d6eece1338443b8ffb4f5f17370.
* Throw from Client::async_get_result if the goal handle was invalidated due to a failed result request
Propagate error message from a failed result request.
Also set result awareness to false if the result request fails so the user can also check before
being hit with an exception.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Guard against mutliple calls to invalidate
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add mocking utils for rclcpp
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add coverage for wait_set_policies
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fix windows issues
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add test comment
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Increase test coverage of rclcpp_lifecycle to 96%
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* test_depend
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* rcutils test_depend
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* More windows warnings
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add tests getters msg type support
* Add missing fini
* Add tests type_support services
* Reformat to re use test structure
* Remove not needed headers
* Improve teardown test cases
* Add nullptr checks to type_support tests
* Reformat type_support testing
* Replace expect tests with asserts
* "Improve error msg for rcl_service_ini/fini fail"
* Improve test readability
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
Without this change, I am unable to build locally.
std_msgs is not declared as a test dependency or find_package'd anywhere, so
I'm not sure why CI ever passed the build phase.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Catch potential exception in destructor and log
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove thrown error from reset and mark it no except
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove unused private function
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove group_in_node from rclcpp::Node
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Increase timeouts for connext for long tests
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fix cmakelists
Signed-off-by: Stephen Brawner <brawner@gmail.com>
It turns out rmw_connext_cpp adds a default waitable that other rmw
implementations do not. Adjusting the unit test to take this into
account in a non-rmw specific manner.
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* fix failing test with Connext since it doesn't wait for discovery
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
* Check for added service in the node graph
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Co-authored-by: Stephen Brawner <brawner@gmail.com>
If the user doesn't retain a reference to the returned shared pointer there will be zero references and their callback will be unregistered.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Unit tests for memory_strategy classes (part 1)
Adds unit tests for:
* strategies/message_pool_memory_strategy.hpp
* memory_strategy.cpp
* message_memory_strategy.cpp
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Update with new macros
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* EXPECT_THROW_EQ and ASSERT_THROW_EQ macros for unittests
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Address PR Feedback
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add unit test for static_executor_entities_collector
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Relocate test_executor to executors directory
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Parametrize test_executors for all executor types
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* More fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Adding issue for tracking
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Unit tests for allocator_memory_strategy.hpp
Part 1 of 2 for this file, but part 2 of 3 for memory strategies
overall
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove find_package
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Remove ref to osrf_testing_tools
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Derive and throw exception in spin_some spin_all
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fix style and add unit test
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fix conversion from negative Duration or Time to the respective message type and throw in Duration::to_rmw_time() if the duration is negative.
rmw_time_t cannot represent negative durations.
Constructors and assignment operators can be just defaulted.
Other changes are mainly cosmetical, to make conversions between signed
and unsigned types and between 32-bit and 64-bit types more explicit.
Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
* Add -Wconversion compiler option and fix implicit conversions that might alter the value
Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
* Fix usage of fixture class in some unit tests by using gtest macro TEST_F() instead of TEST().
Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
* Add compiler option -Wno-sign-conversion to fix build with Clang on macOS
Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
`this->node_options_` might still be `nullptr` for a default initialized NodeOptions instance.
`use_global_arguments()` must return `this->use_global_arguments_`, in analogy to `NodeOptions::enable_rosout()`.
Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
* Check period duration in create_wall_timer
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Adding comments
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add -Wnon-virtual-dtor -Woverloaded-virtual compiler options
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Add missing virtual dtors
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* please linter
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
This keeps it from going out of scope while the executor
is still dealing with QoSEvents.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* [rclcpp_action] Action client holds weak pointers to goal handles
Fixes#861
It is against the design of ROS actions to rely on the status topic for the core implementation,
instead it should just be used for introspection.
Rather than relying on the status topic to remove references to goal handles, the action client
instead holds weak pointers to the goal handles. This way as long as a user holds a reference to
the goal handle they can use it to interact with the action client.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Move cleanup logic to the end of the function
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add TODO
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Log debug messages when dropping a weak references to goal handles
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Improve documentation
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Fixes https://github.com/ros2/rclcpp/issues/955
There are currently two public APIs for users to get the result of a goal.
This change deprecates one of the APIs, which was considered to be unsafe as
it may result in a race with user-code and raise an exception.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Increasing test coverage of rclcpp_components
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Removing throws test for now
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add received message age metric to topic statistics
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Add unit tests
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Add IMU messages in unit test
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Use system time instead of steady time
Test received message age stats values are greater than 0
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Fix test warnings
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Replace IMU messages with new dummy messages
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Remove outdated TODO and unused test variables
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Address review comments
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Address review comments
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Re-add message with header for unit testing
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Address message review feedback
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Remove extra newline
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
* Address more review feedback
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Fix Windows failure
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Only set append_library_dirs once
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
* Add InvalidParameterTypeException
Used to wrap the ParameterTypeException coming from ParameterValue::get() for improving the error message.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Describe new exception
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Update tests
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Added static single threaded executor functionality
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
executor enhanced to run clients and waitable
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
tested executor
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
added semi-dynamic feature to the executor
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
Jenkins error fixes
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
Added static single threaded executor functionality
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
* Added semi-dynamic feature and made changes based on review comments
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
* re-added accidentally deleted code in node.hpp, fixed static_single_threaded_executor.cpp w.r.t. intra-process change since last commit
Signed-off-by: MartinCornelis2 <martin.cornelis@nobleo.nl>
* Remove not needed comparison
wait_set_.size_of_* is always different than '0'
if we are inside the for loop
Signed-off-by: Mauro <mpasserino@irobot.com>
* If new entity added to a node: re-collect entities
Now we check ONLY node guard_conditions_
Some possible guard conditions to be triggered HERE are:
1. Ctrl+C guard condition
2. Executor interrupt_guard_condition_
3. Node guard_conditions_
4. Waitables guard conditions
5. ..more
The previous approach was only checking if NOT (1 & 2),
so if a Waitable was triggered, it would re-collect all
entities, even if no new node entity was added. This was the case
of the intra process manager, who relies on waitables.
Every time a subscriber got a message, all the entities
were collected.
Signed-off-by: Mauro <mpasserino@irobot.com>
* Implement static executor entities collector
Signed-off-by: Mauro <mpasserino@irobot.com>
* fixup and style
Signed-off-by: William Woodall <william@osrfoundation.org>
* mark new classes as final, with non-virtual destructors
Signed-off-by: William Woodall <william@osrfoundation.org>
* adding copyright to static executor files
Signed-off-by: Ishu Goel <ishu.goel@nobleo.nl>
* fixup
Signed-off-by: William Woodall <william@osrfoundation.org>
* cpplint fixes
Signed-off-by: William Woodall <william@osrfoundation.org>
Co-authored-by: Ishu Goel <ishu.goel@nobleo.nl>
Co-authored-by: MartinCornelis2 <martin.cornelis@nobleo.nl>
Co-authored-by: Mauro <mpasserino@irobot.com>
Some headers were being included even though they are not required and other headers were being included transitively.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
It was complaining about an unknown macro RCLCPP_SMART_PTR_DEFINITIONS.
Passing rclcpp include directories to cppcheck resolves the errors
reported in rclcpp_action and rclcpp_lifecycle.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
This fixes a cppcheck error that was detected when including the rclcpp headers in rclcpp_action and rclcpp_lifecycle.
It is not clear to me why cppcheck does not report the unitialized member when testing rclcpp directly.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Switch to using new rcutils_strerror.
Also increase timeouts for test_logging, which should reduce flakes on Windows.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Capturing a cached reference allows a clock object that is not a local
(e.g. the one returned by Node::get_clock()) to be passed to the throttle
logging macro.
Signed-off-by: Matt Schickler <mschickler@gmail.com>
Co-Authored-By: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
The function was previously documented as being deprecated, but this change adds compiler warnings if it is used.
Ignore compiler warnings where the function is being tested and change to the preferred usage elsewhere.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Fix for a build error on 32-bit Windows. Member functions use the
__thiscall convention by default which is incompatible with __cdecl.
Signed-off-by: Sean Kelly <sean@seankelly.dev>
* Assigning make_shared result to variables in test
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* PR fixup
Signed-off-by: Stephen Brawner <brawner@gmail.com>
* Add disable_rosout into NodeOptions.
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
* Update comments
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
* keep implementation consistency by using enable_rosout name
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
* fix error comment
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
* add test case for node options
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
* fix source about copy value and reset rcl_node_options, add more test cases
Signed-off-by: Barry Xu <Barry.Xu@sony.com>
To prevent the Executor::spin_some() method for potentially running
indefinitely long.
Distribution Statement A; OPSEC #2893
Signed-off-by: Roger Strain <rstrain@swri.org>
Whenever a call is made to `rclcpp_action::Client::wait_for_action_server`
a weak pointer to an Event object gets added to the graph_event_ vector
of the NodeGraph interface. This vector will be cleared on a node graph
change event, but if no such event occurs the weak pointer will be stuck
in the vector. Furthermore, if client code issues repeated calls to
`wait_for_action_server` the vector will keep growing.
The fix moves the Event object creation right after the early return from
`wait_for_action_server` so that the Event object is not created in the
case that the server is known to be present and therefore there is no
need to wait for a node graph change event to occur.
Signed-off-by: Adrian Stere <astere@clearpath.ai>
* Put subscription callback tracepoints in intraprocess subscription too
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
* Add missing tracetools header
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
* Move Subscription tracepoints back to outside of intra-process condition
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
* Revert API change by adding new tracepoint as an intermediate
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
* fix up documentation for zero copy api
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
* rename loan_message to borrow_loaned_message
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
* use return_loaned_message_from
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
* basic ipc implementation from alsora/new_ipc_proposal
Signed-off-by: alberto <alberto.soragna@gmail.com>
better use of node_topic create subscription
Signed-off-by: alberto <alberto.soragna@gmail.com>
added intra process manager test
Signed-off-by: alberto <alberto.soragna@gmail.com>
fixed ring buffer and added test
Signed-off-by: alberto <alberto.soragna@gmail.com>
added intra process buffer test
Signed-off-by: alberto <alberto.soragna@gmail.com>
added intra process buffer test
Signed-off-by: alberto <alberto.soragna@gmail.com>
Signed-off-by: alberto <alberto.soragna@gmail.com>
removed intra-process methods from subscription base
Signed-off-by: alberto <alberto.soragna@gmail.com>
using lock_guard instead of unique_lock, renamed var without camel case
Signed-off-by: alberto <alberto.soragna@gmail.com>
using unordered set and references in intra process manager
Signed-off-by: alberto <alberto.soragna@gmail.com>
subscription intra-process does not depend anymore on subscription, but has a copy of the callback
Signed-off-by: alberto <alberto.soragna@gmail.com>
changed buffer API to use rvo
Signed-off-by: Alberto <alberto.soragna@gmail.com>
avoid copying shared_ptr
Signed-off-by: alberto <alberto.soragna@gmail.com>
revert not needed changes to create_subscription
Signed-off-by: alberto <alberto.soragna@gmail.com>
updated tests according to new buffer APIs
Signed-off-by: alberto <alberto.soragna@gmail.com>
updated types in ring buffer implementation avoid using uint32_t
Signed-off-by: alberto <alberto.soragna@gmail.com>
using unique ptr for buffers in subscription_intra_process
Signed-off-by: alberto <alberto.soragna@gmail.com>
added missing std::move in subscription_intra_process constructor
Signed-off-by: alberto <alberto.soragna@gmail.com>
use consisting names for ring_buffer_implementation members
Signed-off-by: alberto <alberto.soragna@gmail.com>
addressing typos, one-liners and similar from ivanpauno review
Signed-off-by: alberto <alberto.soragna@gmail.com>
moved subscription_intra_process_base to its own files and moved non templated method from derived class
Signed-off-by: alberto <alberto.soragna@gmail.com>
removed forward declarations, fixed include subscription_intra_process_base
Signed-off-by: alberto <alberto.soragna@gmail.com>
removed member variable from do_intra_process_publish signature
Signed-off-by: alberto <alberto.soragna@gmail.com>
declare public before private in intra_process_manager_impl
Signed-off-by: alberto <alberto.soragna@gmail.com>
made matches_any_intra_process_publishers const
Signed-off-by: alberto <alberto.soragna@gmail.com>
using const reference in get_all_matching_publishers
Signed-off-by: alberto <alberto.soragna@gmail.com>
added deleter and alloc templates in intra_process_buffer
Signed-off-by: alberto <alberto.soragna@gmail.com>
added RCLCPP_WARN to intra_process_manager_impl
Signed-off-by: alberto <alberto.soragna@gmail.com>
passing context from node to subscription_intra_process
Signed-off-by: alberto <alberto.soragna@gmail.com>
using allocators in intra_process_manager
Signed-off-by: alberto <alberto.soragna@gmail.com>
use size_t instead of int in ring buffer indices
Signed-off-by: alberto <alberto.soragna@gmail.com>
creating buffer inside subscription_intra_process constructor
Signed-off-by: alberto <alberto.soragna@gmail.com>
fix lint errors
Signed-off-by: alberto <alberto.soragna@gmail.com>
throw error if trying to dequeue when buffer empty; remove duplicated methods in intra_process_buffer
Signed-off-by: alberto <alberto.soragna@gmail.com>
added todo for creating an rmw function for checking qos compatibility
Signed-off-by: alberto <alberto.soragna@gmail.com>
test fixes
Signed-off-by: alberto <alberto.soragna@gmail.com>
refactored intra_process_manager, removed ipm impl
Signed-off-by: alberto <alberto.soragna@gmail.com>
added mutex in intra_process_manager add_* methods
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
added allocator to intra_process_buffer
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
added invalid intra_process qos test for subscription
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
throw error if history size is 0 with keep last and ipc
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
using allocator when creating unique_ptr from shared_ptr
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
adding deleter template argument to intra_process buffer
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
fix linter
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
throw error with callbackT different from messageT
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
updated deleter template argument in subscription factory
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
Fix typo in test fixture tear down method name (#787)
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Add free function for creating service clients (#788)
Equivalent to the free function for creating a service.
Resolves#768
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Cmake infrastructure for creating components (#784)
*cmake macro to create components for libraries with multiple nodes
Signed-off-by: Siddharth Kucheria <kucheria@usc.edu>
Allow registering multiple on_parameters_set_callback (#772)
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
fix for multiple nodes not being recognized (#790)
Signed-off-by: Siddharth Kucheria <kucheria@usc.edu>
Remove non-package from ament_target_dependencies() (#793)
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
fix linter issue (#795)
Signed-off-by: Siddharth Kucheria <kucheria@usc.edu>
Make TimeSource ignore use_sim_time events coming from other nodes. (#799)
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
passing deleter template parameter
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
small fixes for failing tests
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
fixed imports in test_intra_process_manager
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
using RCLCPP_SMART_PTR_ALIASES_ONLY and RCLCPP_PUBLIC macros
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
added RCLCPP_PUBLIC macros and virtual destructor to sub intra_process base
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
added unique_ptr alias to macros
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
updated test_intra_process_manager.cpp
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
remove mock msgs from rclcpp (#800)
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Add line break after first open paren in multiline function call (#785)
* Add line break after first open paren in multiline function call
as per developer guide:
https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#open-versus-cuddled-braces
see https://github.com/ament/ament_lint/pull/148
Signed-off-by: Dan Rose <dan@digilabs.io>
Fix dedent when first function argument starts with a brace
Signed-off-by: Dan Rose <dan@digilabs.io>
Line break with multiline if condition
Remove line breaks where allowed.
Signed-off-by: Dan Rose <dan@digilabs.io>
Fixup after rebase
Signed-off-by: Dan Rose <dan@digilabs.io>
Fixup again after reverting indent_paren_open_brace
Signed-off-by: Dan Rose <dan@digilabs.io>
* Revert comment spacing change, condense some lines
Signed-off-by: Dan Rose <dan@digilabs.io>
Adapt to '--ros-args ... [--]'-based ROS args extraction (#816)
* Use --ros-args to deal with node arguments in rclcpp.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Document implicit --ros-args flag in NodeOptions::arguments().
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Add missing size_t to int cast.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Only add implicit --ros-args flag if not present already.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Add some rclcpp::NodeOptions test coverage.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Address peer review comments.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Please cpplint and uncrustify.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Guard against making multiple result requests for a goal handle (#808)
This fixes a runtime error caused by a race condition when making consecutive requests for the
result.
Specifically, this happens if the user provides a result callback when sending a goal and then
calls async_get_result shortly after.
Resolves#783
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Explain return value of spin_until_future_complete (#792)
Signed-off-by: Dan Rose <dan@digilabs.io>
Allow passing logger by const ref (#820)
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Delete unnecessary call for get_node_by_group (#823)
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Fix get_node_interfaces functions taking a pointer (#821)
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
add callback group as member variable and constructor arg (#811)
Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu>
remove callback group as member variable
Wrap documentation examples in code blocks (#830)
This makes the code examples easier to read in the generated documentation.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Crash in callback group pointer vector iterator (#814)
Signed-off-by: Guillaume Autran <gautran@clearpath.ai>
add mutex in add/remove_node and wait_for_work to protect concurrent use/change of memory_strategy_ (#837)
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Fix hang with timers in MultiThreadedExecutor (#835) (#836)
Signed-off-by: Todd Malsbary <todd.malsbary@intel.com>
Use of -r/--remap flags where appropriate. (#834)
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Force explicit --ros-args in NodeOptions::arguments(). (#845)
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Fail on invalid and unknown ROS specific arguments (#842)
* Fail on invalid and unknown ROS specific arguments.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Revert changes to utilities.hpp in rclcpp
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Fully revert change to utilities.hpp
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Fix typo in deprecated warning. (#848)
"it's" instead of its
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Add throwing parameter name if parameter is not set (#833)
* added throwing parameter name if parameter is not set
Signed-off-by: Alex <cvbn127@gmail.com>
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
check valid timer handler 1st to reduce the time window for scan. (#841)
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
remove features and related code which were deprecated in dashing (#852)
Signed-off-by: William Woodall <william@osrfoundation.org>
reset error message before setting a new one, embed the original one (#854)
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
restored virtual destructor in publisher_base
Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com>
* fixup a few things after rebase
Signed-off-by: William Woodall <william@osrfoundation.org>
* refactor some API's and get code compiling again
Signed-off-by: William Woodall <william@osrfoundation.org>
* docs and style changes (whitespace)
Signed-off-by: William Woodall <william@osrfoundation.org>
* move new intra process internals into experimental namespace
Signed-off-by: William Woodall <william@osrfoundation.org>
* uncrustify
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix issues with LoanedMessages after rebase
Signed-off-by: William Woodall <william@osrfoundation.org>
* more fixups
Signed-off-by: William Woodall <william@osrfoundation.org>
* readd logic for avoiding in compatible QoS
Signed-off-by: William Woodall <william@osrfoundation.org>
* avoid an error when intra process is disabled
Signed-off-by: William Woodall <william@osrfoundation.org>
* change intra process to preserve pointer in cyclic_pipeline
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix issue matching topics in intra process
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix some issues with the tests after latest behavior change
Signed-off-by: William Woodall <william@osrfoundation.org>
* address review feedback
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix the initialization order
Signed-off-by: William Woodall <william@osrfoundation.org>
* avoid possible loss of data warning
Signed-off-by: William Woodall <william@osrfoundation.org>
* more fixes related to initialization
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix use of custom allocators
Signed-off-by: William Woodall <william@osrfoundation.org>
As documented in rcl_action, a return code of RCL_RET_ACTION_CLIENT_TAKE_FAILED does not mean that
an error occurred.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* add mechanism to pass rmw impl specific payloads during pub/sub creation
Signed-off-by: William Woodall <william@osrfoundation.org>
* use class instead of struct
Signed-off-by: William Woodall <william@osrfoundation.org>
* narrow API of rmw payload to just use rmw_*_options_t's
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup after recent change
Signed-off-by: William Woodall <william@osrfoundation.org>
* make get_actual_qos return a rclcpp::QoS
Signed-off-by: William Woodall <william@osrfoundation.org>
* make simpler following suggestion from review
Signed-off-by: William Woodall <william@osrfoundation.org>
* streamline creation of publishers after removing deprecated API
Signed-off-by: William Woodall <william@osrfoundation.org>
* use deduced template arguments to cleanup create_subscription
Signed-off-by: William Woodall <william@osrfoundation.org>
* add missing file
Signed-off-by: William Woodall <william@osrfoundation.org>
* streamline creation of subscriptions after removing deprecated API
Signed-off-by: William Woodall <william@osrfoundation.org>
* small subscription code cleanup to match publisher's style
Signed-off-by: William Woodall <william@osrfoundation.org>
* some fixes to rclcpp_lifecycle to match rclcpp
Signed-off-by: William Woodall <william@osrfoundation.org>
* add README to the rclcpp/detail include directory
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup SubscriptionBase's use of visibility macros
Signed-off-by: William Woodall <william@osrfoundation.org>
* reapply function to create default options, as it is still needed on Windows
Signed-off-by: William Woodall <william@osrfoundation.org>
* address review comments
Signed-off-by: William Woodall <william@osrfoundation.org>
* workaround cppcheck 1.89 syntax error
Signed-off-by: William Woodall <william@osrfoundation.org>
* Take parameter overrides provided through the CLI.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Address peer review comments.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Do not 'find_package' rcpputils twice.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Fail on invalid and unknown ROS specific arguments.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Revert changes to utilities.hpp in rclcpp
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Fully revert change to utilities.hpp
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
This fixes a runtime error caused by a race condition when making consecutive requests for the
result.
Specifically, this happens if the user provides a result callback when sending a goal and then
calls async_get_result shortly after.
Resolves#783
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Use --ros-args to deal with node arguments in rclcpp.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Document implicit --ros-args flag in NodeOptions::arguments().
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Add missing size_t to int cast.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Only add implicit --ros-args flag if not present already.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Add some rclcpp::NodeOptions test coverage.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Address peer review comments.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Please cpplint and uncrustify.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* Switch the NodeParameters lock to recursive.
This is so that the on_set_parameter_callback can successfully
call other parameter methods (like get_parameter) without
deadlocking.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Make sure that modifications can't happen within a callback.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Review fixes.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Set parameter_modification_enabled_ in calls that make modifications.
This will prevent any modification from within modification,
which is a good thing.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Fix windows errors.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Switch to an RAII-style recursion guard.
Also update the documentation.
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
* Adding a factory method to create a Duration from seconds
There are many places in the ROS codebase where a time duration is
specified as a floating point number of seconds. A factory function
to create a Duration object from these values makes the code a
bit simpler in many cases.
Signed-off-by: Carl Delsey <carl.r.delsey@intel.com>
* Adding some comments to clarify which constructors get matched.
Signed-off-by: Carl Delsey <carl.r.delsey@intel.com>
The short-term goal of this change is to enable the creation of a
parameter YAML file which is applied to each node, regardless of node
name or namespace.
Future work is to support all wildcard syntax in node names in
parameter YAML files.
Signed-off-by: Scott K Logan <logans@cottsay.net>
* use default parameter descriptor in parameters interface
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
* use default parameter for value
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
* rename initial_parameters in NodeOptions to parameter_overrides
Signed-off-by: William Woodall <william@osrfoundation.org>
* rename automatically_declare_initial_parameters to automatically_declare_parameters_from_overrides
Signed-off-by: William Woodall <william@osrfoundation.org>
* some additional renames I missed
Signed-off-by: William Woodall <william@osrfoundation.org>
* add test for setting after declaring with parameter overrides
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup NodeOptions docs
Signed-off-by: William Woodall <william+github@osrfoundation.org>
Co-Authored-By: chapulina <louise@openrobotics.org>
* clarify relationship between allow_undeclared_parameters and parameter_overrides
Signed-off-by: William Woodall <william@osrfoundation.org>
* Throw nice errors when creating a publisher with intraprocess communication and history keep all or history depth 0.
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
Fixes#705.
If the set_parameters() call fails, it's nice to be able to return a partial result.
Since there is no convenient method to obtain a partial result, we call set_parameters()
once for each parameter.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add missing template functionality to lifecycle_node.
Recent changes to the node_parameters interface was accompanied by additions to the
node.hpp header and implementation files. However, these additions were not also made
to the corresponding lifecycle node files. This PR includes the changes required for
the lifecycle node.
Going forward, I suggest that any code like this that supplements the basic node interfaces
should either be factored out into a separate header that both node and lifecycle_node
include, or that the supplemental code simply be included in the appropriate node_interface
file directly, if possible. That way we can avoid code duplication and its symptoms which
is node and lifecycle_node getting out of sync (which they have several times).
Signed-off-by: Michael Jeronimo <michael.jeronimo@intel.com>
* consolidate documentation to just be in rclcpp/node.hpp
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix visibility macros
Signed-off-by: William Woodall <william@osrfoundation.org>
* deprecation methods that were also deprecated in rclcpp::Node
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup variable name
Signed-off-by: William Woodall <william@osrfoundation.org>
* add missing template method implementations
Signed-off-by: William Woodall <william@osrfoundation.org>
* add more methods that were not ported to lifecycle node originally
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix cpplint
Signed-off-by: William Woodall <william@osrfoundation.org>
For general questions, please ask on ROS answers: https://answers.ros.org, make sure to include at least the `ros2` tag and the rosdistro version you are running, e.g. `ardent`.
For general design discussions, please post on discourse: https://discourse.ros.org/c/ng-ros
Not sure if this is the right repository? Open an issue on https://github.com/ros2/ros2/issues
For Bug report or feature requests, please fill out the relevant category below
-->
## Bug report
**Required Info:**
- Operating System:
- <!-- OS and version (e.g. Windows 10, Ubuntu 16.04...) -->
- Installation type:
- <!-- binaries or from source -->
- Version or commit hash:
- <!-- Output of git rev-parse HEAD, release version, or repos file -->
- DDS implementation:
- <!-- rmw_implementation used (e.g. Fast-RTPS, RTI Connext, etc -->
- Client library (if applicable):
- <!-- e.g. rclcpp, rclpy, or N/A -->
#### Steps to reproduce issue
<!-- Detailed instructions on how to reliably reproduce this issue http://sscce.org/
``` code that can be copy-pasted is preferred ``` -->
```
```
#### Expected behavior
#### Actual behavior
#### Additional information
<!-- If you are reporting a bug delete everything below
If you are requesting a feature deleted everything above this line -->
----
## Feature request
#### Feature description
<!-- Description in a few sentences what the feature consists of and what problem it will solve -->
#### Implementation considerations
<!-- Relevant information on how the feature could be implemented and pros and cons of the different solutions -->
This repository contains the source code for the ROS Client Library for C++ package, included with a standard install of any ROS 2 distro.
rclcpp provides the standard C++ API for interacting with ROS 2.
## Usage
`#include "rclcpp/rclcpp.hpp"` allows use of the most common elements of the ROS 2 system.
The link to the latest API documentation can be found on the [rclcpp package info page](https://docs.ros.org/en/rolling/p/rclcpp).
### Examples
The ROS 2 tutorials [Writing a simple publisher and subscriber](https://docs.ros.org/en/rolling/Tutorials/Writing-A-Simple-Cpp-Publisher-And-Subscriber.html).
and [Writing a simple service and client](https://docs.ros.org/en/rolling/Tutorials/Writing-A-Simple-Cpp-Service-And-Client.html)
This document is a declaration of software quality for the `rclcpp` package, based on the guidelines in [REP-2004](https://reps.openrobotics.org/rep-2004/).
# rclcpp Quality Declaration
The package `rclcpp` claims to be in the **Quality Level 1** category when it is used with a **Quality Level 1** middleware.
Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Requirements for Quality Level 1 in REP-2004](https://reps.openrobotics.org/rep-2004/).
## Version Policy [1]
### Version Scheme [1.i]
`rclcpp` uses `semver` according to the recommendation for ROS Core packages in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#versioning).
### Version Stability [1.ii]
`rclcpp` is at a stable version, i.e. `>= 1.0.0`.
The current version can be found in its [package.xml](package.xml), and its change history can be found in its [CHANGELOG](CHANGELOG.rst).
### Public API Declaration [1.iii]
All symbols in the installed headers are considered part of the public API.
Except for the exclusions listed below, all installed headers are in the `include` directory of the package, headers in any other folders are not installed and considered private.
Headers under the folder `experimental` are not considered part of the public API as they have not yet been stabilized. These symbols are namespaced `rclcpp::experimental`.
Headers under the folder `detail` are not considered part of the public API and are subject to change without notice. These symbols are namespaced `rclcpp::detail`.
### API Stability Policy [1.iv]
`rclcpp` will not break public API within a released ROS distribution, i.e. no major releases once the ROS distribution is released.
### ABI Stability Policy [1.v]
`rclcpp` contains C++ code and therefore must be concerned with ABI stability, and will maintain ABI stability within a ROS distribution.
### ABI and ABI Stability Within a Released ROS Distribution [1.vi]
`rclcpp` will not break API nor ABI within a released ROS distribution, i.e. no major releases once the ROS distribution is released.
## Change Control Process [2]
`rclcpp` follows the recommended guidelines for ROS Core packages in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#change-control-process).
### Change Requests [2.i]
All changes will occur through a pull request, check [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#change-control-process) for additional information.
### Contributor Origin [2.ii]
This package uses DCO as its confirmation of contributor origin policy. More information can be found in [CONTRIBUTING](../CONTRIBUTING.md).
### Peer Review Policy [2.iii]
All pull requests will be peer-reviewed, check [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#change-control-process) for additional information.
### Continuous Integration [2.iv]
All pull requests must pass CI on all [tier 1 platforms](https://reps.openrobotics.org/rep-2000/#support-tiers)
All pull requests must resolve related documentation changes before merging.
## Documentation [3]
### Feature Documentation [3.i]
`rclcpp` has a [feature list](http://docs.ros2.org/latest/api/rclcpp/) and each item in the list links to the corresponding feature documentation. There is documentation for all of the features, and new features require documentation before being added.
### Public API Documentation [3.ii]
The API is publicly available in its [ROS 2 API documentation](http://docs.ros2.org/latest/api/rclcpp/).
### License [3.iii]
The license for `rclcpp` is Apache 2.0, and a summary is in each source file, the type is declared in the [`package.xml`](./package.xml) manifest file, and a full copy of the license is in the [`LICENSE`](../LICENSE) file.
There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp/) can be found a list with the latest results of the various linters being run on the package.
### Copyright Statements [3.iv]
The copyright holders each provide a statement of copyright in each source code file in `rclcpp`.
There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp/copyright/).
## Testing [4]
### Feature Testing [4.i]
Each feature in `rclcpp` has corresponding tests which simulate typical usage, and they are located in the [`test`](https://github.com/ros2/rclcpp/tree/rolling/test) directory.
New features are required to have tests before being added.
Each part of the public API has tests, and new additions or changes to the public API require tests before being added.
The tests aim to cover both typical usage and corner cases, but are quantified by contributing to code coverage.
### Coverage [4.iii]
`rclcpp` follows the recommendations for ROS Core packages in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#code-coverage), and opts to use line coverage instead of branch coverage.
This includes:
- tracking and reporting line coverage statistics
- achieving and maintaining a reasonable branch line coverage (90-100%)
- no lines are manually skipped in coverage calculations
Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers.
Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastCompletedBuild/cobertura/src_ros2_rclcpp_rclcpp_src_rclcpp/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#note-on-coverage-runs).
`rclcpp` has a line coverage `>= 95%`, which is calculated over all directories within `rclcpp` with the exception of the `experimental` directory.
### Performance [4.iv]
`rclcpp` follows the recommendations for performance testing of C/C++ code in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#performance), and opts to do performance analysis on each release rather than each change.
The performance tests of `rclcpp` are located in the [test/benchmark directory](https://github.com/ros2/rclcpp/tree/rolling/rclcpp/test/benchmark).
System level performance benchmarks that cover features of `rclcpp` can be found at:
Changes that introduce regressions in performance must be adequately justified in order to be accepted and merged.
### Linters and Static Analysis [4.v]
`rclcpp` uses and passes all the ROS 2 standard linters and static analysis tools for a C++ package as described in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#linters-and-static-analysis). Passing implies there are no linter/static errors when testing against CI of supported platforms.
Below are evaluations of each of `rclcpp`'s run-time and build-time dependencies that have been determined to influence the quality.
It has several "buildtool" dependencies, which do not affect the resulting quality of the package, because they do not contribute to the public library API.
It also has several test dependencies, which do not affect the resulting quality of the package, because they are only used to build and run the test code.
### Direct and Optional Runtime ROS Dependencies [5.i]/[5.ii]
`rclcpp` has the following runtime ROS dependencies:
#### `libstatistics_collector`
The `libstatistics_collector` package provides lightweight aggregation utilities to collect statistics and measure message metrics.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros-tooling/libstatistics_collector/tree/rolling/QUALITY_DECLARATION.md).
#### `rcl`
`rcl` a library to support implementation of language specific ROS 2 Client Libraries.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/rolling/rcl/QUALITY_DECLARATION.md).
#### `rcl_yaml_param_parser`
The `rcl_yaml_param_parser` package provides an API that is used to parse YAML configuration files which may be used to configure ROS and specific nodes.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl/tree/rolling/rcl_yaml_param_parser/QUALITY_DECLARATION.md).
#### `rcpputils`
The `rcpputils` package provides an API which contains common utilities and data structures useful when programming in C++.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcpputils/blob/rolling/QUALITY_DECLARATION.md).
#### `rcutils`
The `rcutils` package provides an API which contains common utilities and data structures useful when programming in C.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcutils/blob/rolling/QUALITY_DECLARATION.md).
#### `rmw`
`rmw` is the ROS 2 middleware library.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rmw/blob/rolling/rmw/QUALITY_DECLARATION.md).
#### `statistics_msgs`
The `statistics_msgs` package contains ROS 2 message definitions for reporting statistics for topics and system resources.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/rolling/statistics_msgs/QUALITY_DECLARATION.md).
#### `tracetools`
The `tracetools` package provides utilities for instrumenting the code in `rclcpp` so that it may be traced for debugging and performance analysis.
It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/ros2_tracing/blob/rolling/tracetools/QUALITY_DECLARATION.md).
### Direct Runtime non-ROS Dependency [5.iii]
`rclcpp` has no run-time or build-time dependencies that need to be considered for this declaration.
## Platform Support [6]
`rclcpp` supports all of the tier 1 platforms as described in [REP-2000](https://reps.openrobotics.org/rep-2000/#support-tiers), and tests each change against all of them.
The link to the latest rclcpp API documentation, which includes a complete list of its main components and features, can be found on the [rclcpp package info page](https://docs.ros.org/en/rolling/p/rclcpp).
## Quality Declaration
This package claims to be in the **Quality Level 1** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.
> [rclcpp_action] There exists a thread-safe and non-thread-safe way to get the goal result from an action client. We probably want to remove the public interface to the non-thread safe call (or consolidate somehow): https://github.com/ros2/rclcpp/issues/955
`rclcpp_action` is out of scope atm.
**Notes from 2020-03-19**: To be handled in separate API review.
## Architecture
### Calling Syntax and Keeping Node-like Class APIs in Sync
> Currently, much of the API is exposed via the `rclcpp::Node` class, and due to the nature of the current architecture there is a lot of repeated code to expose these methods and then call the implementations which are in other classes like `rclcpp::node_interfaces::NodeTopics`, for example.
>
> Also, we have other versions of the class `rclcpp::Node` with different semantics and interfaces, like `rclcpp_lifecycle::LifecycleNode`, and we have been having trouble keeping the interface provided there up to date with how things are done in `rclcpp::Node`. Since `LifecycleNode` has a different API from `Node` in some important cases, it does not just inherit from `Node`.
>
> There are two main proposals (as I see it) to try and address this issue, either (a) break up the functionality in `Node` so that it is in separate classes and make `Node` multiple inherit from those classes, and then `LifecycleNode` could selectively inherit from those as well, or (b) change our calling convention from `node->do_thing(...)` to be `do_thing(node, ...)`.
>
> For (a) which commonly referred to as the [Policy Based Design Pattern](https://en.wikipedia.org/wiki/Modern_C%2B%2B_Design#Policy-based_design), we'd be reversing previous design decisions which we discussed at length where we decided to use composition over inheritance for various reasons.
> One of the reasons was testing, with the theory that having simpler separate interfaces we could more easily mock them as needed for testing.
> The testing goal would still be met, either by keeping the "node_interface" classes as-is or by mocking the classes that node would multiple inherit from, however it's harder to indicate that a function needs a class that multiple inherits from several classes but not others.
> Also having interdependency between the classes which are inherited from is a bit complicated in this design pattern.
>
> For (b), we would be changing how we recommend all code be written (not a trivial thing to do at all), because example code like `auto pub = node->create_publsiher(...);` would be come `auto pub = create_publisher(node, ...);`.
> This has some distinct advantages, however, in that it allows us to write these functions, like `create_publisher(node, ...)`, so that the node argument can be any class that meets the criteria of the function.
> That not only means that when we add a feature it automatically works with `Node` and `LifecycleNode` without adding anything to them, it also means that user defined `Node`-like classes will also work, even if they do not inherit from or provide the complete interface for `rclcpp::Node`.
> Another major downside of this approach is discoverability of the API when using auto-completion in text editors, as `node-><tab>` will often give you a list of methods to explore, but with the new calling convention, there's not way to get an auto complete for code who's first argument is a `Node`-like class.
>
> Both of the above approaches address some of the main concerns, which are: keeping `Node` and `LifecycleNode` in sync, reducing the size of the `Node` class so it is more easily maintained, documented, and so that related functions are grouped more clearly.
- https://github.com/ros2/rclcpp/issues/898
- https://github.com/ros2/rclcpp/issues/509
- https://github.com/ros2/rclcpp/issues/855
- https://github.com/ros2/rclcpp/issues/985
- subnode feature is in rclcpp::Node only, complicating "node using" API designs
- "Many programmers are tempted to write member functions to get the benefits of the member function syntax (e.g. "dot-autocomplete" to list member functions);[6] however, this leads to excessive coupling between classes.[7]"
**Suggested Action**: Document the discussion and defer until Foxy.
**Notes from 2020-03-19**:
- Another version of (b) could be to have classes that are constructed with node, e.g. `Publisher(node, ...)` rather than `node->create_publisher(...)`
-`NodeInterface<LifecycleNode>::<tab>` -> only life cycle node methods
- (karsten) use interface classes directly, e.g. node->get_X_interface()->do_thing()
- (dirk) use macros to add methods to class
- Question: Do we want tab-completable API (specifically list of member functions)?
- Question: Is consistency in calling between core and non-core features more important than tab-completion?
- Add better example of adding new feature and not needing to touch `rclcpp::Node`.
- (dirk) methods and free functions not mutually exclusive.
### Scoped Versus Non-Scoped Entities (e.g. Publishers/Subscriptions)
> Currently, Publisher and Subscription (and similar entities) are scoped, meaning that when they are created they are added to the ROS graph as a side effect, and when they are let out of scope they remove themselves from the graph too.
> Additionally, they necessarily have shared state with the system, for instance when you are spinning on a node, the executor shares ownership of the Subscriptions with the user.
> Therefore, the Subscription only gets removed when both the user and executor are done with it.
>
> This shared ownership is accomplished with the use of shared pointers and weak pointers.
>
> There are a few concerns here, (a) use of shared pointers confuses users, (b) overhead of shared pointers and lack of an ability to use these classes on the stack rather than the heap, and (c) complexity of shutdown of an entity from the users perspective.
>
> For (a), some users are overwhelmed by the need to use a shared pointer.
> In ROS 1 this was avoided by having a class which itself just thinly wraps a shared pointer (see: https://github.com/ros/ros_comm/blob/ac9f88c59a676ca6895e13445fc7d71f398ebe1f/clients/roscpp/include/ros/subscriber.h#L108-L111).
> This could be achieved in ROS 2 either by doing the same with a wrapper class (at the expense of lots of repeated code), or by eliminating the need for using shared ownership.
>
> For (b), for some use cases, especially resource constrained / real-time / safety-critical environments, requiring these classes to be on the heap rather than the stack is at least inconvenient.
> Additionally, there is a cost associated with using shared pointers, in the storage of shared state and in some implementation the use of locks or at least atomics for thread-safety.
>
> For (c), this is the most concerning drawback, because right now when a user lets their shared pointer to a, for example, Subscription go out of scope, a post condition is not that the Subscription is destroyed, nor that it has been removed from the graph.
> In stead, the behavior is more like "at some point in the future the Subscription will be destroyed and removed from the graph, when the system is done with it".
> This isn't a very satisfactory contract, as some users may wish to know when the Subscription has been deleted, but cannot easily know that.
>
> The benefit to the shared state is a safety net for users.
> The alternative would be to document that a Subscription, again for example, cannot be deleted until the system is done with it.
> We'd basically be pushing the responsibility onto the user to ensure the shared ownership is handled properly by the execution of their application, i.e. they create the Subscription, share a reference with the system (adding it by reference to an executor, for example), and they have to make sure the system is done with it before deleting the Subscription.
>
>Separately, from the above points, there is the related concern of forcing the user to keep a copy of their entities in scope, whether it be with a shared pointer or a class wrapping one.
> There is the desire to create it and forget it in some cases.
> The downside to this is that if/when the user wants to destroy the entity, they have no way of doing that as they have no handle or unique way to address the entity.
>
> One proposed solution would be to have a set of "named X" APIs, e.g. `create_named_subscription` rather than just `create_subscription`.
> This would allow the user to address the Subscription in the future in order to obtain a new reference to it or delete it.
- https://github.com/ros2/rclcpp/issues/506
- https://github.com/ros2/rclcpp/issues/726
**Suggested Action**: Consolidate to a single issue, and defer.
**Notes from 2020-03-23**:
- (chris) Putting ownership mechanics on user is hard.
- (shane) warn on unused to catch issues with immediately deleted items
- (tfoote) debugging output for destruction so it easy to see when reviewing logs
- (chris) possible to create API that checks for destruction
- (william) might lead to complex synchronization issues
- (tfoote) could add helper classes to make scoped things non-scoped
- (shane) concerned that there is no longer "one good way" to do it
### Allow QoS to be configured externally, like we allow remapping of topic names
> Suggestion from @stonier: allow the qos setting on a topic to be changed externally at startup, similar to how we do topic remapping (e.g., do it on the command-line using appropriate syntax).
>
> To keep the syntax manageable, we might just allow profiles to be picked.
- https://github.com/ros2/rclcpp/issues/239
**Suggested Action**: Update issue, defer for now.
**Notes from 2020-03-19**:
- (wjwwood) it depends on the QoS setting, but many don't make sense, mostly because they can change some of the behaviors of underlying API
- (dirk) Should developers expose a parameter instead?
- (multiple) should be a feature that makes configuring them (after opt-in) consistent
- (jacob) customers feedback was that this was expected, surprised it was not allowed
- (karsten) could limit to profiles
## Init/shutdown and Context
### Consider renaming `rclcpp::ok()`
> Old discussion to rename `rclcpp::ok()` to something more specific, like `rclcpp::is_not_shutdown()` or the corollary `rclcpp::is_shutdown()`.
- https://github.com/ros2/rclcpp/issues/3
**Suggested Action**: Defer.
**Notes from 2020-03-19**:
- (shane) preference to not have a negative in the function name
## Executor
### Exposing Scheduling of Tasks in Executor and a Better Default
> Currently there is a hard coded procedure for handling ready tasks in the executor, first timers, then subscriptions, and so on.
> This scheduling is not fair and results in non-deterministic behavior and starvation issues.
>
> We should provide a better default scheduling which is fairer and ideally deterministic, something like round-robin or FIFO.
>
> Additionally, we should make it easier to let the user override the scheduling logic in the executor.
- https://github.com/ros2/rclcpp/pull/614
- https://github.com/ros2/rclcpp/issues/633
- https://github.com/ros2/rclcpp/issues/392
**Suggested Action**: Follow up on proposals to implement FIFO scheduling and refactor the Executor design to more easily expose the scheduling logic.
**Notes from 2020-03-19**:
- No comments.
### Make it possible to wait on entities (e.g. Subscriptions) without an Executor
> Currently, it is only possible to use things like Timers and Subscriptions and Services with an executor.
> It should be possible, however, to either poll these entities or wait on them and then decide which to process as a user.
>
> This is most easily accomplished with a WaitSet-like class.
- https://github.com/ros2/rclcpp/issues/520
**Suggested Action**: implement WaitSet class in rclcpp so that this is possible, and make "waitable" entities such that they can be polled, e.g. `Subscription`s should have a user facing `take()` method, which can fail if no data is available.
**Notes from 2020-03-19**:
- No comments.
### Make it possible to use multiple executors per node
> Currently, you cannot use more than one executor per node, this limits your options when it comes to distributing work within a node across threads.
> You can use a multi-threaded executor, or make your own executor which does this, but it is often convenient to be able to spin part of the node separately from the the rest of the node.
- https://github.com/ros2/rclcpp/issues/519
**Suggested Action**: Make this possible, moving the exclusivity to be between an executor and callback groups rather than nodes.
**Notes from 2020-03-19**:
- No comments.
### Implement a Lock-free Executor
> This would presumably be useful for real-time and safety critical systems where locks and any kind of blocking code is considered undesirable.
- https://github.com/ros2/rclcpp/issues/77
**Suggested Action**: Keep in backlog until someone needs it specifically.
**Notes from 2020-03-19**:
- No comments.
### Add implementation of `spin_some()` to the `MultiThreadedExecutor`
> Currently `spin_some()` is only available in the `SingleThreadedExecutor`.
- https://github.com/ros2/rclcpp/issues/85
**Suggested Action**: Defer.
**Notes from 2020-03-19**:
- No comments.
## Node
### Do argument parsing outside of node constructor
> Things that come from command line arguments should be separately passed into the node's constructor rather than passing in arguments and asking the node to do the parsing.
- https://github.com/ros2/rclcpp/issues/492
**Suggested Action**: Defer until after foxy.
**Notes from 2020-03-23**:
- (dirk) may be related to ROS 1 heritage of argc/argv being passed to node directly
- (shane) impacts rcl API as well, two parts "global options" as well node specific options
- (dirk) what is the recommendation to users that want to add arguments programmatically
- user should be able to get non-ros argc/argv somehow (seems like you can now)
- (jacob) the argument in NodeOptions are used for application specific argument via component loading as well
## Timer
### Timer based on ROS Time
> `node->create_wall_timer` does exactly what it says; creates a timer that will call the callback when the wall time expires. But this is almost never what the user wants, since this won’t work properly in simulation. Suggestion: deprecate `create_wall_timer`, add a new method called `create_timer` that takes the timer to use as one of the arguments, which defaults to ROS_TIME.
**Suggested Action**: Promote `rclcpp::create_timer()` which is templated on a clock type, as suggested, but leave `create_wall_timer` as a convenience.
**Notes from 2020-03-19**:
- (shane) may be a `rclcpp::create_timer()` that can be used to create a non-wall timer already
## Publisher
## Subscription
### Callback Signature
> Is there a reason the subscription callback must have a smart pointer argument instead of accepting a const-reference argument?
**Suggested Action**: Provide const reference as an option, add documentation as to the implications of one callback signature versus others.
**Notes from 2020-03-19**:
- (dirk) have const reference and document it
## Service Server
### Allow for asynchronous Service Server callbacks
> Currently, the only callback signature for Service Servers takes a request and must return a response.
> This means that all of the activity of the service server has to happen within that function.
> This can cause issues, specifically if you want to call another service within the current service server's callback, as it causes deadlock issues trying to synchronously call the second service within a spin callback.
> More generally, it seems likely that long running service server callbacks may be necessary in the future and requiring them to be synchronous would tie up at least on thread in the spinning executor unnecessarily.
- https://github.com/ros2/rclcpp/issues/491
**Suggested Action**: Defer.
**Notes from 2020-03-23**:
- (dirk) likely new API, so possible to backport
## Service Client
### Callback has SharedFuture rather than const reference to response
> Why does the Client::send_async_request take in a callback that has a SharedFuture argument instead of an argument that is simply a const-reference (or smart pointer) to the service response type? The current API seems to imply that the callback ought to check whether the promise is broken or fulfilled before trying to access it. Is that the case? If so, it should be documented in the header.
- (wjwwood) we wanted the user to handle error cases with the future?
- (dirk) future allows for single callback (rather than one for response and one for error)
- (jacob) actions uses a "wrapped result" object
### rclcpp missing synchronous `send_request` and issues with deadlocks
> This has been reported by several users, but there is only an `async_send_request` currently. `rclpy` has a synchronous `send_request` but it has issues with deadlock, specifically if you call it without spinning in another thread then it will deadlock. Or if you call it from within a spin callback when using a single threaded executor, it will deadlock.
### Implicitly cast integer values for double parameters
> If we try to pass an integer value to a double parameter from the command line or from a parameters YAML file we get a `rclcpp::ParameterTypeException`.
> For example, passing a parameter from the command line:
>
> ros2 run foo_package foo_node --ros-args -p foo_arg:=1
>
> results in the following error:
>
> terminate called after throwing an instance of 'rclcpp::ParameterTypeException'
> what(): expected [double] got [integer]
>
> and we can fix it by explicitly making our value a floating point number:
>
> ros2 run foo_package foo_node --ros-args -p foo_arg:=1.0
>
> But, it seems reasonable to me that if a user forgets to explicitly provide a floating point value that we should implicitly cast an integer to a float (as is done in many programming languages).
- https://github.com/ros2/rclcpp/issues/979
**Suggested Action**: Continue with issue.
**Notes from 2020-03-23**:
- (shane) says "yes please" :)
### Use `std::variant` instead of custom `ParameterValue` class
> This is only possible if C++17 is available, but it would simplify our code, make our interface more standard, and allow us to use constexpr-if to simply our templated code.
**Suggested Action**: Create an issue for future work.
**Notes from 2020-03-23**:
- (chris) not sure churn is worth
- (ivan) other places for std::variant, like AnySubscriptionCallback
### Cannot set name or value on `Parameter`/`ParameterValue`
> Both `Parameter` and `ParameterValue` are read-only after construction.
- (chris/william) setting values on temporary (local) objects is not reflected in the node, so misleading
## Parameter Clients
### No timeout option with synchronous parameter client calls
> As an example, SyncParametersClient::set_parameters doesn't take a timeout option. So, if anything goes wrong in the service call (e.g. the server goes down), we will get stuck waiting indefinitely.
**Suggested Action**: Reconcile class and file name, switch to singular name?
**Notes from on-line, post 2020-03-23 meeting**:
- (tfoote) +1 for homogenizing to singular
### `SyncParametersClient::get_parameters` doesn't allow you to detect error cases
> E.g. https://github.com/ros2/rclcpp/blob/249b7d80d8f677edcda05052f598de84f4c2181c/rclcpp/src/rclcpp/parameter_client.cpp#L246-L257 returns an empty vector if something goes wrong which is also a valid response.
**Suggested Action**: Throw an exception to indicate if something went wrong and document other expected conditions of the API.
**Notes from on-line, post 2020-03-23 meeting**:
- (tfoote) An empty list is not a valid response unless you passed in an empty list. The return should have the same length as the request in the same order. Any parameters that are not set should return a ParameterVariant with type PARAMETER_NOT_SET. to indicate that it was polled and determined to not be set. Suggested action improve documentation of the API to clarify a short or incomplete.
- (jacobperron) I think throwing an exception is also a valid action, making it clear that an error occurred.
- (wjwwood) Using exceptions to indicate an exceptional case (something went wrong) seems reasonable to me too.
## Clock
### Clock Jump callbacks on System or Steady time?
> Currently time jump callbacks are registered via Clock::create_jump_handler(). Jump handlers are only invoked by TimeSource::set_clock(). This is only called if the clock type is RCL_ROS_TIME and ROS time is active.
- https://github.com/ros2/rclcpp/issues/528
**Suggested Action**: Document that time jumping is only detected with ROS time, consider a warning.
**Notes from on-line, post 2020-03-23 meeting**:
- (tfoote) There should be no jumps in steady time. If there's a big change in system time, it doesn't necessarily mean that time jumped, just that you might have been sleeping for a long time. Most ntp systems adjust the slew rate these days instead of jumping but still that's an external process and I don't know of any APIs to introspect the state of the clock. I'm not sure that we have a way to detect jumps in time for system or steady time. To that end I think that we should be clear that we only provide callbacks when simulation time starts or stops, or simulation time jumps. We should also strongly recommend that operators not actively adjust their system clocks while running ROS nodes.
- (jacobperron) I agree with Tully, if we don't have a way to detect system time jumps then I think we should just document that this only works with ROS time. In addition to documentation, we could log an info or warning message if the user registers jump callback with steady or system time, but it may be unnecessarily noisy.
Until ROS 2 Foxy, all parameters could change type anytime, except if the user installed a parameter callback to reject a change.
This could generate confusing errors, for example:
```
$ ros2 run demo_nodes_py listener &
$ ros2 param set /listener use_sim_time not_a_boolean
[ERROR] [1614712713.233733147] [listener]: use_sim_time parameter set to something besides a bool
Set parameter successful
$ ros2 param get /listener use_sim_time
String value is: not_a_boolean
```
For most use cases, having static parameter types is enough.
This article documents some of the decisions that were made when implementing static parameter types enforcement in:
* https://github.com/ros2/rclcpp/pull/1522
* https://github.com/ros2/rclpy/pull/683
## Allowing dynamically typed parameters
There might be some scenarios where dynamic typing is desired, so a `dynamic_typing` field was added to the [parameter descriptor](https://github.com/ros2/rcl_interfaces/blob/09b5ed93a733adb9deddc47f9a4a8c6e9f584667/rcl_interfaces/msg/ParameterDescriptor.msg#L25).
The original requirement came in **gazebo_ros_pkgs** for setting individual wheel slip parameters based on global wheel slip value [link to original issue](https://github.com/ros-simulation/gazebo_ros_pkgs/pull/1365).
The main requirement is to set one or more parameters after another parameter is set successfully.
Additionally, it would be nice if users could be notified locally (via a callback) when parameters have been set successfully (i.e. post validation).
Related discussion can be found in [#609](https://github.com/ros2/rclcpp/issues/609) [#1789](https://github.com/ros2/rclcpp/pull/1789)
With the current parameters API, the `add_on_set_parameters_callback` is intended for validation of parameter values before they are set, it should **not** cause any side-effects.
There is also the `ParameterEventHandler` that publishes changes to node parameters on `/parameter_events` topic for external nodes to see. Though the node could subscribe to the `/parameter_events` topic to be notified of changes to its own parameters, it is less than ideal since there is a delay caused by waiting for an executor to process the callback.
We propose adding a `PostSetParametersCallbackHandle` for successful parameter set similar to `OnSetParametersCallbackHandle` for parameter validation. Also, we propose adding a `PreSetParametersCallbackHandle` useful for modifying list of parameters being set.
The validation callback is often abused to trigger side effects in the code, for instance updating class attributes even before a parameter has been set successfully. Instead of relying on the `/parameter_events` topic to be notified of parameter changes, users can register a callback with a new API, `add_post_set_parameters_callback`.
It is possible to use the proposed `add_post_set_parameters_callback` for setting additional parameters, but this might result in infinite recursion and does not allow those additional parameters to be set atomically with the original parameter(s) changed.
To workaround these issues, we propose adding a "pre set" callback type that can be registered with `add_pre_set_parameters_callback`, which will be triggered before the validation callbacks and can be used to modify the parameter list.
* Users could call `set_parameter` while processing a message from the `/parameter_events` topic, however, there is extra overhead in having to create subscription (as noted earlier).
* Users could call `set_parameter` inside the "on set" parameters callback, however it is not well-defined how side-effects should handle cases where parameter validation fails.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.