Bugfix/lighting command off to off behavior (#1314)

src/bacnet/basic/sys/lighting_command.c

@@ -393,6 +393,42 @@ float lighting_command_operating_range_clamp(
     return clamped_value;
 }
 
+/**
+ * @brief Determine if the values are both within the normalized OFF range
+ * @details The physical output level, or non-normalized range,
+ *  is specified as the linearized percentage (0..100%)
+ *  of the possible light output range with 0.0% being off,
+ *  1.0% being dimmest, and 100.0% being brightest.
+ *  The actual range represents the subset of physical output levels
+ *  defined by Min_Actual_Value and Max_Actual_Value
+ *  (or 1.0 to 100.0% if these properties are not present).
+ *  The normalized range is always 0.0 to 100.0% where
+ *  1.0% = bottom of the actual range and 100.0% = top of the actual range.
+ * @param data - dimmer data structure
+ * @param value1 [in] value to check if it is within the normalized OFF range
+ * @param value2 [in] value to check if it is within the normalized OFF range
+ * @return true if both values are within the normalized OFF range, false
+ * otherwise
+ */
+static bool lighting_command_is_normalized_off_to_off_nolock(
+    struct bacnet_lighting_command_data *data, float value1, float value2)
+{
+    float min_value, max_value;
+
+    /* check normalized range within physical limits */
+    max_value = lighting_command_physical_range_clamp(data->Max_Actual_Value);
+    min_value = lighting_command_physical_range_clamp(data->Min_Actual_Value);
+    if (isgreater(min_value, max_value)) {
+        /* the high and low configured actual values are swapped */
+        min_value = max_value;
+    }
+    if (isless(value1, min_value) && isless(value2, min_value)) {
+        return true;
+    }
+
+    return false;
+}
+
 /**
  * @brief Clamp the value within the normalized ON range 1% to 100%.
  * @details The physical output level, or non-normalized range,
@@ -417,14 +453,13 @@ static float lighting_command_normalized_on_range_clamp_nolock(
     /* clamp range within physical limits */
     max_value = lighting_command_physical_range_clamp(data->Max_Actual_Value);
     min_value = lighting_command_physical_range_clamp(data->Min_Actual_Value);
-    /* valid range check for high and low trim values */
     if (isgreater(min_value, max_value)) {
-        /* swap the trims if they are inverse */
+        /* swap the configured high and low actual values if they are inverse */
         swap_value = min_value;
         min_value = max_value;
         max_value = swap_value;
     }
-    /* clamp value within trim values, if non-zero */
+    /* clamp value within actual min/max values */
     if (isgreater(value, max_value)) {
         value = max_value;
     } else if (isless(value, min_value)) {
@@ -551,7 +586,14 @@ static void lighting_command_fade_handler(
     float target_value;
 
     old_value = data->Tracking_Value;
-    /* clamp Tracking value within the Normalized ON Range */
+    if (lighting_command_is_normalized_off_to_off_nolock(
+            data, old_value, data->Target_Level)) {
+        /* check for OFF to OFF transition */
+        data->Tracking_Value = 0.0f;
+        data->In_Progress = BACNET_LIGHTING_IDLE;
+        data->Lighting_Operation = BACNET_LIGHTS_STOP;
+    } else {
+        /* clamp Target value within the Normalized ON Range */
         target_value = lighting_command_normalized_on_range_clamp_nolock(
             data, data->Target_Level);
         if ((milliseconds >= data->Fade_Time) ||
@@ -582,8 +624,10 @@ static void lighting_command_fade_handler(
             data->In_Progress = BACNET_LIGHTING_FADE_ACTIVE;
         }
         /* clamp Tracking Value inclusively within the Operating Range */
-    data->Tracking_Value = lighting_command_operating_range_clamp_fade_nolock(
+        data->Tracking_Value =
+            lighting_command_operating_range_clamp_fade_nolock(
                 data, data->Tracking_Value, milliseconds);
+    }
     /* notify */
     lighting_command_tracking_value_event(
         data, old_value, data->Tracking_Value);
@@ -611,7 +655,14 @@ static void lighting_command_ramp_handler(
         operating_value;
 
     old_value = data->Tracking_Value;
-    /* clamp Tracking value within the Normalized ON Range */
+    if (lighting_command_is_normalized_off_to_off_nolock(
+            data, old_value, data->Target_Level)) {
+        /* check for OFF to OFF transition */
+        data->Tracking_Value = 0.0f;
+        data->In_Progress = BACNET_LIGHTING_IDLE;
+        data->Lighting_Operation = BACNET_LIGHTS_STOP;
+    } else {
+        /* clamp Target value within the Normalized ON Range */
         target_value = lighting_command_normalized_on_range_clamp_nolock(
             data, data->Target_Level);
         if (!islessgreater(data->Tracking_Value, target_value)) {
@@ -656,8 +707,8 @@ static void lighting_command_ramp_handler(
                 data->Lighting_Operation = BACNET_LIGHTS_STOP;
             }
             /* clamp target within min/max, if needed */
-        step_value =
+            step_value = lighting_command_normalized_on_range_clamp_nolock(
-            lighting_command_normalized_on_range_clamp_nolock(data, step_value);
+                data, step_value);
             if (data->Lighting_Operation == BACNET_LIGHTS_STOP) {
                 if (isless(data->Target_Level, 1.0f)) {
                     /* jump target to OFF if below normalized min */
@@ -675,6 +726,7 @@ static void lighting_command_ramp_handler(
         operating_value = lighting_command_operating_range_clamp_fade_nolock(
             data, data->Tracking_Value, milliseconds);
         data->Tracking_Value = operating_value;
+    }
     /* notify */
     lighting_command_tracking_value_event(
         data, old_value, data->Tracking_Value);

test/bacnet/basic/sys/lighting_command/src/main.c

@@ -277,6 +277,18 @@ static void test_lighting_command_unit(void)
     zassert_true(
         is_float_equal(data.Last_On_Value, 100.0f), "last-on-value=%f",
         data.Last_On_Value);
+    /* off to off should not clamp to the minimum ON level */
+    data.Tracking_Value = 0.0f;
+    Tracking_Value = data.Tracking_Value;
+    target_level = 0.0f;
+    lighting_command_fade_to(&data, target_level, fade_time);
+    milliseconds = fade_time / 2;
+    lighting_command_timer(&data, milliseconds);
+    zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
+    zassert_true(is_float_equal(Tracking_Value, 0.0f), NULL);
+    zassert_true(is_float_equal(data.Tracking_Value, 0.0f), NULL);
+    zassert_true(data.Lighting_Operation == BACNET_LIGHTS_STOP, NULL);
+    zassert_true(is_float_equal(data.Last_On_Value, 100.0f), NULL);
     /* low trim */
     data.Low_Trim_Value = 10.0f;
     target_level = 1.0f;
@@ -610,6 +622,20 @@ static void test_lighting_command_unit(void)
                 "Tracking_Value=%f", Tracking_Value);
         }
     } while (data.Lighting_Operation != BACNET_LIGHTS_STOP);
+    zassert_true(
+        is_float_equal(data.Last_On_Value, data.Max_Actual_Value), NULL);
+    /* off to off should not clamp to the minimum ON level */
+    data.Tracking_Value = 0.0f;
+    Tracking_Value = data.Tracking_Value;
+    target_level = 0.0f;
+    milliseconds = 100;
+    ramp_rate = 1.0f;
+    lighting_command_ramp_to(&data, target_level, ramp_rate);
+    lighting_command_timer(&data, milliseconds);
+    zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
+    zassert_true(is_float_equal(Tracking_Value, 0.0f), NULL);
+    zassert_true(is_float_equal(data.Tracking_Value, 0.0f), NULL);
+    zassert_true(data.Lighting_Operation == BACNET_LIGHTS_STOP, NULL);
     zassert_true(
         is_float_equal(data.Last_On_Value, data.Max_Actual_Value), NULL);