* 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>
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>
* 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>
* 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>
* 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>
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>
* 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>
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>
* 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>
* 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>
* 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>
* 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>
* 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>
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>
* improve interoperability with rclcpp::Duration and std::chrono
Signed-off-by: William Woodall <william@osrfoundation.org>
* add to_rmw_time to Duration
Signed-off-by: William Woodall <william@osrfoundation.org>
* add new QoS class to rclcpp
Signed-off-by: William Woodall <william@osrfoundation.org>
* changes to NodeBase, NodeTopics, etc in preparation for changes to pub/sub
Signed-off-by: William Woodall <william@osrfoundation.org>
* refactor publisher creation to use new QoS class
Signed-off-by: William Woodall <william@osrfoundation.org>
* refactor subscription creation to use new QoS class
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixing fallout from changes to pub/sub creation
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixed Windows error: no appropriate default constructor available
why? who knows
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixed Windows error: could not deduce template argument for 'PublisherT'
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix missing vftable linker error on Windows
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix more cases of no suitable default constructor errors...
Signed-off-by: William Woodall <william@osrfoundation.org>
* prevent msvc from trying to interpret some cases as functions
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>
* add C++ version of default action qos
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixing lifecycle subscription signatures
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix allocators (we actually use this already in the pub/sub factory)
Signed-off-by: William Woodall <william@osrfoundation.org>
* suppress cppcheck on false positive syntax error
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix more cppcheck syntax error false positives
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix case where sub-type of QoS is used
Signed-off-by: William Woodall <william@osrfoundation.org>
* fixup get_node_topics_interface.hpp according to reviews and tests
Signed-off-by: William Woodall <william@osrfoundation.org>
* additional fixes based on local testing and CI
Signed-off-by: William Woodall <william@osrfoundation.org>
* another trick to avoid 'no appropriate default constructor available'
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix compiler error with clang on macOS
Signed-off-by: William Woodall <william@osrfoundation.org>
* disable build failure tests until we can get Jenkins to ignore their output
Signed-off-by: William Woodall <william@osrfoundation.org>
* suppress more cppcheck false positives
Signed-off-by: William Woodall <william@osrfoundation.org>
* add missing visibility macros to default QoS profile classes
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix another case of 'no appropriate default constructor available'
Signed-off-by: William Woodall <william@osrfoundation.org>
* unfortunately this actaully fixes a build error on Windows...
Signed-off-by: William Woodall <william@osrfoundation.org>
* fix typos
Signed-off-by: William Woodall <william@osrfoundation.org>
* Populate return code of CancelGoal service response
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Throw if there is an error processing a cancel goal request
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Make cancel callback signature consistent across cancel methods and add tests
Refactored the callback signature for canceling one goal. Now it is the same as the other cancel methods.
This makes it easier to communicate the error code to the user.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Address review
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
