mirror of
https://github.com/basnijholt/adaptive-lighting.git
synced 2026-09-11 22:34:04 +02:00
fix: use quantization-aware comparison in skip_redundant_commands filter (#1513)
* fix: use quantization-aware comparison in skip_redundant_commands filter _remove_redundant_attributes() compared target values against light state with exact equality, but many targets can never round-trip exactly through a device with coarser resolution: - brightness: HA's 0-255 scale vs the 0-99 Z-Wave Multilevel Switch scale leaves 156 of 255 targets that never converge (e.g. 230 -> 89 -> 229), - color_temp_kelvin: the kelvin -> mired -> kelvin round trip leaves most kelvin targets off by up to ~21 K at 6500 K (e.g. 5500 -> 182 -> 5495). Such attributes survived the filter and were re-sent every interval forever, which on larger Z-Wave meshes is enough to jam the controller. Compare brightness with a tolerance of 2 (the exact worst case of the 0-99 scale) and color temperature in mired space, where devices actually quantize and where the comparison is exact at every kelvin value. Both are far below the manual-control detection thresholds (BRIGHTNESS_CHANGE = 25, COLOR_TEMP_CHANGE = 100), so they cannot mask a genuine user change. Fixes #1512 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017P2wLQYGQH6npY5op5CKVD * fix: tolerate one mired to cover both floor- and round-based conversions The previous exact mired equality assumed round-based kelvin<->mired conversion, but HA core's color_temperature_kelvin_to_mired() and color_temperature_mired_to_kelvin() both use math.floor, under which a target like 5500 K comes back as 5524 K in a different rounded mired bucket and would never be filtered. Flooring in the comparison instead would merely flip the failure onto integrations that round. Comparing with a tolerance of one mired converges for both conversion schemes (verified by brute force over 1000-10000 K: zero stuck targets under either pipeline) and can hide at most ~2 mireds, far below the ~5.5 mired just-noticeable difference for color temperature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017P2wLQYGQH6npY5op5CKVD --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Bas Nijholt <bas@nijho.lt>
This commit is contained in:
parent
5df9b30d32
commit
6d46b82313
2 changed files with 117 additions and 8 deletions
|
|
@ -45,6 +45,15 @@ BRIGHTNESS_ATTRS = {
|
|||
ATTR_BRIGHTNESS_STEP_PCT,
|
||||
}
|
||||
|
||||
# Worst-case rounding error when Home Assistant's 0-255 brightness scale
|
||||
# round-trips through a device with coarser resolution (e.g., the 0-99 Z-Wave
|
||||
# Multilevel Switch scale). A light cannot report back a value more precise than
|
||||
# its own scale, so exact equality would never hold for such targets and
|
||||
# 'skip_redundant_commands' would keep sending them forever. The tolerance sits
|
||||
# far below the manual-control-detection threshold (BRIGHTNESS_CHANGE = 25), so
|
||||
# it cannot mask a genuine user change.
|
||||
BRIGHTNESS_TOLERANCE = 2
|
||||
|
||||
ServiceData = dict[str, Any]
|
||||
|
||||
|
||||
|
|
@ -113,20 +122,46 @@ def _split_service_call_data(service_data: ServiceData) -> list[ServiceData]:
|
|||
return service_datas
|
||||
|
||||
|
||||
def _is_attribute_satisfied(key: str, value: Any, attributes: dict[str, Any]) -> bool:
|
||||
"""Whether the light's current state already satisfies this target value."""
|
||||
if key not in attributes:
|
||||
return False
|
||||
current = attributes[key]
|
||||
if not isinstance(current, (int, float)) or not isinstance(value, (int, float)):
|
||||
return value == current
|
||||
if key == ATTR_BRIGHTNESS:
|
||||
return abs(value - current) <= BRIGHTNESS_TOLERANCE
|
||||
if key == ATTR_COLOR_TEMP_KELVIN and value > 0 and current > 0:
|
||||
# Compare in mired space: most integrations quantize color temperature
|
||||
# to whole mireds, and the kelvin error of that quantization grows
|
||||
# quadratically with kelvin (~21 K at 6500 K, ~50 K at 10000 K), so no
|
||||
# fixed kelvin tolerance fits the whole range. The tolerance of one
|
||||
# mired absorbs the difference between conversion schemes: HA core's
|
||||
# helpers floor (e.g. 5500 K -> 181 mired -> 5524 K) while some
|
||||
# integrations round (5500 K -> 182 mired -> 5495 K), and no exact
|
||||
# equality converges for both. One mired is far below the ~5.5 mired
|
||||
# just-noticeable difference for color temperature.
|
||||
return abs(round(1_000_000 / value) - round(1_000_000 / current)) <= 1
|
||||
return value == current
|
||||
|
||||
|
||||
def _remove_redundant_attributes(
|
||||
service_data: ServiceData,
|
||||
state: State,
|
||||
) -> ServiceData:
|
||||
"""Filter service data by removing attributes that already equal the given state.
|
||||
"""Filter service data by removing attributes already satisfied by the state.
|
||||
|
||||
Removes all attributes from service call data whose values are already present
|
||||
in the target entity's state.
|
||||
in the target entity's state. Quantized attributes (brightness, color temp) are
|
||||
compared with a small tolerance: a light whose resolution is coarser than Home
|
||||
Assistant's cannot report back the exact value it was given, so exact equality
|
||||
would never hold and the attribute would never be filtered.
|
||||
"""
|
||||
attributes: dict[str, Any] = dict(state.attributes)
|
||||
return {
|
||||
k: v
|
||||
for k, v in service_data.items()
|
||||
if k not in attributes or v != attributes[k]
|
||||
if not _is_attribute_satisfied(k, v, attributes)
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -95,14 +95,82 @@ async def test_split_service_call_data(input_data, expected_data_list):
|
|||
),
|
||||
(
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 10, ATTR_TRANSITION: 2},
|
||||
State("light.test", STATE_ON, {ATTR_BRIGHTNESS: 11}),
|
||||
State("light.test", STATE_ON, {ATTR_BRIGHTNESS: 13}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 10, ATTR_TRANSITION: 2},
|
||||
),
|
||||
(
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 230, ATTR_TRANSITION: 2},
|
||||
State("light.test", STATE_ON, {ATTR_BRIGHTNESS: 229}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_TRANSITION: 2},
|
||||
),
|
||||
(
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 230, ATTR_TRANSITION: 2},
|
||||
State("light.test", STATE_ON, {ATTR_BRIGHTNESS: 227}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 230, ATTR_TRANSITION: 2},
|
||||
),
|
||||
(
|
||||
{
|
||||
ATTR_ENTITY_ID: "light.test",
|
||||
ATTR_COLOR_TEMP_KELVIN: 5500,
|
||||
ATTR_TRANSITION: 2,
|
||||
},
|
||||
State("light.test", STATE_ON, {ATTR_COLOR_TEMP_KELVIN: 5495}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_TRANSITION: 2},
|
||||
),
|
||||
(
|
||||
{
|
||||
ATTR_ENTITY_ID: "light.test",
|
||||
ATTR_COLOR_TEMP_KELVIN: 5500,
|
||||
ATTR_TRANSITION: 2,
|
||||
},
|
||||
State("light.test", STATE_ON, {ATTR_COLOR_TEMP_KELVIN: 5524}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_TRANSITION: 2},
|
||||
),
|
||||
(
|
||||
{
|
||||
ATTR_ENTITY_ID: "light.test",
|
||||
ATTR_COLOR_TEMP_KELVIN: 6500,
|
||||
ATTR_TRANSITION: 2,
|
||||
},
|
||||
State("light.test", STATE_ON, {ATTR_COLOR_TEMP_KELVIN: 6494}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_TRANSITION: 2},
|
||||
),
|
||||
(
|
||||
{
|
||||
ATTR_ENTITY_ID: "light.test",
|
||||
ATTR_COLOR_TEMP_KELVIN: 5500,
|
||||
ATTR_TRANSITION: 2,
|
||||
},
|
||||
State("light.test", STATE_ON, {ATTR_COLOR_TEMP_KELVIN: 5400}),
|
||||
{
|
||||
ATTR_ENTITY_ID: "light.test",
|
||||
ATTR_COLOR_TEMP_KELVIN: 5500,
|
||||
ATTR_TRANSITION: 2,
|
||||
},
|
||||
),
|
||||
(
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_HS_COLOR: (30.0, 40.0)},
|
||||
State("light.test", STATE_ON, {ATTR_HS_COLOR: (30.0, 40.0)}),
|
||||
{ATTR_ENTITY_ID: "light.test"},
|
||||
),
|
||||
(
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 10},
|
||||
State("light.test", STATE_ON, {ATTR_BRIGHTNESS: None}),
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 10},
|
||||
),
|
||||
],
|
||||
ids=[
|
||||
"pass all attributes on empty state",
|
||||
"remove attributes whose values equal the state",
|
||||
"keep attributes whose values differ from the state",
|
||||
"remove brightness within quantization tolerance (0-99 device scale)",
|
||||
"keep brightness outside quantization tolerance",
|
||||
"remove color temp within one mired (round-converting integration)",
|
||||
"remove color temp within one mired (floor-converting HA core helpers)",
|
||||
"remove color temp within one mired (6500 K)",
|
||||
"keep color temp more than one mired away",
|
||||
"remove non-numeric attributes on exact equality",
|
||||
"keep attribute when state value is None",
|
||||
],
|
||||
)
|
||||
async def test_remove_redundant_attributes(
|
||||
|
|
@ -167,18 +235,18 @@ async def test_has_relevant_service_data_attributes(
|
|||
[],
|
||||
),
|
||||
(
|
||||
[{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 11}],
|
||||
[{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 15}],
|
||||
True,
|
||||
[{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 11}],
|
||||
[{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 15}],
|
||||
),
|
||||
(
|
||||
[
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 11},
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 15},
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 22},
|
||||
],
|
||||
True,
|
||||
[
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 11},
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 15},
|
||||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 22},
|
||||
],
|
||||
),
|
||||
|
|
@ -192,6 +260,11 @@ async def test_has_relevant_service_data_attributes(
|
|||
{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 22},
|
||||
],
|
||||
),
|
||||
(
|
||||
[{ATTR_ENTITY_ID: "light.test", ATTR_BRIGHTNESS: 11}],
|
||||
True,
|
||||
[],
|
||||
),
|
||||
],
|
||||
ids=[
|
||||
"single item passed through without filtering",
|
||||
|
|
@ -201,6 +274,7 @@ async def test_has_relevant_service_data_attributes(
|
|||
"filter keeps item with relevant attribute that is different from state",
|
||||
"filter keeps two items with relevant attributes that are different from state",
|
||||
"filter removes item that equals state and keeps items that differs from state",
|
||||
"filter removes item with relevant attribute within tolerance of the state",
|
||||
],
|
||||
)
|
||||
async def test_create_service_call_data_iterator(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue