Make sure to reset the trigger.

That is, once we have discovered what the guard condition status is, we should immediately reset the trigger condition and wait for the next one. Otherwise we risk triggering over and over again. Signed-off-by: Chris Lalancette <[email protected]>
ros2 · Jan 23, 2024 · 9e1cd9a · 9e1cd9a
1 parent 2d1e333
commit 9e1cd9a
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 14 deletions.
diff --git a/rmw_zenoh_cpp/src/detail/guard_condition.cpp b/rmw_zenoh_cpp/src/detail/guard_condition.cpp
@@ -52,15 +52,14 @@ void GuardCondition::detach_condition()
 }
 
 ///==============================================================================
-bool GuardCondition::has_triggered() const
+bool GuardCondition::get_and_reset_trigger()
 {
   std::lock_guard<std::mutex> lock(internal_mutex_);
-  return has_triggered_;
-}
+  bool ret = has_triggered_;
 
-///==============================================================================
-void GuardCondition::reset_trigger()
-{
-  std::lock_guard<std::mutex> lock(internal_mutex_);
+  // There is no data associated with the guard condition, so as soon as the callers asks about the
+  // state, we can immediately reset and get ready for the next trigger.
   has_triggered_ = false;
+
+  return ret;
 }
diff --git a/rmw_zenoh_cpp/src/detail/guard_condition.hpp b/rmw_zenoh_cpp/src/detail/guard_condition.hpp
@@ -33,10 +33,7 @@ class GuardCondition final
 
   void detach_condition();
 
-  bool has_triggered() const;
-
-  // Resets has_triggered_ to false.
-  void reset_trigger();
+  bool get_and_reset_trigger();
 
 private:
   mutable std::mutex internal_mutex_;

diff --git a/rmw_zenoh_cpp/src/rmw_zenoh.cpp b/rmw_zenoh_cpp/src/rmw_zenoh.cpp
@@ -3026,8 +3026,10 @@ static bool has_triggered_condition(
   if (guard_conditions) {
     for (size_t i = 0; i < guard_conditions->guard_condition_count; ++i) {
       GuardCondition * gc = static_cast<GuardCondition *>(guard_conditions->guard_conditions[i]);
-      if (gc != nullptr && gc->has_triggered()) {
-        return true;
+      if (gc != nullptr) {
+        if (gc->get_and_reset_trigger()) {
+          return true;
+        }
       }
     }
   }
@@ -3186,7 +3188,7 @@ rmw_wait(
         gc->detach_condition();
         // According to the documentation for rmw_wait in rmw.h, entries in the
         // array that have *not* been triggered should be set to NULL
-        if (!gc->has_triggered()) {
+        if (!gc->get_and_reset_trigger()) {
           guard_conditions->guard_conditions[i] = nullptr;
         }
       }