Update Last_On_Value logic for Present_Value property (#1205)

* Fixed lighting output object lighting-command last-on-value to only
  be updated with the last value of the Present_Value property that
  was greater than or equal to 1.0%, keeping in mind that the Present_Value
  shall indicate the target level of the operation and not the current
  value.

CHANGELOG.md

@@ -84,6 +84,11 @@ The git repositories are hosted at the following sites:
 
 ### Fixed
 
+* Fixed lighting output object lighting-command last-on-value to only
+  be updated with the last value of the Present_Value property that
+  was greater than or equal to 1.0%, keeping in mind that the Present_Value
+  shall indicate the target level of the operation and not the current
+  value. (#1205)
 * Fixed CreateObject service list-of-initial-values encoding and decoding.
   Changed the data structure to be similar to WriteProperty. (#1199)
 * Fixed lighting-output object blink warn to honor blink-warn-enable.

src/bacnet/basic/object/lo.c

@@ -431,6 +431,11 @@ Present_Value_Set(struct object_data *pObject, float value, unsigned priority)
         (priority != 6 /* reserved */)) {
         priority--;
         BIT_SET(pObject->Priority_Active_Bits, priority);
+        /* Writes to Present_Value at a value greater than 0.0%
+           but less than 1.0% shall be clamped to 1.0%.*/
+        if ((isgreater(value, 0.0)) && (isless(value, 1.0))) {
+            value = 1.0f;
+        }
         pObject->Priority_Array[priority] = value;
         status = true;
     }

src/bacnet/basic/sys/lighting_command.c

@@ -139,6 +139,44 @@ float lighting_command_step_increment_clamp(float step_increment)
     return step_increment;
 }
 
+/**
+ * @brief Calculate the target value for a step down command
+ * @param tracking_value [in] current tracking value
+ * @param step_increment [in] step increment value
+ * @return target value for step down command
+ */
+float lighting_command_step_down_target_value(
+    float tracking_value, float step_increment)
+{
+    float target_value, step_value;
+
+    step_value = lighting_command_step_increment_clamp(step_increment);
+    if (isgreaterequal(tracking_value, step_value)) {
+        target_value = tracking_value - step_value;
+    } else {
+        target_value = 0.0f;
+    }
+
+    return target_value;
+}
+
+/**
+ * @brief Calculate the target value for a step up command
+ * @param tracking_value [in] current tracking value
+ * @param step_increment [in] step increment value
+ * @return target value for step up command
+ */
+float lighting_command_step_up_target_value(
+    float tracking_value, float step_increment)
+{
+    float target_value, step_value;
+
+    step_value = lighting_command_step_increment_clamp(step_increment);
+    target_value = tracking_value + step_value;
+
+    return target_value;
+}
+
 /**
  * @brief Clamp the value within the operating range between low and high
  *  end trim values.
@@ -245,9 +283,6 @@ static void lighting_command_tracking_value_event(
 {
     if (data->Overridden) {
         value = lighting_command_operating_range_clamp(data, value);
-        if (isgreaterequal(value, 1.0)) {
-            data->Last_On_Value = value;
-        }
         lighting_command_tracking_value_notify(data, old_value, value);
         if (data->Overridden_Momentary) {
             data->Overridden = false;
@@ -255,9 +290,6 @@ static void lighting_command_tracking_value_event(
     } else if (!data->Out_Of_Service) {
         data->Overridden_Momentary = false;
         value = lighting_command_operating_range_clamp(data, value);
-        if (isgreaterequal(value, 1.0)) {
-            data->Last_On_Value = value;
-        }
         lighting_command_tracking_value_notify(data, old_value, value);
     } else {
         debug_printf(
@@ -409,13 +441,13 @@ static void lighting_command_ramp_handler(
 static void
 lighting_command_step_up_handler(struct bacnet_lighting_command_data *data)
 {
-    float old_value, target_value, step_value;
+    float old_value, target_value;
 
     old_value = data->Tracking_Value;
-    step_value = lighting_command_step_increment_clamp(data->Step_Increment);
-    /* inhibit ON if the value is already OFF */
     if (isgreaterequal(old_value, data->Min_Actual_Value)) {
-        target_value = old_value + step_value;
+        /* inhibit ON if the value is already OFF */
+        target_value = lighting_command_step_up_target_value(
+            data->Tracking_Value, data->Step_Increment);
         data->Tracking_Value =
             lighting_command_normalized_on_range_clamp(data, target_value);
         data->In_Progress = BACNET_LIGHTING_IDLE;
@@ -437,15 +469,11 @@ lighting_command_step_up_handler(struct bacnet_lighting_command_data *data)
 static void
 lighting_command_step_down_handler(struct bacnet_lighting_command_data *data)
 {
-    float old_value, target_value, step_value;
+    float old_value, target_value;
 
-    old_value = target_value = data->Tracking_Value;
+    old_value = data->Tracking_Value;
-    step_value = lighting_command_step_increment_clamp(data->Step_Increment);
+    target_value = lighting_command_step_down_target_value(
-    if (isgreaterequal(target_value, step_value)) {
+        data->Tracking_Value, data->Step_Increment);
-        target_value -= step_value;
-    } else {
-        target_value = data->Min_Actual_Value;
-    }
     data->Tracking_Value =
         lighting_command_normalized_on_range_clamp(data, target_value);
     data->In_Progress = BACNET_LIGHTING_IDLE;
@@ -466,11 +494,11 @@ lighting_command_step_down_handler(struct bacnet_lighting_command_data *data)
 static void
 lighting_command_step_on_handler(struct bacnet_lighting_command_data *data)
 {
-    float old_value, target_value, step_value;
+    float old_value, target_value;
 
-    old_value = target_value = data->Tracking_Value;
+    old_value = data->Tracking_Value;
-    step_value = lighting_command_step_increment_clamp(data->Step_Increment);
+    target_value = lighting_command_step_up_target_value(
-    target_value += step_value;
+        data->Tracking_Value, data->Step_Increment);
     data->Tracking_Value =
         lighting_command_normalized_range_clamp(data, target_value);
     data->In_Progress = BACNET_LIGHTING_IDLE;
@@ -491,15 +519,11 @@ lighting_command_step_on_handler(struct bacnet_lighting_command_data *data)
 static void
 lighting_command_step_off_handler(struct bacnet_lighting_command_data *data)
 {
-    float old_value, target_value, step_value;
+    float old_value, target_value;
 
-    old_value = target_value = data->Tracking_Value;
+    old_value = data->Tracking_Value;
-    step_value = lighting_command_step_increment_clamp(data->Step_Increment);
+    target_value = lighting_command_step_down_target_value(
-    if (isgreaterequal(target_value, step_value)) {
+        data->Tracking_Value, data->Step_Increment);
-        target_value -= step_value;
-    } else {
-        target_value = 0.0f;
-    }
     data->Tracking_Value =
         lighting_command_normalized_range_clamp(data, target_value);
     data->In_Progress = BACNET_LIGHTING_IDLE;
@@ -697,6 +721,10 @@ void lighting_command_fade_to(
     data->Fade_Time = fade_time;
     data->Lighting_Operation = BACNET_LIGHTS_FADE_TO;
     data->Target_Level = value;
+    if (isgreaterequal(value, 1.0)) {
+        /* the last value that was greater than or equal to 1.0%.*/
+        data->Last_On_Value = value;
+    }
 }
 
 /**
@@ -714,6 +742,10 @@ void lighting_command_ramp_to(
     data->Ramp_Rate = lighting_command_ramp_rate_clamp(ramp_rate);
     data->Lighting_Operation = BACNET_LIGHTS_RAMP_TO;
     data->Target_Level = value;
+    if (isgreaterequal(value, 1.0)) {
+        /* the last value that was greater than or equal to 1.0%.*/
+        data->Last_On_Value = value;
+    }
 }
 
 /**
@@ -727,12 +759,49 @@ void lighting_command_step(
     BACNET_LIGHTING_OPERATION operation,
     float step_increment)
 {
+    float target_value;
+
     if (!data) {
         return;
     }
+    if (((operation == BACNET_LIGHTS_STEP_UP) ||
+         (operation == BACNET_LIGHTS_STEP_DOWN)) &&
+        (!islessgreater(data->Tracking_Value, 0.0))) {
+        /* If the starting level of Tracking_Value is 0.0%,
+        then this operation is ignored. */
+        return;
+    }
     data->Lighting_Operation = operation;
     data->Fade_Time = 0;
     data->Step_Increment = step_increment;
+    /* determine the last-on-value for the given step operation */
+    if (operation == BACNET_LIGHTS_STEP_UP) {
+        target_value = lighting_command_step_up_target_value(
+            data->Tracking_Value, data->Step_Increment);
+        target_value =
+            lighting_command_normalized_on_range_clamp(data, target_value);
+    } else if (operation == BACNET_LIGHTS_STEP_DOWN) {
+        target_value = lighting_command_step_down_target_value(
+            data->Tracking_Value, data->Step_Increment);
+        target_value =
+            lighting_command_normalized_on_range_clamp(data, target_value);
+    } else if (operation == BACNET_LIGHTS_STEP_ON) {
+        target_value = lighting_command_step_up_target_value(
+            data->Tracking_Value, data->Step_Increment);
+        target_value =
+            lighting_command_normalized_range_clamp(data, target_value);
+    } else if (operation == BACNET_LIGHTS_STEP_OFF) {
+        target_value = lighting_command_step_down_target_value(
+            data->Tracking_Value, data->Step_Increment);
+        target_value =
+            lighting_command_normalized_range_clamp(data, target_value);
+    } else {
+        return;
+    }
+    if (isgreaterequal(target_value, 1.0)) {
+        /* the last value that was greater than or equal to 1.0%.*/
+        data->Last_On_Value = target_value;
+    }
 }
 
 /**
@@ -774,6 +843,10 @@ void lighting_command_stop(struct bacnet_lighting_command_data *data)
         return;
     }
     data->Lighting_Operation = BACNET_LIGHTS_STOP;
+    if (isgreaterequal(data->Tracking_Value, 1.0)) {
+        /* the last value that was greater than or equal to 1.0%.*/
+        data->Last_On_Value = data->Tracking_Value;
+    }
 }
 
 /**

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

@@ -137,9 +137,9 @@ static void test_lighting_command_blink_unit(BACNET_LIGHTING_COMMAND_DATA *data)
  * Tests for Dimmer Command
  */
 #if defined(CONFIG_ZTEST_NEW_API)
-ZTEST(lighting_command_tests, test_lighting_command_command_unit)
+ZTEST(lighting_command_tests, test_lighting_command_unit)
 #else
-static void test_lighting_command_command_unit(void)
+static void test_lighting_command_unit(void)
 #endif
 {
     BACNET_LIGHTING_COMMAND_DATA data = { 0 };
@@ -194,6 +194,7 @@ static void test_lighting_command_command_unit(void)
     /* fade up */
     target_level = 100.0f;
     lighting_command_fade_to(&data, 100.0f, fade_time);
+    zassert_true(is_float_equal(data.Last_On_Value, 100.0f), NULL);
     milliseconds = fade_time / 2;
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_FADE_ACTIVE, NULL);
@@ -203,10 +204,12 @@ static void test_lighting_command_command_unit(void)
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(is_float_equal(Tracking_Value, target_level), NULL);
+    zassert_true(is_float_equal(data.Last_On_Value, 100.0f), NULL);
     /* fade down */
     target_level = 0.0f;
     lighting_command_fade_to(&data, target_level, fade_time);
     milliseconds = fade_time / 2;
+    zassert_true(is_float_equal(data.Last_On_Value, 100.0f), NULL);
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_FADE_ACTIVE, NULL);
     zassert_true(
@@ -215,11 +218,15 @@ static void test_lighting_command_command_unit(void)
     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.Last_On_Value, 100.0f), "last-on-value=%f",
+        data.Last_On_Value);
     /* low trim */
     data.Low_Trim_Value = 10.0f;
     target_level = 1.0f;
     milliseconds = 10;
     lighting_command_fade_to(&data, target_level, 0);
+    zassert_true(is_float_equal(data.Last_On_Value, 1.0f), NULL);
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_TRIM_ACTIVE, NULL);
     zassert_true(is_float_equal(Tracking_Value, data.Low_Trim_Value), NULL);
@@ -230,15 +237,18 @@ static void test_lighting_command_command_unit(void)
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(is_float_equal(Tracking_Value, target_level), NULL);
     data.Low_Trim_Value = data.Min_Actual_Value;
+    zassert_true(is_float_equal(data.Last_On_Value, 1.0f), NULL);
     /* high trim */
     data.High_Trim_Value = 90.0f;
     target_level = 100.0f;
     milliseconds = 10;
     lighting_command_fade_to(&data, target_level, 0);
+    zassert_true(is_float_equal(data.Last_On_Value, target_level), NULL);
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_TRIM_ACTIVE, NULL);
     zassert_true(is_float_equal(Tracking_Value, data.High_Trim_Value), NULL);
     data.High_Trim_Value = data.Max_Actual_Value;
+    zassert_true(is_float_equal(data.Last_On_Value, target_level), NULL);
     /* override */
     override_level = 42.0f;
     target_level = 100.0f;
@@ -308,12 +318,16 @@ static void test_lighting_command_command_unit(void)
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(
         is_float_equal(Tracking_Value, target_step + target_level), NULL);
+    zassert_true(
+        is_float_equal(data.Last_On_Value, target_step + target_level), NULL);
     /* clamp to max */
     target_step = 100.0f;
     lighting_command_step(&data, BACNET_LIGHTS_STEP_UP, target_step);
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(is_float_equal(Tracking_Value, data.Max_Actual_Value), NULL);
+    zassert_true(
+        is_float_equal(data.Last_On_Value, data.Max_Actual_Value), NULL);
     /* turn ON, then step UP */
     target_step = 1.0f;
     target_level = 0.0f;
@@ -326,12 +340,15 @@ static void test_lighting_command_command_unit(void)
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(is_float_equal(Tracking_Value, 1.0f), NULL);
+    zassert_true(is_float_equal(data.Last_On_Value, 1.0f), NULL);
     /* clamp to max */
     target_step = 100.0f;
     lighting_command_step(&data, BACNET_LIGHTS_STEP_ON, target_step);
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(is_float_equal(Tracking_Value, data.Max_Actual_Value), NULL);
+    zassert_true(
+        is_float_equal(data.Last_On_Value, data.Max_Actual_Value), NULL);
     /* step DOWN, not off */
     target_step = 1.0f;
     target_level = data.Min_Actual_Value + target_step;
@@ -344,6 +361,8 @@ static void test_lighting_command_command_unit(void)
     lighting_command_timer(&data, milliseconds);
     zassert_true(data.In_Progress == BACNET_LIGHTING_IDLE, NULL);
     zassert_true(is_float_equal(Tracking_Value, data.Min_Actual_Value), NULL);
+    zassert_true(
+        is_float_equal(data.Last_On_Value, data.Min_Actual_Value), NULL);
     /* clamp to min */
     target_step = 100.0f;
     lighting_command_step(&data, BACNET_LIGHTS_STEP_DOWN, target_step);
@@ -362,6 +381,8 @@ static void test_lighting_command_command_unit(void)
     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.Last_On_Value, data.Min_Actual_Value), NULL);
     /* blink warn - immediate off */
     data.Blink.Interval = 0;
     data.Blink.Duration = 0;
@@ -448,6 +469,8 @@ static void test_lighting_command_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.Min_Actual_Value), NULL);
     /* large elapsed timer - ramp up */
     target_level = data.Max_Actual_Value;
     milliseconds = 2000;
@@ -467,6 +490,8 @@ static void test_lighting_command_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);
 
     /* out-of-service */
     target_level = 100.0f;
@@ -564,8 +589,7 @@ ZTEST_SUITE(lighting_command_tests, NULL, NULL, NULL, NULL, NULL);
 void test_main(void)
 {
     ztest_test_suite(
-        lighting_command_tests,
+        lighting_command_tests, ztest_unit_test(test_lighting_command_unit));
-        ztest_unit_test(test_lighting_command_command_unit));
 
     ztest_run_test_suite(lighting_command_tests);
 }