From e9b163f0426ab30ccc608b7586d68165a5c76a9f Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Tue, 25 Jan 2022 16:13:32 -0800 Subject: [PATCH 1/3] spin_all with a zero timeout. Signed-off-by: Tomoya Fujita --- rclcpp/include/rclcpp/executor.hpp | 3 ++- .../rclcpp/executors/static_single_threaded_executor.hpp | 6 +++--- rclcpp/src/rclcpp/executor.cpp | 4 ++-- .../rclcpp/executors/static_single_threaded_executor.cpp | 4 ++-- rclcpp/test/rclcpp/test_executor.cpp | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/rclcpp/include/rclcpp/executor.hpp b/rclcpp/include/rclcpp/executor.hpp index f3f6e9bfe4..7ff37dbef1 100644 --- a/rclcpp/include/rclcpp/executor.hpp +++ b/rclcpp/include/rclcpp/executor.hpp @@ -305,7 +305,8 @@ class Executor * If the time that waitables take to be executed is longer than the period on which new waitables * become ready, this method will execute work repeatedly until `max_duration` has elapsed. * - * \param[in] max_duration The maximum amount of time to spend executing work. Must be positive. + * \param[in] max_duration The maximum amount of time to spend executing work, must be >= 0. + * \throw throw std::invalid_argument if max_duration is less than 0. * Note that spin_all() may take longer than this time as it only returns once max_duration has * been exceeded. */ diff --git a/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp b/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp index 61da15e125..4e922e704a 100644 --- a/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp +++ b/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp @@ -99,9 +99,9 @@ class StaticSingleThreadedExecutor : public rclcpp::Executor /// Static executor implementation of spin all /** - * This non-blocking function will execute entities until - * timeout or no more work available. If new entities get ready - * while executing work available, they will be executed + * This non-blocking function will execute entities until timeout (must be >= 0) + * or no more work available. + * If new entities get ready while executing work available, they will be executed * as long as the timeout hasn't expired. * * Example: diff --git a/rclcpp/src/rclcpp/executor.cpp b/rclcpp/src/rclcpp/executor.cpp index 6ed3ace557..73b7b8d80d 100644 --- a/rclcpp/src/rclcpp/executor.cpp +++ b/rclcpp/src/rclcpp/executor.cpp @@ -414,8 +414,8 @@ void Executor::spin_some(std::chrono::nanoseconds max_duration) void Executor::spin_all(std::chrono::nanoseconds max_duration) { - if (max_duration <= 0ns) { - throw std::invalid_argument("max_duration must be positive"); + if (max_duration < 0ns) { + throw std::invalid_argument("max_duration must be greater than or equal to 0"); } return this->spin_some_impl(max_duration, true); } diff --git a/rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp b/rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp index 683d300c06..209fcde556 100644 --- a/rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp +++ b/rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp @@ -71,8 +71,8 @@ StaticSingleThreadedExecutor::spin_some(std::chrono::nanoseconds max_duration) void StaticSingleThreadedExecutor::spin_all(std::chrono::nanoseconds max_duration) { - if (max_duration <= std::chrono::nanoseconds(0)) { - throw std::invalid_argument("max_duration must be positive"); + if (max_duration < std::chrono::nanoseconds(0)) { + throw std::invalid_argument("max_duration must be greater than or equal to 0"); } return this->spin_some_impl(max_duration, true); } diff --git a/rclcpp/test/rclcpp/test_executor.cpp b/rclcpp/test/rclcpp/test_executor.cpp index 0b007d519b..bdbb0a1079 100644 --- a/rclcpp/test/rclcpp/test_executor.cpp +++ b/rclcpp/test/rclcpp/test_executor.cpp @@ -248,7 +248,7 @@ TEST_F(TestExecutor, spin_all_invalid_duration) { RCLCPP_EXPECT_THROW_EQ( dummy.spin_all(std::chrono::nanoseconds(-1)), - std::invalid_argument("max_duration must be positive")); + std::invalid_argument("max_duration must be greater than or equal to 0")); } TEST_F(TestExecutor, spin_some_in_spin_some) { From 35cc61cf3c7c287c47192107966a9a8c9d5fa505 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Tue, 8 Feb 2022 09:01:49 -0800 Subject: [PATCH 2/3] add explanation when timeout is equal to 0. Signed-off-by: Tomoya Fujita --- rclcpp/include/rclcpp/executor.hpp | 1 + .../include/rclcpp/executors/static_single_threaded_executor.hpp | 1 + 2 files changed, 2 insertions(+) diff --git a/rclcpp/include/rclcpp/executor.hpp b/rclcpp/include/rclcpp/executor.hpp index 7ff37dbef1..ce1cdd8886 100644 --- a/rclcpp/include/rclcpp/executor.hpp +++ b/rclcpp/include/rclcpp/executor.hpp @@ -306,6 +306,7 @@ class Executor * become ready, this method will execute work repeatedly until `max_duration` has elapsed. * * \param[in] max_duration The maximum amount of time to spend executing work, must be >= 0. + * `0` means it keeps executing until no more work is available. * \throw throw std::invalid_argument if max_duration is less than 0. * Note that spin_all() may take longer than this time as it only returns once max_duration has * been exceeded. diff --git a/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp b/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp index 4e922e704a..e646234248 100644 --- a/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp +++ b/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp @@ -101,6 +101,7 @@ class StaticSingleThreadedExecutor : public rclcpp::Executor /** * This non-blocking function will execute entities until timeout (must be >= 0) * or no more work available. + * If timeout is `0`, this keeps executing until no more work is available. * If new entities get ready while executing work available, they will be executed * as long as the timeout hasn't expired. * From 4dd2b772cf8373fd7bf5032172fa3d59de652ee2 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Thu, 10 Feb 2022 14:04:10 -0800 Subject: [PATCH 3/3] add note that potentially it blocks forever. Signed-off-by: Tomoya Fujita --- rclcpp/include/rclcpp/executor.hpp | 2 +- .../rclcpp/executors/static_single_threaded_executor.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rclcpp/include/rclcpp/executor.hpp b/rclcpp/include/rclcpp/executor.hpp index ce1cdd8886..a2dd124ff0 100644 --- a/rclcpp/include/rclcpp/executor.hpp +++ b/rclcpp/include/rclcpp/executor.hpp @@ -306,7 +306,7 @@ class Executor * become ready, this method will execute work repeatedly until `max_duration` has elapsed. * * \param[in] max_duration The maximum amount of time to spend executing work, must be >= 0. - * `0` means it keeps executing until no more work is available. + * `0` is potentially block forever until no more work is available. * \throw throw std::invalid_argument if max_duration is less than 0. * Note that spin_all() may take longer than this time as it only returns once max_duration has * been exceeded. diff --git a/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp b/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp index e646234248..5294605eaf 100644 --- a/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp +++ b/rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp @@ -101,7 +101,7 @@ class StaticSingleThreadedExecutor : public rclcpp::Executor /** * This non-blocking function will execute entities until timeout (must be >= 0) * or no more work available. - * If timeout is `0`, this keeps executing until no more work is available. + * If timeout is `0`, potentially it blocks forever until no more work is available. * If new entities get ready while executing work available, they will be executed * as long as the timeout hasn't expired. *