Add null pointer check and comment cleanup

models/vehicle_management/compound_events/include/compound_event_manager.hh

@@ -34,7 +34,15 @@ class CompoundEventsManager : public VehicleEventsManager
     CompoundEventsManager is responsible for the memory-management cleanup /
     destruction of these event / watch-value instances.*/
   std::list<WatchValuesBaseCore*>  managed_triggers; /* (--)
-    List of all triggers being used, used to unlock triggers.*/
+    List of all triggers being managed by the manager. This list is used for:
+    - at the start of each cycle all of these triggers are unlocked; when a
+      trigger on this list trigger is updated, it gets locked so that it only
+      gets evaluated once per cycle; this is useful if a trigger is used in
+      multiple events, and especially so for multi-shot triggers monitoring
+      for something like a variable reaching a maximum value.
+    - when looking for a trigger by name, it is this list that gets searched.
+    Events are updated from their event(s). They do not need to be included in
+    this list, especially if a trigger is used in only one event.*/
   std::list<WatchValuesBaseCore*>  allocated_triggers; /* (--)
     Set of triggers allocated for use by any trigger-set.*/
   EventTriggerSet allocated_triggers_; /* (--)

models/vehicle_management/compound_events/include/event_trigger.hh

@@ -68,7 +68,7 @@ struct EventTriggerBase
     MIN_CONDITIONAL_FIRST,
     ABS_MAX_CONDITIONAL_FIRST,
     ABS_MIN_CONDITIONAL_FIRST,
-    
+
     // options with managed values of reference:
     MAX_LAST,   // triggers when variable reaches a pt >= reference (LAST occurance)
                 // and resets reference to this value.
@@ -133,14 +133,12 @@ struct EventTriggerBase
     The value obtained when the template reference value (or template
     reference pointer) is cast to type double. This defines the threshold
     for logical comparisons made in this class.*/
-  bool locked{false}; /* (--)
-    Boolean used to lock at each cycle, so each WatchValuesBase cannot be 
-    tested more than once per cycle, leading to unintended behavior.*/
 
  public:
   EventTriggerBase() = default;
   void apply_function_modifier();
   bool has_conditional_reference() const;
+  bool has_managed_value() const;
   void set_new_reference();
 };
 
@@ -153,17 +151,13 @@ class EventTrigger : public WatchValuesDelay<T>,
                      public EventTriggerBase
 {
   bool relative_to_activation_ET{false}; /* (--)
-    This is a bit kludgy. relative_to_activation is inherited from
-    WatchValuesDelay (from WatchValuesBase) and is a public flag;
-    for an EventTrigger, this needs to be locked down at activation.
-    relative_to_activation_protected is also inherited from WatchValuesDelay
-    and is set at activation.
-    But this protected variable is used in WatchValuesBase::test_crossing()
-    to reset "reference" based on the current value of variable.
-    We need to reset "reference" based on the current value of "variable"
-    after the modifier function has been applied.
-    So we cannot use relative_to_activation_protected and need a
-    new variable, similar in intent, specifically for EventTrigger.*/
+    This is a protected form of relative_to_activation. used to support
+    applying the function-modified value of the monitored variable as
+    recorded at trigger-activation. It works similarly to the inherited
+    relative_to_activation_protected, but for applying the function-modified
+    value instead of the raw-value.bit kludgy.
+    See documentation section "Pre-comparison Function Application" for more
+    comprehensive discussion.*/
 
  public:
   explicit EventTrigger( const double & delay_ref,
@@ -197,43 +191,52 @@ Purpose:
   WatchValues implementations.
 Notes:
 - Going all the way back to WatchValuesBaseCore, the method test_crossing()
-  is the method-template entry-point for determining whether the event
-  should be triggered. For data types double, float, and any others with
-  the "use_threshold_crossing_trigger" flag set, the call to test_crossing
-  eventually transfers to test_crossing_dbl(double), which evaluates
-  whether the value is on the "triggered" side of the threshold, and sets
-  the event_triggered flag in response.
+    is the method-template entry-point for determining whether the event
+    should be triggered.
+- For data types double, float, and any others with the
+    "use_threshold_crossing_trigger" flag set, the call to test_crossing
+    eventually transfers to test_crossing_dbl(double), which evaluates
+    whether the value is on the "triggered" side of the threshold, and sets
+    the event_triggered flag in response.
 - In WatchValuesDelay, the test_crossing() method is expanded to wrap
-  the delay mechanism around the core WatchValuesBaseCore::test_crossing_dbl
-  functionality.
-- In this model, we utilize the wrapped-up capabilities provided by
-  WatchValuesDelay::test_crossing and overwrite the basic
-  threshold-crossing evaluations in WatchValuesBaseCore::test_crossing_dbl
-  to include the new requirements encapsulated in the enumerations
-  TriggerCondition, FunctionModifier, and DirectionLimit.
-- The reference (threshold) value can be:
-   - Fixed: pre-defined to a specific value
-   - Fixed: computed at trigger-activations to be a pre-defined offset from
-       the value of the monitored variable at trigger-activation.
-   - Variable: using a pointer to access the reference-variable.
-   - Variable: using that pointer to access the extent to which the
-       reference (threshold) value should be offset from the recorded baseline
-       value of the monitored variable at trigger-activation.
-  Case 1, and 2 use the class member value reference_dbl.
-  Case 3 uses the passed argument ref_val.
-  Case 4 uses ref_val + the value stored at activation, which is a
-  post-modified value unknown to WatchValuesBase so cannot be incorporated
-  in WatchValuesBase prior to entry into this method.
+    the delay mechanism around the core WatchValuesBaseCore::test_crossing
+    functionality.
+- In this model (which inherits the WatchValuesDelay functionality), we utilize
+    the wrapped-up capabilities provided by WatchValuesDelay and overwrite the
+    basic threshold-crossing evaluations (as previously implemented in
+    WatchValuesBaseCore::test_crossing_dbl) to include the new requirements
+    encapsulated in the enumerations
+    -  TriggerCondition,
+    -  FunctionModifier, and
+    -  DirectionLimit.
+- The 2nd argument (ref_val) is passed into here from WatchValuesBase.
+    This represents the trigger threshold and can be:
+     1. Fixed: pre-defined to a specific value
+     2. Fixed: computed at trigger-activations from the value of the
+               monitored variable at trigger-activation; allows the application
+               of the same function_modifier to the monitored variable at model
+               activation and now.
+     3. Variable: equal to the value of another sim variable at the current time.
+     4. Variable: equal to the sum of the value of another sim variable and the
+                  value of the monitored variable at trigger-activation, after
+                  applying the function-modifier.
+    Case 1, and 2 use the class member value reference_dbl.
+    Case 3 uses the passed argument ref_val.
+    Case 4 uses ref_val + the value stored at trigger-activation
 ******************************************************************************/
   void test_crossing_dbl( double var_val,
                           double ref_val) override
   {
     if (this->reference_is_variable) {
       if (relative_to_activation_ET) {
+        // Case 4 above in notes:
         reference_dbl = ref_val + variable_dbl_activation;
       } else {
+        // Case 3 above in notes
         reference_dbl = ref_val;
       }
+      // else:
+      // Cases 1 and 2 above in notes, reference_dbl is already known.
     }
 
     variable_dbl = var_val;
@@ -255,7 +258,7 @@ Notes:
     case EQ:
       // FIXME the use of is_near_equal() in this and other cases in this method,
       // with the default ulp of 0.5, tests for actual equality to the precision of
-      // the representation. 
+      // the representation.
       this->event_triggered = MathUtils::is_near_equal(variable_dbl, reference_dbl);
       break;
     case NE:
@@ -485,17 +488,17 @@ Notes:
              (comparison_logic == ABS_MIN_LAST)) {
       reference_dbl = std::abs(variable_dbl);
     }
-    else if ((comparison_logic == MAX_CONDITIONAL_FIRST) || 
+    else if ((comparison_logic == MAX_CONDITIONAL_FIRST) ||
              (comparison_logic == MAX_CONDITIONAL_LAST)) {
       reference_dbl = std::numeric_limits<double>::lowest();
     }
-    else if ((comparison_logic == MIN_CONDITIONAL_FIRST) || 
+    else if ((comparison_logic == MIN_CONDITIONAL_FIRST) ||
              (comparison_logic == MIN_CONDITIONAL_LAST) ||
-             (comparison_logic == ABS_MIN_CONDITIONAL_FIRST) || 
+             (comparison_logic == ABS_MIN_CONDITIONAL_FIRST) ||
              (comparison_logic == ABS_MIN_CONDITIONAL_LAST)) {
       reference_dbl = std::numeric_limits<double>::max();
     }
-    else if ((comparison_logic == ABS_MAX_CONDITIONAL_FIRST) || 
+    else if ((comparison_logic == ABS_MAX_CONDITIONAL_FIRST) ||
              (comparison_logic == ABS_MAX_CONDITIONAL_LAST)) {
       reference_dbl = 0.0;
     }
@@ -522,6 +525,7 @@ Notes:
     }
 
     // All actions complete, set the active flag and exit.
+    WatchValuesDelay<T>::activate();
     this->active = true;
   }
 };

models/vehicle_management/compound_events/include/event_trigger_set.hh

@@ -49,6 +49,7 @@ class EventTriggerSet : public WatchValuesSet
 
   void populate_conditional_reference_trigger_list();
   void update_conditional_trigger_references();
+  void force_multi_shot_for_managed_values();
   WatchValuesBaseCore * find_trigger( const std::string& trigger_name_in);
 };
 #endif

models/vehicle_management/compound_events/src/compound_event.cc

@@ -128,11 +128,15 @@ CompoundEvent::test_crossing()
 }
 
 /*****************************************************************************
-test_crossing
+test_crossing_iterative
 Purpose:
-  FIXME Comment Me
+  Sets the status of a CompoundEvent according to the status of its triggers.
+  This method is iterative in that when moving from an Unarmed state to an
+  armed state, it can call itself if the event allows for simultaneous
+  processing of its arming triggers and other triggers (including action and
+  disarming triggers)An iterative call to process the Event's triggers
 *****************************************************************************/
-void 
+void
 CompoundEvent::test_crossing_iterative()
 {
   switch (status) {
@@ -379,6 +383,11 @@ CompoundEvent::activate()
     status = Armed;
     action_triggers.subscribe();
   }
+
+  //Apply multi_shot to triggers where necessary:
+  // * Action triggers with managed references
+  action_triggers.force_multi_shot_for_managed_values();
+
   WatchValuesDelay::activate();
 
   // Identify which of the triggers has a conditional reference value.

models/vehicle_management/compound_events/src/compound_event_manager.cc

@@ -70,7 +70,8 @@ CompoundEventsManager::update()
 {
   if (!enabled) {return;}
   // Iterate through managed triggers and unlock them for the current sim cycle
-  // (meant to be executed before a trigger is updated, where it will then be locked)
+  // (meant to be executed before a trigger is updated, where it will then be
+  // locked)
   for ( WatchValuesBaseCore * trigger : managed_triggers) {
     trigger->locked = false;
   }
@@ -110,10 +111,15 @@ add_trigger
 Purpose:
   Adds a trigger to the list of managed_triggers, which will enable locking
   and unlocking of the trigger.
+
+  FIXME Turner 2026/07
+  It is not clear why this method takes a pointer rather than a reference.
 *****************************************************************************/
 void
 CompoundEventsManager::add_trigger(WatchValuesBaseCore* new_trigger)
 {
+  if (new_trigger == nullptr) {return;}
+
   managed_triggers.push_back( new_trigger);
   new_trigger->externally_managed = true;
 }

models/vehicle_management/compound_events/src/event_trigger.cc

@@ -70,6 +70,54 @@ EventTriggerBase::has_conditional_reference() const
           (comparison_logic == ABS_MIN_CONDITIONAL_LAST));
 }
 
+/*****************************************************************************
+has_managed_value_reference
+Purpose:
+  Returns a simple flag indicating whether the trigger has
+  managed values of reference.  These should be multi-shot.
+*****************************************************************************/
+bool
+EventTriggerBase::has_managed_value() const
+{
+  switch (comparison_logic) {
+    case MAX_FIRST:
+    case MIN_FIRST:
+    case ABS_MAX_FIRST:
+    case ABS_MIN_FIRST:
+    case MAX_LAST:
+    case MIN_LAST:
+    case ABS_MAX_LAST:
+    case ABS_MIN_LAST:
+    case MAX_CONDITIONAL_FIRST:
+    case MIN_CONDITIONAL_FIRST:
+    case ABS_MAX_CONDITIONAL_FIRST:
+    case ABS_MIN_CONDITIONAL_FIRST:
+    case MAX_CONDITIONAL_LAST:
+    case MIN_CONDITIONAL_LAST:
+    case ABS_MAX_CONDITIONAL_LAST:
+    case ABS_MIN_CONDITIONAL_LAST:
+      return true;
+    case Undefined:
+    case EQ:
+    case NE:
+    case GT:
+    case LT:
+    case GE:
+    case LE:
+    case ABS_EQ:
+    case ABS_GT:
+    case ABS_GE:
+    case ABS_LT:
+    case ABS_LE:
+    case OC:
+    case Change:
+    case Crossing:
+      return false;
+  }
+
+  return false;
+}
+
 /*****************************************************************************
 set_new_reference
 Purpose:

models/vehicle_management/compound_events/src/event_trigger_set.cc

@@ -73,6 +73,24 @@ EventTriggerSet::populate_conditional_reference_trigger_list()
   }
 }
 
+/*****************************************************************************
+Name: force_multi_shot_for_managed_values
+Purpose:
+  Triggers that have managed value references should always have multi_shot
+  forced.  Otherwise, they'll inappropriately stop updating.
+*****************************************************************************/
+void
+EventTriggerSet::force_multi_shot_for_managed_values()
+{
+  for (WatchValuesBaseCore * trigger : triggers) {
+    auto * event_trigger = dynamic_cast<EventTriggerBase*>(trigger);
+      if (event_trigger != nullptr &&
+        event_trigger->has_managed_value()) {
+      trigger->multi_shot = true;
+    }
+  }
+}
+
 /*****************************************************************************
 Name: update_conditional_trigger_references
 Purpose:

models/vehicle_management/compound_events/verif/SIM_verif_prebuilt_multitriggers/include/verif_events_manager.hh

@@ -101,6 +101,7 @@ class VerifCompoundEventsManager : public CompoundEventsManager
     add_event( event);
 
   }
+
   VerifCompoundEventsManager(const VerifCompoundEventsManager&) = delete;
   VerifCompoundEventsManager& operator=(const VerifCompoundEventsManager&) = delete;
 };

models/vehicle_management/compound_events/verif/SIM_verif_trigger_test1/include/verif_events_manager.hh

@@ -44,6 +44,7 @@ class VerifCompoundEventsManager : public CompoundEventsManager
     event.action_triggers.multi_shot = true;
     add_event( event);
   }
+
   VerifCompoundEventsManager(const VerifCompoundEventsManager&) = delete;
   VerifCompoundEventsManager& operator=(const VerifCompoundEventsManager&) = delete;
 };

models/vehicle_management/compound_events/verif/SIM_verif_trigger_test2/include/verif_events_manager.hh

@@ -35,6 +35,7 @@ class VerifCompoundEventsManager : public CompoundEventsManager
     event.action_triggers.add_trigger(trigger);
     add_event( event);
   }
+
   VerifCompoundEventsManager(const VerifCompoundEventsManager&) = delete;
   VerifCompoundEventsManager& operator=(const VerifCompoundEventsManager&) = delete;
 };