From ce875c004bb66d4d90a5e8c24589514c5a4c5517 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Sat, 25 Apr 2026 10:10:30 -0500 Subject: [PATCH] Bugfix/lighting command off to off behavior (#1314) --- src/bacnet/basic/sys/lighting_command.c | 214 +++++++++++------- .../basic/sys/lighting_command/src/main.c | 26 +++ 2 files changed, 159 insertions(+), 81 deletions(-) diff --git a/src/bacnet/basic/sys/lighting_command.c b/src/bacnet/basic/sys/lighting_command.c index 4f7e8442..ba4a9249 100644 --- a/src/bacnet/basic/sys/lighting_command.c +++ b/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,39 +586,48 @@ static void lighting_command_fade_handler( float target_value; old_value = data->Tracking_Value; - /* clamp Tracking value within the Normalized ON Range */ - target_value = lighting_command_normalized_on_range_clamp_nolock( - data, data->Target_Level); - if ((milliseconds >= data->Fade_Time) || - (!islessgreater(data->Tracking_Value, target_value))) { - /* stop fading */ - if (isless(data->Target_Level, 1.0f)) { - /* jump target to OFF if below normalized min */ - data->Tracking_Value = 0.0f; - } else { - data->Tracking_Value = target_value; - } + 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; - data->Fade_Time = 0; } else { - /* fading */ - x1 = 0.0f; - x2 = (float)milliseconds; - x3 = (float)data->Fade_Time; - if (isless(old_value, data->Min_Actual_Value)) { - y1 = data->Min_Actual_Value; + /* 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) || + (!islessgreater(data->Tracking_Value, target_value))) { + /* stop fading */ + if (isless(data->Target_Level, 1.0f)) { + /* jump target to OFF if below normalized min */ + data->Tracking_Value = 0.0f; + } else { + data->Tracking_Value = target_value; + } + data->In_Progress = BACNET_LIGHTING_IDLE; + data->Lighting_Operation = BACNET_LIGHTS_STOP; + data->Fade_Time = 0; } else { - y1 = old_value; + /* fading */ + x1 = 0.0f; + x2 = (float)milliseconds; + x3 = (float)data->Fade_Time; + if (isless(old_value, data->Min_Actual_Value)) { + y1 = data->Min_Actual_Value; + } else { + y1 = old_value; + } + y3 = target_value; + data->Tracking_Value = linear_interpolate(x1, x2, x3, y1, y3); + data->Fade_Time -= milliseconds; + data->In_Progress = BACNET_LIGHTING_FADE_ACTIVE; } - y3 = target_value; - data->Tracking_Value = linear_interpolate(x1, x2, x3, y1, y3); - data->Fade_Time -= milliseconds; - 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, data->Tracking_Value, milliseconds); } - /* clamp Tracking Value inclusively within the Operating Range */ - 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,70 +655,78 @@ static void lighting_command_ramp_handler( operating_value; old_value = data->Tracking_Value; - /* clamp Tracking 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)) { - /* stop ramping */ - if (isless(data->Target_Level, 1.0f)) { - /* jump target to OFF if below normalized min */ - data->Tracking_Value = 0.0f; - } else { - data->Tracking_Value = target_value; - } + 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 { - ramp_rate = lighting_command_ramp_rate_clamp(data->Ramp_Rate); - /* determine the number of steps */ - if (milliseconds <= 1000) { - /* percent per second */ - steps = linear_interpolate( - 0.0f, (float)milliseconds, 1000.0f, 0.0f, ramp_rate); - } else { - steps = ((float)milliseconds * ramp_rate) / 1000.0f; - } - if (isless(old_value, target_value)) { - step_value = old_value + steps; - if (isgreater(step_value, target_value)) { - /* stop ramping */ - data->Lighting_Operation = BACNET_LIGHTS_STOP; - } - } else if (isgreater(old_value, target_value)) { - if (isgreater(old_value, steps)) { - step_value = old_value - steps; - } else { - step_value = target_value; - } - if (isless(step_value, target_value)) { - /* stop ramping */ - 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)) { /* stop ramping */ - step_value = target_value; - data->Lighting_Operation = BACNET_LIGHTS_STOP; - } - /* clamp target within min/max, if needed */ - step_value = - lighting_command_normalized_on_range_clamp_nolock(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 */ data->Tracking_Value = 0.0f; } else { - data->Tracking_Value = step_value; + data->Tracking_Value = target_value; } data->In_Progress = BACNET_LIGHTING_IDLE; + data->Lighting_Operation = BACNET_LIGHTS_STOP; } else { - data->Tracking_Value = step_value; - data->In_Progress = BACNET_LIGHTING_RAMP_ACTIVE; + ramp_rate = lighting_command_ramp_rate_clamp(data->Ramp_Rate); + /* determine the number of steps */ + if (milliseconds <= 1000) { + /* percent per second */ + steps = linear_interpolate( + 0.0f, (float)milliseconds, 1000.0f, 0.0f, ramp_rate); + } else { + steps = ((float)milliseconds * ramp_rate) / 1000.0f; + } + if (isless(old_value, target_value)) { + step_value = old_value + steps; + if (isgreater(step_value, target_value)) { + /* stop ramping */ + data->Lighting_Operation = BACNET_LIGHTS_STOP; + } + } else if (isgreater(old_value, target_value)) { + if (isgreater(old_value, steps)) { + step_value = old_value - steps; + } else { + step_value = target_value; + } + if (isless(step_value, target_value)) { + /* stop ramping */ + data->Lighting_Operation = BACNET_LIGHTS_STOP; + } + } else { + /* stop ramping */ + step_value = target_value; + data->Lighting_Operation = BACNET_LIGHTS_STOP; + } + /* clamp target within min/max, if needed */ + step_value = lighting_command_normalized_on_range_clamp_nolock( + 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 */ + data->Tracking_Value = 0.0f; + } else { + data->Tracking_Value = step_value; + } + data->In_Progress = BACNET_LIGHTING_IDLE; + } else { + data->Tracking_Value = step_value; + data->In_Progress = BACNET_LIGHTING_RAMP_ACTIVE; + } } + /* clamp Tracking_Value inclusively within the Operating Range */ + operating_value = lighting_command_operating_range_clamp_fade_nolock( + data, data->Tracking_Value, milliseconds); + data->Tracking_Value = operating_value; } - /* clamp Tracking_Value inclusively within the Operating Range */ - 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); diff --git a/test/bacnet/basic/sys/lighting_command/src/main.c b/test/bacnet/basic/sys/lighting_command/src/main.c index 600ee193..63f528ca 100644 --- a/test/bacnet/basic/sys/lighting_command/src/main.c +++ b/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);