diff --git a/rclcpp/test/rclcpp/executors/test_executors.cpp b/rclcpp/test/rclcpp/executors/test_executors.cpp index 85586b90c0..5b3206b131 100644 --- a/rclcpp/test/rclcpp/executors/test_executors.cpp +++ b/rclcpp/test/rclcpp/executors/test_executors.cpp @@ -454,6 +454,14 @@ class TestWaitable : public rclcpp::Waitable (void) data; count_++; std::this_thread::sleep_for(3ms); + try { + std::lock_guard lock(execute_promise_mutex_); + execute_promise_.set_value(); + } catch (const std::future_error & future_error) { + if (future_error.code() != std::future_errc::promise_already_satisfied) { + throw; + } + } } void @@ -480,8 +488,18 @@ class TestWaitable : public rclcpp::Waitable return count_; } + std::future + reset_execute_promise_and_get_future() + { + std::lock_guard lock(execute_promise_mutex_); + execute_promise_ = std::promise(); + return execute_promise_.get_future(); + } + private: bool is_ready_called_before_take_data = false; + std::promise execute_promise_; + std::mutex execute_promise_mutex_; size_t count_ = 0; rclcpp::GuardCondition gc_; }; @@ -668,25 +686,22 @@ TYPED_TEST(TestExecutorsOnlyNode, missing_event) rclcpp::Node::SharedPtr node(this->node); auto callback_group = node->create_callback_group( rclcpp::CallbackGroupType::MutuallyExclusive, - true); + false); + std::chrono::seconds max_spin_duration(2); auto waitable_interfaces = node->get_node_waitables_interface(); auto my_waitable = std::make_shared(); auto my_waitable2 = std::make_shared(); waitable_interfaces->add_waitable(my_waitable, callback_group); waitable_interfaces->add_waitable(my_waitable2, callback_group); - executor.add_node(this->node); + executor.add_callback_group(callback_group, node->get_node_base_interface()); my_waitable->trigger(); my_waitable2->trigger(); - // a node has some default subscribers, that need to get executed first, therefore the loop - for (int i = 0; i < 10; i++) { - executor.spin_once(std::chrono::milliseconds(10)); - if (my_waitable->get_count() > 0) { - // stop execution, after the first waitable has been executed - break; - } + { + auto my_waitable_execute_future = my_waitable->reset_execute_promise_and_get_future(); + executor.spin_until_future_complete(my_waitable_execute_future, max_spin_duration); } EXPECT_EQ(1u, my_waitable->get_count()); @@ -696,17 +711,26 @@ TYPED_TEST(TestExecutorsOnlyNode, missing_event) // This removes my_waitable2 from the list of ready events, and triggers a call to wait_for_work callback_group->can_be_taken_from().exchange(false); - //now there should be no ready event - executor.spin_once(std::chrono::milliseconds(10)); + // now there should be no ready event + { + auto my_waitable2_execute_future = my_waitable2->reset_execute_promise_and_get_future(); + auto future_code = executor.spin_until_future_complete( + my_waitable2_execute_future, + std::chrono::milliseconds(100)); // expected to timeout + EXPECT_EQ(future_code, rclcpp::FutureReturnCode::TIMEOUT); + } EXPECT_EQ(1u, my_waitable->get_count()); EXPECT_EQ(0u, my_waitable2->get_count()); - //unblock the callback group + // unblock the callback group callback_group->can_be_taken_from().exchange(true); - //now the second waitable should get processed - executor.spin_once(std::chrono::milliseconds(10)); + // now the second waitable should get processed + { + auto my_waitable2_execute_future = my_waitable2->reset_execute_promise_and_get_future(); + executor.spin_until_future_complete(my_waitable2_execute_future, max_spin_duration); + } EXPECT_EQ(1u, my_waitable->get_count()); EXPECT_EQ(1u, my_waitable2->get_count());