Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Rate to select the clock to work with #2123

Merged
merged 18 commits into from
Aug 31, 2023

Conversation

AlexeyMerzlyakov
Copy link
Contributor

This is the continue of [#1373] discussion and related to the ros-navigation/navigation2#3325 issue in Nav2

The initial idea is to make rclcpp::Rate to work with ROS-time (currently it works only with system and steady times).

What was made:

  • Add rclcpp::Clock pointer to the rclcpp::Rate constructor to allow developer to specify the clock to work with (by-default it is still RCL_SYSTEM_TIME if no clock is specified)
  • Add rclcpp::ROSRate rate class, respective to ROS-time

Current implementation points:

  • Getting rid of templates' structure allows developer to select the clock to work with in dynamics
  • Renamed rclcpp::GenericRate -> to rclcpp::Rate and gave it the default clock to work with
  • rclcpp::Rate and rclcpp::WallRate API remaining to be untouched as much as possible
  • Although there are some changes in API: rclcpp::Duration instead of std::chrono::nanoseconds in durations and Rate::get_type() routine instead of previously used Rate::is_steady()
  • New rclcpp::ROSRate class that working with ROS time

@AlexeyMerzlyakov
Copy link
Contributor Author

The PR contains only the time.hpp change. If the change will be agreed, I will fix/add new tests on it.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

overall lgtm

rclcpp/include/rclcpp/rate.hpp Show resolved Hide resolved
rclcpp/include/rclcpp/rate.hpp Outdated Show resolved Hide resolved
rclcpp/include/rclcpp/rate.hpp Outdated Show resolved Hide resolved
rclcpp/include/rclcpp/rate.hpp Outdated Show resolved Hide resolved
@AlexeyMerzlyakov
Copy link
Contributor Author

The build was failed due to outdated branch. Rebased over latest rolling and force-pushed branch.

Copy link
Collaborator

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM @clalancette is there a chance to get this in before Iron? This is an important update

@@ -42,19 +45,20 @@ using std::chrono::duration_cast;
using std::chrono::nanoseconds;

template<class Clock = std::chrono::high_resolution_clock>
class GenericRate : public RateBase
class [[deprecated("use rclcpp::Rate class instead of GenericRate")]] GenericRate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this derive from RateBase again?

Copy link
Contributor Author

@AlexeyMerzlyakov AlexeyMerzlyakov Apr 14, 2023

Choose a reason for hiding this comment

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

Since of meaning of clock types has been updated (ROS-respecitve clock type added), new rcl_clock_type_t get_type() method was introduced in a RateBase basic class as a alternative/replacement for bool is_steady() one (I would like to completely remove is_steady() method from on now as unnecessary, but since old API should be supported for compatibility of rclcpp::Rate and rclcpp::WallRate classes, is_steady was remain). GenericRate if being a child-class of RateBase, now should implement get_type() method. But since GenericRate - is a deprecated class, it might be excess, I believe.

Therefore GenericRate was taken out of parenthood of RateBase remaining the same functionality as it previously has; while new RateBase -> Rate -> WallRate/ROSRate lineage was introduced instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. Should rate base be marked with a similar deprecation warning? @fujitatomoya what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry to be late to get back to you. What we need to deprecate is RateBase::is_steady since the application can get the clock type?

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with removing GenericRate from the hierarchy is that this breaks certain applications. Consider for example if a downstream node has std::vector<RateBase> in their code. In that case, they will no longer be able to store a GenericRate in there, since it is no longer a child class of RateBase.

So I think we should add this back to the hierarchy (i.e. make this a child of RateBase), and implement get_type() as returning either RCL_SYSTEM_TIME (if Clock::is_steady is false), or RCL_STEADY_TIME (if Clock::is_steady is true).

Copy link
Contributor Author

@AlexeyMerzlyakov AlexeyMerzlyakov Jul 26, 2023

Choose a reason for hiding this comment

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

@fujitatomoya, OK. Thanks for reaching us. I've made is_steady() method to be deprecated for all classes in this commit.

@clalancette, Got your point. This indeed makes sense, so RateBase -> GenericRate relationship was restored in the same commit.

@clalancette
Copy link
Contributor

Here is CI (just on Linux) to see how much we would have to change to get this in:

  • Linux Build Status

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@clalancette
Copy link
Contributor

So there were a lot of failures in CI. That may be because this needs to be rebased, I'm not sure. @AlexeyMerzlyakov @SteveMacenski can you rebase this onto the latest and we can try again?

@wjwwood

This comment was marked as outdated.

@mergify

This comment was marked as outdated.

@wjwwood

This comment was marked as outdated.

@mergify

This comment was marked as outdated.

@clalancette
Copy link
Contributor

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Apr 18, 2023

rebase

✅ Branch has been successfully rebased

@wjwwood
Copy link
Member

wjwwood commented Apr 18, 2023

New CI after rebase: Build Status

@SteveMacenski
Copy link
Collaborator

@AlexeyMerzlyakov take a look. The policy is that deprecations in the core need to have PRs to resolve so that builds are green without deprecated API use. So anywhere that we have failures we should have complimentary PRs in the ROS 2 core so that everything turns over OK. Unfortunately due to that delay, this won't be able to get in before Iron's freeze

@SteveMacenski
Copy link
Collaborator

Any update?

@AlexeyMerzlyakov
Copy link
Contributor Author

Waiting until ros2/system_tests#516 (that covers failed tests) to be reviewed on the way to its merge. Then we need to restart testing again in this PR to make sure that all regressions were resolved.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

overall lgtm! just one minor comment.

@@ -42,19 +45,20 @@ using std::chrono::duration_cast;
using std::chrono::nanoseconds;

template<class Clock = std::chrono::high_resolution_clock>
class GenericRate : public RateBase
class [[deprecated("use rclcpp::Rate class instead of GenericRate")]] GenericRate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry to be late to get back to you. What we need to deprecate is RateBase::is_steady since the application can get the clock type?

@AlexeyMerzlyakov
Copy link
Contributor Author

AlexeyMerzlyakov commented Jul 26, 2023

For the clarity, I've prepared API Rate* change briefly summarized in the following document:
Rate_API_change.pdf

Hope, this might help to take an overall look on this change.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Thank you so much for putting together the document explaining the change. It is really helpful.

I've left one change inline.

Besides that, the fact that the Rate and WallRate classes now require a context to be initialized is a big change from the previous implementation. That doesn't necessarily mean we shouldn't do this, but I do want to have a conversation about it with the maintainers. @fujitatomoya @alsora @wjwwood what do you think about this change in semantics?

Comment on lines 142 to 146
{
// Time coming into sleep
auto now = clock_->now();
// Time of next interval
auto next_interval = last_interval_ + period_;
// Detect backwards time flow
if (now < last_interval_) {
// Best thing to do is to set the next_interval to now + period
next_interval = now + period_;
}
// Update the interval
last_interval_ += period_;
// If the time_to_sleep is negative or zero, don't sleep
if (next_interval <= now) {
// If an entire cycle was missed then reset next interval.
// This might happen if the loop took more than a cycle.
// Or if time jumps forward.
if (now > next_interval + period_) {
last_interval_ = now + period_;
}
// Either way do not sleep and return false
return false;
}
// Calculate the time to sleep
auto time_to_sleep = next_interval - now;
// Sleep (will get interrupted by ctrl-c, may not sleep full time)
return clock_->sleep_for(time_to_sleep);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this class is not a template, I'd like to see all of this implementation moved into a rate.cpp file. That will help with compile times of downstream projects.

The same goes for all of the other methods below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved all Rate implementations into source-file in 3d7b223

@fujitatomoya
Copy link
Collaborator

Thank you so much for putting together the document explaining the change. It is really helpful.

I've left one change inline.

Besides that, the fact that the Rate and WallRate classes now require a context to be initialized is a big change from the previous implementation. That doesn't necessarily mean we shouldn't do this, but I do want to have a conversation about it with the maintainers. @fujitatomoya @alsora @wjwwood what do you think about this change in semantics?

that is true, after this PR, context must be initialized before using those classes. i think that would not be a problem for ROS 2 applications. and depending on ROS 2 Clock objects makes sense to me to support ROSRate.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I've left a couple more small things to fix, then I think this will be good.

As far as:

Do we need to add similar checks for obsolete GenericRate constructors, or better leave it as-is, making changes only in working API?

I'm fine with only adding the checks to the new API, and leaving GenericRate as-is.

rclcpp/include/rclcpp/rate.hpp Outdated Show resolved Hide resolved
rclcpp/include/rclcpp/rate.hpp Outdated Show resolved Hide resolved
@clalancette
Copy link
Contributor

clalancette commented Aug 28, 2023

Here's another round of CI to see where we are with this:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

rclcpp/include/rclcpp/rate.hpp Show resolved Hide resolved
rclcpp/include/rclcpp/rate.hpp Show resolved Hide resolved
rclcpp/include/rclcpp/rate.hpp Show resolved Hide resolved

TEST(TestRate, incorrect_constuctor) {
// Constructor with 0-frequency
try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we use EXPECT_THROW here?

Copy link
Contributor Author

@AlexeyMerzlyakov AlexeyMerzlyakov Aug 29, 2023

Choose a reason for hiding this comment

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

Yes, there is needed macro in utils/rclcpp_gtest_macros.hpp. Switched to it in the following commit. Thanks for notice!

Copy link
Collaborator

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Beyond comments that were made, this looks good to me!

AlexeyMerzlyakov and others added 4 commits August 29, 2023 09:45
Co-authored-by: Chris Lalancette <[email protected]>
Signed-off-by: Alexey Merzlyakov <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Signed-off-by: Alexey Merzlyakov <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Signed-off-by: Alexey Merzlyakov <[email protected]>
@AlexeyMerzlyakov
Copy link
Contributor Author

Oops, seems like GitHub auto-commits broken the DCO signature test. Rebasing it with adding the signature and force-pushing the branch.

@clalancette
Copy link
Contributor

Let's see if Windows is happier with this:

  • Windows Build Status

@clalancette
Copy link
Contributor

The good news is that it looks like we got beyond the compiling phase on Windows. So I'm optimistic that this will be OK there. I'm going to kick of the rest of CI here to parallelize things:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@clalancette
Copy link
Contributor

So we are getting closer, but unfortunately Windows says:

non dll-interface class 'rclcpp::RateBase' used as base for dll-interface class 'rclcpp::Rate' (compiling source file C:\ci\ws\src\ros2\demos\action_tutorials\action_tutorials_cpp\src\fibonacci_action_server.cpp)

I'm honestly not sure what that means; that is a new one for me. Maybe we need RCLCPP_PUBLIC on RateBase? That's not a pattern I'm familiar with, but that may be what it means.

Signed-off-by: Chris Lalancette <[email protected]>
@clalancette
Copy link
Contributor

clalancette commented Aug 31, 2023

OK, I think my latest commit (bff4fd0) should fix things on Windows. Let's run CI again and see if that is the case:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

@AlexeyMerzlyakov
Copy link
Contributor Author

AlexeyMerzlyakov commented Aug 31, 2023

@clalancette, thank you for helping with Windows builds!
The latest Windows build fail seems really strange and related to some CI / cache issue:

03:59:06   * seven_zip_archive[iconv] action extractc:/temp/local-mode-cache/cache/cookbooks/windows/libraries/windows_helper.rb:74:in `cached_file': URI.unescape is obsolete (StructuredWarnings::StandardWarning)

However, Clang builds give linking problems (is it related to the latest commit?)

04:37:36 [  8%] Building CXX object CMakeFiles/rclcpp.dir/src/rclcpp/any_executable.cpp.o
04:37:36 In file included from /home/jenkins-agent/workspace/ci_linux_clang_libcxx/ws/src/ros2/rclcpp/rclcpp/src/rclcpp/any_executable.cpp:15:
04:37:36 In file included from /home/jenkins-agent/workspace/ci_linux_clang_libcxx/ws/src/ros2/rclcpp/rclcpp/include/rclcpp/any_executable.hpp:20:
04:37:36 In file included from /home/jenkins-agent/workspace/ci_linux_clang_libcxx/ws/src/ros2/rclcpp/rclcpp/include/rclcpp/callback_group.hpp:30:
04:37:36 In file included from /home/jenkins-agent/workspace/ci_linux_clang_libcxx/ws/src/ros2/rclcpp/rclcpp/include/rclcpp/timer.hpp:31:
04:37:36 /home/jenkins-agent/workspace/ci_linux_clang_libcxx/ws/src/ros2/rclcpp/rclcpp/include/rclcpp/rate.hpp:43:3: error: an attribute list cannot appear here
04:37:36   [[deprecated("use get_type() instead")]] virtual bool is_steady() const = 0;

Unfortunately, I checked only Linux builds when prepared the commit, so missed this point. If Windows will fail, on next week I'll try to get out with Windows builds and check the problem as well

@clalancette
Copy link
Contributor

The latest Windows build fail seems really strange and related to some CI / cache issue:

Yeah, it looks like the network was having problems there. I'll kick them off again.

However, Clang builds give linking problems (is it related to the latest commit?)

Yeah, most probably. We'll have to take a closer look at building with clang.

Signed-off-by: Chris Lalancette <[email protected]>
@clalancette
Copy link
Contributor

clalancette commented Aug 31, 2023

I think f54ca7d should fix it on clang. Let's see if that is happy across the board:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • RHEL Build Status
  • Clang Build Status

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Thanks for all of the back-and-forth here @AlexeyMerzlyakov . With the latest changes, this looks good to me. I'm going to go ahead and merge, along with ros2/system_tests#516

@clalancette clalancette merged commit bc43577 into ros2:rolling Aug 31, 2023
2 checks passed
@clalancette
Copy link
Contributor

Just as an FYI, I released this into Rolling yesterday and I think there was some downstream fallout from this change. In particular, look at:

All of those failures revolve around rclcpp::Rate, which makes me suspect this. I'm going to take a look, but @AlexeyMerzlyakov it would be helpful if you could look as well.

@clalancette
Copy link
Contributor

OK, I found it. See #2301 which fixes it.

Barry-Xu-2018 pushed a commit to Barry-Xu-2018/rclcpp that referenced this pull request Jan 12, 2024
* 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 <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
CursedRock17 pushed a commit to CursedRock17/rclcpp that referenced this pull request Apr 2, 2024
Signed-off-by: methylDragon <[email protected]>

Fix a format-security warning when building with clang (ros2#2171)

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 <[email protected]>

Add missing stdexcept include (ros2#2186)

Signed-off-by: Øystein Sture <[email protected]>

Fix race condition in events-executor (ros2#2177)

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 <[email protected]>

Changelog

Signed-off-by: Yadunund <[email protected]>

21.1.1

Declare rclcpp callbacks before the rcl entities (ros2#2024)

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 <[email protected]>
Co-authored-by: Mauro Passerino <[email protected]>

add mutex to protect events_executor current entity collection (ros2#2187)

* add mutex to protect events_executor current entity collection and unit-test

Signed-off-by: Alberto Soragna <[email protected]>

* be more precise with mutex locks; make stress test less stressfull

Signed-off-by: Alberto Soragna <[email protected]>

* fix uncrustify error

Signed-off-by: Alberto Soragna <[email protected]>

---------

Signed-off-by: Alberto Soragna <[email protected]>

Feature/available capacity of ipm (ros2#2173)

* added available_capacity to get the lowest number of free capacity for intra-process communication for a publisher

Signed-off-by: Joshua Hampp <[email protected]>

* added unit tests for available_capacity

Signed-off-by: Joshua Hampp <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>

* fixed typos in comments

Signed-off-by: Joshua Hampp <[email protected]>

* Updated warning

Co-authored-by: Alberto Soragna <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>

* returning 0 if ipm is disabled in lowest_available_ipm_capacity

Signed-off-by: Joshua Hampp <[email protected]>

* return 0 if no subscribers are present in lowest_available_capacity

Signed-off-by: Joshua Hampp <[email protected]>

* updated unit test

Signed-off-by: Joshua Hampp <[email protected]>

* update unit test

Signed-off-by: Joshua Hampp <[email protected]>

* moved available_capacity to a lambda function to be able to handle subscriptions which went out of scope

Signed-off-by: Joshua Hampp <[email protected]>

* updated unit test to check subscriptions which went out of scope

Signed-off-by: Joshua Hampp <[email protected]>

---------

Signed-off-by: Joshua Hampp <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>
Co-authored-by: Joshua Hampp <[email protected]>
Co-authored-by: Joshua Hampp <[email protected]>
Co-authored-by: Alberto Soragna <[email protected]>

remove nolint since ament_cpplint updated for the c++17 header (ros2#2198)

Signed-off-by: Chen Lihui <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

21.2.0

Use TRACETOOLS_ prefix for tracepoint-related macros (ros2#2162)

Signed-off-by: Christophe Bedard <[email protected]>

Remove flaky stressAddRemoveNode test (ros2#2206)

Signed-off-by: Michael Carroll <[email protected]>

Fix up misspellings of "receive". (ros2#2208)

Signed-off-by: Chris Lalancette <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

21.3.0

Enable callback group tests for connextdds (ros2#2182)

* 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 <[email protected]>

Modifies timers API to select autostart state (ros2#2005)

* Modifies timers API to select autostart state

* Removes unnecessary variables

* Adds autostart documentation and expands some timer test

Signed-off-by: Voldivh <[email protected]>

warning: comparison of integer expressions of different signedness (ros2#2219)

  ros2#2167 (comment)

Signed-off-by: Tomoya Fujita <[email protected]>

Revamp the test_subscription.cpp tests. (ros2#2227)

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 <[email protected]>

Move always_false_v to detail namespace (ros2#2232)

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 <[email protected]>

Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (ros2#2224)

* 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 <[email protected]>
Signed-off-by: William Woodall <[email protected]>

Switch lifecycle to use the RCLCPP macros. (ros2#2233)

This ensures that they'll go out to /rosout and the disk.

Signed-off-by: Chris Lalancette <[email protected]>

Implement get_node_type_descriptions_interface for lifecyclenode and add smoke test for it (ros2#2237)

Signed-off-by: Emerson Knapp <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

22.0.0

Stop using constref signature of benchmark DoNotOptimize. (ros2#2238)

* 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 <[email protected]>

Instrument loaned message publication code path (ros2#2240)

Signed-off-by: Christophe Bedard <[email protected]>

associated clocks should be protected by mutex. (ros2#2255)

Signed-off-by: Tomoya Fujita <[email protected]>

Change associated clocks storage to unordered_set (ros2#2257)

Signed-off-by: Luca Della Vedova <[email protected]>

Adding Missing Group Exceptions (ros2#2256)

* Adding Missing Group Exceptions

Signed-off-by: CursedRock17 <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Co-authored-by: Tomoya Fujita <[email protected]>

Add spin_all shortcut (ros2#2246)

Signed-off-by: Tony Najjar <[email protected]>

Remove an unused variable from the events executor tests. (ros2#2270)

Signed-off-by: Chris Lalancette <[email protected]>

Add a pimpl inside rclcpp::Node for future distro backports (ros2#2228)

* Add a pimpl inside rclcpp::Node for future distro backports

Signed-off-by: Emerson Knapp <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>

Adding Custom Unknown Type Error (ros2#2272)

Signed-off-by: CursedRock17 <[email protected]>
Co-authored-by: Christophe Bedard <[email protected]>

Do not crash Executor when send_response fails due to client failure. (ros2#2276)

* Do not crash Executor when send_response fails due to client failure.

Related to ros2/ros2#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 <[email protected]>

* Update rclcpp/include/rclcpp/service.hpp

Co-authored-by: Tomoya Fujita <[email protected]>
Signed-off-by: Zang MingJie <[email protected]>

* address review comments.

Signed-off-by: Tomoya Fujita <[email protected]>

---------

Signed-off-by: Zang MingJie <[email protected]>
Signed-off-by: Tomoya Fujita <[email protected]>
Co-authored-by: Zang MingJie <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

22.1.0

doc fix: call `canceled` only after goal state is in canceling. (ros2#2266)

Signed-off-by: Tomoya Fujita <[email protected]>

Fix a typo in a comment. (ros2#2283)

Signed-off-by: Chris Lalancette <[email protected]>

Revamp list_parameters to be more efficient and easier to read. (ros2#2282)

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 <[email protected]>

Add rcl_logging_interface as an explicit dependency. (ros2#2284)

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 <[email protected]>

add logger level service to lifecycle node. (ros2#2277)

Signed-off-by: Tomoya Fujita <[email protected]>

Remove unnecessary lambda captures in the tests. (ros2#2289)

* Remove unnecessary lambda captures in the tests.

This was pointed out by compiling with clang.

Signed-off-by: Chris Lalancette <[email protected]>

Correct the position of a comment. (ros2#2290)

Signed-off-by: Jiaqi Li <[email protected]>

Make Rate to select the clock to work with (ros2#2123)

* 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 <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>

Fix C++20 allocator construct deprecation (ros2#2292)

Signed-off-by: Guilherme Rodrigues <[email protected]>

Topic correct typeadapter deduction (ros2#2294)

* fix TypeAdapter deduction

Signed-off-by: Chen Lihui <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

22.2.0

Cleanup flaky timers_manager tests. (ros2#2299)

* 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 <[email protected]>

fix(ClientGoalHandle): Made mutex recursive to prevent deadlocks (ros2#2267)

* 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 <[email protected]>
Co-authored-by: Tomoya Fujita <[email protected]>

Update API docs links in package READMEs (ros2#2302)

Signed-off-by: Christophe Bedard <[email protected]>

Fix the return type of Rate::period. (ros2#2301)

In a recent commit (bc43577),
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 <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

23.0.0

fix the depth to relative in list_parameters (ros2#2300)

* fix the depth to relative in list_parameters

Signed-off-by: leeminju531 <[email protected]>

Decouple rosout publisher init from node init. (ros2#2174)

Signed-off-by: Tomoya Fujita <[email protected]>

Documentation for list_parameters  (ros2#2315)

* list_parameters documentation

Signed-off-by: CursedRock17 <[email protected]>

Removing Old Connext Tests (ros2#2313)

* Removing Old Connext Tests

Signed-off-by: CursedRock17 <[email protected]>

Update SignalHandler get_global_signal_handler to avoid complex types in static memory (ros2#2316)

* Update SignalHandler get_global_signal_handler to avoid complex types in static memory

This was flagged by msan as a problem.

There's a description of why this is a potential problem here: https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables

Signed-off-by: Tully Foote <[email protected]>
Co-authored-by: William Woodall <[email protected]>

Add locking to protect the TimeSource::NodeState::node_base_ (ros2#2320)

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 <[email protected]>

Add missing header required by the rclcpp::NodeOptions type (ros2#2324)

Signed-off-by: Ignacio Vizzo <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

23.1.0

Adding API to copy all parameters from one node to another (ros2#2304)

Signed-off-by: stevemacenski <[email protected]>

remove invalid sized allocation test for SerializedMessage. (ros2#2330)

Signed-off-by: Tomoya Fujita <[email protected]>

add clients & services count (ros2#2072)

* add clients & services count

* add count clients,services tests

Signed-off-by: leeminju531 <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

23.2.0

Fix rclcpp_lifecycle inclusion on Windows. (ros2#2331)

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 <[email protected]>

Remove useless ROSRate class (ros2#2326)

Signed-off-by: Alexey Merzlyakov <[email protected]>

Fixes pointed out by the clang analyzer. (ros2#2339)

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 <[email protected]>

address rate related flaky tests. (ros2#2329)

Signed-off-by: Tomoya Fujita <[email protected]>

Adjust rclcpp usage of type description service (ros2#2344)

Signed-off-by: Michael Carroll <[email protected]>

Add missing 'enable_rosout' comments (ros2#2345)

Signed-off-by: Jiaqi Li <[email protected]>

Use message_info in SubscriptionTopicStatistics instead of typed message (ros2#2337)

* 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 <[email protected]>
Co-authored-by: Tomoya Fujita <[email protected]>

Disable the loaned messages inside the executor. (ros2#2335)

* 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 <[email protected]>

Add a custom deleter when constructing rcl_service_t (ros2#2351)

* 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 <[email protected]>

Serialized Messages with Topic Statistics (ros2#2274)

Signed-off-by: CursedRock17 <[email protected]>

rclcpp::Time::max() clock type support. (ros2#2352)

Signed-off-by: Tomoya Fujita <[email protected]>

Updates to not use std::move in some places. (ros2#2353)

gcc 13.1.1 complains that these uses inhibit copy elision.

Signed-off-by: Chris Lalancette <[email protected]>

fix (signal_handler.hpp): spelling (ros2#2356)

Signed-off-by: Zard-C <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

24.0.0

fix(rclcpp_components): increase the service queue sizes in component_container (ros2#2363)

* fix(rclcpp_components): increase the service queue sizes in component_container

Signed-off-by: M. Fatih Cırıt <[email protected]>

Support users holding onto shared pointers in the message memory pool (ros2#2336)

* 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 <[email protected]>

Fix data race in EventHandlerBase (ros2#2349)

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 <[email protected]>

aligh with rcl that a rosout publisher of a node might not exist (ros2#2357)

* 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 <[email protected]>

feat(rclcpp_components): support events executor in node main template (ros2#2366)

Signed-off-by: wep21 <[email protected]>

Switch to target_link_libraries. (ros2#2374)

That way we can hide more of the implementation by using
the PRIVATE keyword.

Signed-off-by: Chris Lalancette <[email protected]>

Adding QoS to subscription options (ros2#2323)

* Adding QoS to subscription options

Signed-off-by: CursedRock17 <[email protected]>

make type support helper supported for service (ros2#2209)

* make type support helper supported for service and action as well

Signed-off-by: Chen Lihui <[email protected]>

* not to use template and only add the necessary service type currently

Signed-off-by: Chen Lihui <[email protected]>

* update comment

Signed-off-by: Chen Lihui <[email protected]>

* add deprecated cycle for `get_typesupport_handle`

Signed-off-by: Chen Lihui <[email protected]>

---------

Signed-off-by: Chen Lihui <[email protected]>

Updated GenericSubscription to AnySubscriptionCallback (ros2#1928)

* added rclcpp::SerializedMessage support for AnySubscriptionCallback

Signed-off-by: Joshua Hampp <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>

* using AnySubscription callback for generic subscriptiion

Signed-off-by: Joshua Hampp <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>

* updated tests

Signed-off-by: Joshua Hampp <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>

* Remove comment

Signed-off-by: Joshua Hampp <[email protected]>

---------

Signed-off-by: Joshua Hampp <[email protected]>
Signed-off-by: Joshua Hampp <[email protected]>
Co-authored-by: Joshua Hampp <[email protected]>
Co-authored-by: Jacob Perron <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

25.0.0

Increase timeout for rclcpp_lifecycle to 360 (ros2#2395)

* Increase timeout for rclcpp_lifecycle to 360

Signed-off-by: Jorge Perez <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>

Stop storing the context in the guard condition. (ros2#2400)

* 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 <[email protected]>

Add transient local durability support to publisher and subscriptions when using intra-process communication (ros2#2303)

* Add intra process transient local durability support to publisher and subscription

Signed-off-by: Jeffery Hsu <[email protected]>

* Remove durability_is_transient_local_ from publisher_base
Signed-off-by: Jeffery Hsu <[email protected]>

* Design changes that move most transient local publish functionalities out of
intra process manager into intra process manager

Signed-off-by: Jeffery Hsu <[email protected]>

* Move transient local publish to a separate function

Signed-off-by: Jeffery Hsu <[email protected]>

* Remove publisher buffer weak ptr from intra process manager when it associated publisher is removed.

Signed-off-by: Jeffery Hsu <[email protected]>

* Remove incorrectly placed RCLCPP_PUBLIC

Signed-off-by: Jeffery Hsu <[email protected]>

* Add missing RCLCPP_PUBLIC

Signed-off-by: Jeffery Hsu <[email protected]>

* Expand RingBufferImplementation beyond shared_ptr and unique_ptr

Signed-off-by: Jeffery Hsu <[email protected]>

* Comment and format fix

Signed-off-by: Jeffery Hsu <[email protected]>

---------

Signed-off-by: Jeffery Hsu <[email protected]>

Increase the cppcheck timeout to 600 seconds. (ros2#2409)

Signed-off-by: Chris Lalancette <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

26.0.0

Make sure to mark RingBuffer methods as 'override'. (ros2#2410)

This gets rid of a warning when building under clang.

Signed-off-by: Chris Lalancette <[email protected]>

Removed deprecated header (ros2#2413)

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

[events executor] - Fix Behavior with Timer Cancel (ros2#2375)

* fix

Signed-off-by: Matt Condino <[email protected]>

* add timer cancel tests

Signed-off-by: Matt Condino <[email protected]>

* cleanup header include

Signed-off-by: Matt Condino <[email protected]>

* reverting change to timer_greater function

Signed-off-by: Gus Brigantino <[email protected]>

* use std::optional, and handle edgecase of 1 cancelled timer

Signed-off-by: Matt Condino <[email protected]>

* clean up run_timers func

Signed-off-by: Gus Brigantino <[email protected]>

* some fixes and added tests for cancel then reset of timers.

Signed-off-by: Matt Condino <[email protected]>

* refactor and clean up. remove cancelled timer tracking.

Signed-off-by: Matt Condino <[email protected]>

* remove unused method for size()

Signed-off-by: Matt Condino <[email protected]>

* linting

Signed-off-by: Matt Condino <[email protected]>

* relax timing constraints in tests

Signed-off-by: Matt Condino <[email protected]>

* further relax timing constraints to ensure windows tests are not flaky.

Signed-off-by: Matt Condino <[email protected]>

* use sim clock for tests, pub clock at .25 realtime rate.

Signed-off-by: Matt Condino <[email protected]>

---------

Signed-off-by: Matt Condino <[email protected]>
Signed-off-by: Gus Brigantino <[email protected]>
Co-authored-by: Gus Brigantino <[email protected]>

Split test_executors up into smaller chunks. (ros2#2421)

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 <[email protected]>

Changelog.

Signed-off-by: Chris Lalancette <[email protected]>

27.0.0

feat: add/minus for msg::Time and rclcpp::Duration (ros2#2419)

* feat: add/minus for msg::Time and rclcpp::Duration

Signed-off-by: HuaTsai <[email protected]>

crash on no class found (ros2#2415)

* crash on no class found

* error on no class found instead of no callback groups

Signed-off-by: Adam Aposhian <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>

Update quality declaration documents (ros2#2427)

Signed-off-by: Christophe Bedard <[email protected]>

Set hints to find the python version we actually want. (ros2#2426)

The comment in the commit explains the reasoning behind it.

Signed-off-by: Chris Lalancette <[email protected]>

fix doxygen syntax for NodeInterfaces (ros2#2428)

Signed-off-by: Jonas Otto <[email protected]>

Remove the set_deprecated signatures in any_subscription_callback. (ros2#2431)

These have been deprecated since April 2021, so it is safe
to remove them now.

Signed-off-by: Chris Lalancette <[email protected]>

Add EXECUTOR docs (ros2#2440)

Signed-off-by: Ruddick Lawrence <[email protected]>

Various cleanups to deal with uncrustify 0.78. (ros2#2439)

These should also work with uncrustify 0.72.

Signed-off-by: Chris Lalancette <[email protected]>

Rule of five: implement move operators (ros2#2425)

Signed-off-by: Tim Clephas <[email protected]>

Implement generic client (ros2#2358)

* Implement generic client

Signed-off-by: Barry Xu <[email protected]>

* Fix the incorrect parameter declaration

Signed-off-by: Barry Xu <[email protected]>

* Deleted copy constructor and assignment for FutureAndRequestId

Signed-off-by: Barry Xu <[email protected]>

* Update codes after rebase

Signed-off-by: Barry Xu <[email protected]>

* Address review comments

Signed-off-by: Barry Xu <[email protected]>

* Address review comments from iuhilnehc-ynos

Signed-off-by: Barry Xu <[email protected]>

* Correct an error in a description

Signed-off-by: Barry Xu <[email protected]>

* Fix window build errors

Signed-off-by: Barry Xu <[email protected]>

* Address review comments from William

Signed-off-by: Barry Xu <[email protected]>

* Add doc strings to create_generic_client

Signed-off-by: Barry Xu <[email protected]>

---------

Signed-off-by: Barry Xu <[email protected]>

Fix TypeAdapted publishing with large messages. (ros2#2443)

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 <[email protected]>

Modify rclcpp_action::GoalUUID hashing algorithm (ros2#2441)

* Add unit tests for hashing rclcpp_action::GoalUUID's

* Use the FNV-1a hash algorithm for Goal UUID

Signed-off-by: Mauro Passerino <[email protected]>

relax the test simulation rate for timer canceling tests. (ros2#2453)

Signed-off-by: Tomoya Fujita <[email protected]>

Revert "relax the test simulation rate for timer canceling tests. (ros2#2453)" (ros2#2456)

This reverts commit 1c350d0.

Signed-off-by: Tomoya Fujita <[email protected]>

enable simulation clock for timer canceling test. (ros2#2458)

* enable simulation clock for timer canceling test.

Signed-off-by: Tomoya Fujita <[email protected]>

* move MainExecutorTypes to test_executors_timer_cancel_behavior.cpp.

Signed-off-by: Tomoya Fujita <[email protected]>

---------

Signed-off-by: Tomoya Fujita <[email protected]>

refactor and improve the parameterized spin_some tests for executors (ros2#2460)

* refactor and improve the spin_some parameterized tests for executors

Signed-off-by: William Woodall <[email protected]>

* disable spin_some_max_duration for the StaticSingleThreadedExecutor and EventsExecutor

Signed-off-by: William Woodall <[email protected]>

* fixup and clarify the docstring for Executor::spin_some()

Signed-off-by: William Woodall <[email protected]>

* style

Signed-off-by: William Woodall <[email protected]>

* review comments

Signed-off-by: William Woodall <[email protected]>

---------

Signed-off-by: William Woodall <[email protected]>

fix spin_some_max_duration unit-test for events-executor (ros2#2465)

Signed-off-by: Alberto Soragna <[email protected]>

Do not generate the exception when action service response timeout. (ros2#2464)

* Do not generate the exception when action service response timeout.

Signed-off-by: Tomoya Fujita <[email protected]>

* address review comment.

Signed-off-by: Tomoya Fujita <[email protected]>

---------

Signed-off-by: Tomoya Fujita <[email protected]>

Changelog.

Signed-off-by: Marco A. Gutierrez <[email protected]>

28.0.0

Signed-off-by: Marco A. Gutierrez <[email protected]>

fix flakiness in TestTimersManager unit-test (ros2#2468)

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 <[email protected]>

Utilize rclcpp::WaitSet as part of the executors (ros2#2142)

* Deprecate callback_group call taking context

Signed-off-by: Michael Carroll <[email protected]>

* Add base executor objects that can be used by implementors

Signed-off-by: Michael Carroll <[email protected]>

* Template common operations

Signed-off-by: Michael Carroll <[email protected]>

* Address reviewer feedback:

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

Signed-off-by: Michael Carroll <[email protected]>

* Lint

Signed-off-by: Michael Carroll <[email protected]>

* Address reviewer feedback and fix templates

Signed-off-by: Michael Carroll <[email protected]>

* Lint and docs

Signed-off-by: Michael Carroll <[email protected]>

* Make executor own the notify waitable

Signed-off-by: Michael Carroll <[email protected]>

* Add pending queue to collector, remove from waitable

Also change node's get_guard_condition to return shared_ptr

Signed-off-by: Michael Carroll <[email protected]>

* Change interrupt guard condition to shared_ptr

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

Signed-off-by: Michael Carroll <[email protected]>

* Lint and docs

Signed-off-by: Michael Carroll <[email protected]>

* Utilize rclcpp::WaitSet as part of the executors

Signed-off-by: Michael Carroll <[email protected]>

* Don't exchange atomic twice

Signed-off-by: Michael Carroll <[email protected]>

* Fix add_node and add more tests

Signed-off-by: Michael Carroll <[email protected]>

* Make get_notify_guard_condition follow API tick-tock

Signed-off-by: Michael Carroll <[email protected]>

* Improve callback group tick-tocking

Signed-off-by: Michael Carroll <[email protected]>

* Don't lock twice

Signed-off-by: Michael Carroll <[email protected]>

* Address reviewer feedback

Signed-off-by: Michael Carroll <[email protected]>

* Add thread safety annotations and make locks consistent

Signed-off-by: Michael Carroll <[email protected]>

* @wip

Signed-off-by: Michael Carroll <[email protected]>

* Reset callback groups for multithreaded executor

Signed-off-by: Michael Carroll <[email protected]>

* Avoid many small function calls when building executables

Signed-off-by: Michael Carroll <[email protected]>

* Re-trigger guard condition if buffer has data

Signed-off-by: Michael Carroll <[email protected]>

* Address reviewer feedback

Signed-off-by: Michael Carroll <[email protected]>

* Trace points

Signed-off-by: Michael Carroll <[email protected]>

* Remove tracepoints

Signed-off-by: Michael Carroll <[email protected]>

* Reducing diff

Signed-off-by: Michael Carroll <[email protected]>

* Reduce diff

Signed-off-by: Michael Carroll <[email protected]>

* Uncrustify

Signed-off-by: Michael Carroll <[email protected]>

* Restore tests

Signed-off-by: Michael Carroll <[email protected]>

* Back to weak_ptr and reduce test time

Signed-off-by: Michael Carroll <[email protected]>

* reduce diff and lint

Signed-off-by: Michael Carroll <[email protected]>

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

Signed-off-by: Michael Carroll <[email protected]>

* Restore more tests

Signed-off-by: Michael Carroll <[email protected]>

* Fix multithreaded test

Signed-off-by: Michael Carroll <[email protected]>

* Fix assert

Signed-off-by: Michael Carroll <[email protected]>

* Fix constructor test

Signed-off-by: Michael Carroll <[email protected]>

* Change ready_executables signature back

Signed-off-by: Michael Carroll <[email protected]>

* Don't enforce removing callback groups before nodes

Signed-off-by: Michael Carroll <[email protected]>

* Remove the "add_valid_node" API

Signed-off-by: Michael Carroll <[email protected]>

* Only notify if the trigger condition is valid

Signed-off-by: Michael Carroll <[email protected]>

* Only trigger if valid and needed

Signed-off-by: Michael Carroll <[email protected]>

* Fix spin_some/spin_all implementation

Signed-off-by: Michael Carroll <[email protected]>

* Restore single threaded executor

Signed-off-by: Michael Carroll <[email protected]>

* Picking ABI-incompatible executor changes

Signed-off-by: Michael Carroll <[email protected]>

* Add PIMPL

Signed-off-by: Michael Carroll <[email protected]>

* Additional waitset prune

Signed-off-by: Michael Carroll <[email protected]>

* Fix bad merge

Signed-off-by: Michael Carroll <[email protected]>

* Expand test timeout

Signed-off-by: Michael Carroll <[email protected]>

* Introduce method to clear expired entities from a collection

Signed-off-by: Michael Carroll <[email protected]>

* Make sure to call remove_expired_entities().

Signed-off-by: Chris Lalancette <[email protected]>

* Prune queued work when callback group is removed

Signed-off-by: Michael Carroll <[email protected]>

* Prune subscriptions from dynamic storage

Signed-off-by: Michael Carroll <[email protected]>

* Styles fixes.

Signed-off-by: Chris Lalancette <[email protected]>

* Re-trigger guard conditions

Signed-off-by: Michael Carroll <[email protected]>

* Condense to just use watiable.take_data

Signed-off-by: Michael Carroll <[email protected]>

* Lint

Signed-off-by: Michael Carroll <[email protected]>

* Address reviewer comments (nits)

Signed-off-by: Michael Carroll <[email protected]>

* Lock mutex when copying

Signed-off-by: Michael Carroll <[email protected]>

* Refactors to static single threaded based on reviewers

Signed-off-by: Michael Carroll <[email protected]>

* More small refactoring

Signed-off-by: Michael Carroll <[email protected]>

* Lint

Signed-off-by: Michael Carroll <[email protected]>

* Lint

Signed-off-by: Michael Carroll <[email protected]>

* Add ready executable accessors to WaitResult

Signed-off-by: Michael Carroll <[email protected]>

* Make use of accessors from wait_set

Signed-off-by: Michael Carroll <[email protected]>

* Fix tests

Signed-off-by: Michael Carroll <[email protected]>

* Fix more tests

Signed-off-by: Michael Carroll <[email protected]>

* Tidy up single threaded executor implementation

Signed-off-by: Michael Carroll <[email protected]>

* Don't null out timer, rely on call

Signed-off-by: Michael Carroll <[email protected]>

* change how timers are checked from wait result in executors

Signed-off-by: William Woodall <[email protected]>

* peak -> peek

Signed-off-by: William Woodall <[email protected]>

* fix bug in next_waitable logic

Signed-off-by: William Woodall <[email protected]>

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

Signed-off-by: William Woodall <[email protected]>

* style

Signed-off-by: William Woodall <[email protected]>

---------

Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: William Woodall <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Co-authored-by: William Woodall <[email protected]>

update rclcpp::Waitable API to use references and const (ros2#2467)

Signed-off-by: William Woodall <[email protected]>

Add tracepoint for generic publisher/subscriber (ros2#2448)

Signed-off-by: h-suzuki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timer callbacks and rate sleep behave differently if sim clock update rate is slow
6 participants