Skip to content

Commit 9060baf

Browse files
jrhubottjrhubott
authored
fix: compare floor against actual position under manual override (#534) (#536)
1 parent 36e2282 commit 9060baf

4 files changed

Lines changed: 203 additions & 5 deletions

File tree

‎custom_components/adaptive_cover_pro/coordinator.py‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1508,9 +1508,18 @@ async def async_handle_state_change(
15081508
# gate would otherwise drop the return-to-calculated command. Short-
15091509
# circuit when the set is empty so callers that bypass __init__ (e.g.
15101510
# gate-matrix fixtures) don't need to wire _last_state_change_entity.
1511+
# When the manual-override hold is the winner, a floor sensor releasing
1512+
# must NOT take the release force-path — otherwise the override's
1513+
# theoretical default (e.g. 90%) would be force-driven onto the cover.
1514+
# The cover stays where the floor left it; the override keeps holding
1515+
# the (now recomputed) physical position (#534).
1516+
override_holding = (
1517+
self._pipeline_result is not None
1518+
and self._pipeline_result.control_method is ControlMethod.MANUAL
1519+
)
15111520
trigger_entity: str | None = None
15121521
custom_position_released = False
1513-
if custom_position_released_entities:
1522+
if custom_position_released_entities and not override_holding:
15141523
trigger_entity = self._last_state_change_entity
15151524
custom_position_released = (
15161525
trigger_entity is not None
@@ -1534,11 +1543,20 @@ async def async_handle_state_change(
15341543
self.logger.debug("Outside time window — skipping position update")
15351544
return
15361545

1546+
# A floor-clamp raised the winner this cycle (#534). When manual
1547+
# override holds the cover below an active floor, the clamped value is
1548+
# already in cover-position space and must bypass the time/position
1549+
# delta gates so the raise reaches the cover.
1550+
floor_clamp = bool(
1551+
self._pipeline_result is not None
1552+
and self._pipeline_result.floor_clamp_applied
1553+
)
15371554
use_force = (
15381555
is_safety
15391556
or force_override_released
15401557
or custom_position_sensor_triggered
15411558
or custom_position_released
1559+
or floor_clamp
15421560
)
15431561
if force_override_released:
15441562
reason = "force_override_cleared"
@@ -1555,6 +1573,13 @@ async def async_handle_state_change(
15551573
trigger_entity,
15561574
state,
15571575
)
1576+
elif floor_clamp:
1577+
reason = "floor_clamp"
1578+
self.logger.debug(
1579+
"Floor clamp active — bypassing time/position delta gates to "
1580+
"raise cover to floor position %s",
1581+
state,
1582+
)
15581583
else:
15591584
reason = (
15601585
self._pipeline_result.control_method.value

‎custom_components/adaptive_cover_pro/pipeline/registry.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,26 @@ def evaluate(self, snapshot: PipelineSnapshot) -> PipelineResult:
138138
# arithmetic lives in exactly one place (pipeline/floors.py).
139139
active_floors = gather_active_floors(snapshot)
140140
floor_pos, floor_info = effective_floor(active_floors)
141+
# The position the floor must raise: where the cover will actually end
142+
# up. manual_override holds the cover at held_position (its physical
143+
# position), not winner.position (the theoretical default it shadows),
144+
# so the floor must clamp against held_position when present (#534).
145+
# Every other handler leaves held_position=None, so this preserves the
146+
# existing behaviour exactly.
147+
effective_winner_pos = (
148+
winner.held_position
149+
if winner.held_position is not None
150+
else winner.position
151+
)
141152
clamped_position = winner.position
142-
if floor_info is not None and floor_pos > winner.position:
153+
if floor_info is not None and floor_pos > effective_winner_pos:
143154
clamped_position = floor_pos
144155
trace.append(
145156
DecisionStep(
146157
handler="floor_clamp",
147158
matched=True,
148159
reason=(
149-
f"floor raised winner from {winner.position}% to "
160+
f"floor raised winner from {effective_winner_pos}% to "
150161
f"{floor_pos}% by {floor_info.label}"
151162
),
152163
position=floor_pos,
@@ -162,7 +173,7 @@ def evaluate(self, snapshot: PipelineSnapshot) -> PipelineResult:
162173
if (
163174
floor_info is not None
164175
and info is floor_info
165-
and floor_pos > winner.position
176+
and floor_pos > effective_winner_pos
166177
):
167178
continue # this floor *did* win — already emitted as floor_clamp
168179
trace.append(
@@ -171,7 +182,7 @@ def evaluate(self, snapshot: PipelineSnapshot) -> PipelineResult:
171182
matched=False,
172183
reason=(
173184
f"floor {info.position}% inactive "
174-
f"(winner {winner.position}% above floor)"
185+
f"(winner {effective_winner_pos}% above floor)"
175186
),
176187
position=info.position,
177188
)

‎tests/test_coordinator_integration.py‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@ def _make_pipeline_result(
3030
position: int = 50,
3131
control_method: ControlMethod = ControlMethod.SOLAR,
3232
bypass_auto_control: bool = False,
33+
floor_clamp_applied: bool = False,
3334
) -> PipelineResult:
3435
return PipelineResult(
3536
position=position,
3637
control_method=control_method,
3738
reason="test",
3839
bypass_auto_control=bypass_auto_control,
40+
floor_clamp_applied=floor_clamp_applied,
3941
)
4042

4143

@@ -396,6 +398,80 @@ async def test_trigger_outside_window_pre_start_no_command(self):
396398
coordinator._cmd_svc.apply_position.assert_not_called()
397399

398400

401+
class TestFloorClampUnderManualOverride:
402+
"""A floor-clamp under an armed manual override must dispatch and not snap to default (#534)."""
403+
404+
@pytest.mark.asyncio
405+
async def test_floor_clamp_forces_dispatch_under_manual_override(self):
406+
"""A floor-clamped position under manual override calls context with force=True."""
407+
from custom_components.adaptive_cover_pro.coordinator import (
408+
AdaptiveDataUpdateCoordinator,
409+
)
410+
411+
result = _make_pipeline_result(
412+
position=80,
413+
control_method=ControlMethod.MANUAL,
414+
floor_clamp_applied=True,
415+
)
416+
coordinator = _make_coordinator(
417+
entities=["cover.blind"],
418+
pipeline_result=result,
419+
)
420+
coordinator.manager.is_cover_manual.return_value = True
421+
422+
await AdaptiveDataUpdateCoordinator.async_handle_state_change(
423+
coordinator, state=80, options={}
424+
)
425+
426+
coordinator._build_position_context.assert_called_once_with(
427+
"cover.blind",
428+
{},
429+
force=True,
430+
is_safety=False,
431+
sun_just_appeared=coordinator._check_sun_validity_transition.return_value,
432+
)
433+
434+
@pytest.mark.asyncio
435+
async def test_floor_release_under_armed_override_does_not_force_to_default(self):
436+
"""Floor sensor off while override armed: stay at floor, no force to default (#534).
437+
438+
When the floor sensor releases and manual override is still the winner,
439+
the manual-hold winner re-emits its theoretical default (90). The
440+
coordinator must NOT take the custom_position_released force path —
441+
otherwise it would drive the cover to that default. force stays False.
442+
"""
443+
from custom_components.adaptive_cover_pro.coordinator import (
444+
AdaptiveDataUpdateCoordinator,
445+
)
446+
447+
result = _make_pipeline_result(
448+
position=90,
449+
control_method=ControlMethod.MANUAL,
450+
floor_clamp_applied=False,
451+
)
452+
coordinator = _make_coordinator(
453+
entities=["cover.blind"],
454+
pipeline_result=result,
455+
)
456+
coordinator.manager.is_cover_manual.return_value = True
457+
coordinator._last_state_change_entity = "binary_sensor.cp1"
458+
459+
await AdaptiveDataUpdateCoordinator.async_handle_state_change(
460+
coordinator,
461+
state=90,
462+
options={},
463+
custom_position_released_entities={"binary_sensor.cp1"},
464+
)
465+
466+
coordinator._build_position_context.assert_called_once_with(
467+
"cover.blind",
468+
{},
469+
force=False,
470+
is_safety=False,
471+
sun_just_appeared=coordinator._check_sun_validity_transition.return_value,
472+
)
473+
474+
399475
class TestCustomPositionSensorReleaseEdgeBypassesGate:
400476
"""Custom-position sensor release-edge mirrors force-override release (#365).
401477

‎tests/test_pipeline/test_floor_composition.py‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
from custom_components.adaptive_cover_pro.pipeline.handlers.custom_position import (
2323
CustomPositionHandler,
2424
)
25+
from custom_components.adaptive_cover_pro.pipeline.handlers.manual_override import (
26+
ManualOverrideHandler,
27+
)
2528
from custom_components.adaptive_cover_pro.pipeline.handlers.weather import (
2629
WeatherOverrideHandler,
2730
)
@@ -586,6 +589,89 @@ def test_inactive_floor_below_winner_keeps_flag_false() -> None:
586589
assert result.floor_clamp_applied is False
587590

588591

592+
def test_floor_raises_manual_override_held_below_floor() -> None:
593+
"""A floor raises the cover when manual override holds it below the floor (#534).
594+
595+
Manual override wins with ``position`` = theoretical default (90) but
596+
``held_position`` = the cover's actual physical position (50). A min-mode
597+
floor at 80% must be measured against the *physical* 50%, not the
598+
theoretical 90% — so the floor fires and raises the cover to 80%.
599+
"""
600+
cover = _climate_cover(direct_sun_valid=False)
601+
snap = make_snapshot(
602+
cover=cover,
603+
manual_override_active=True,
604+
current_cover_position=50,
605+
default_position=90,
606+
direct_sun_valid=False,
607+
custom_position_sensors=[
608+
_cp_state(
609+
"binary_sensor.cp1",
610+
is_on=True,
611+
position=80,
612+
min_mode=True,
613+
sensor_name="Table",
614+
)
615+
],
616+
)
617+
handlers = [
618+
_cp_handler(1, "binary_sensor.cp1", 80),
619+
ManualOverrideHandler(),
620+
]
621+
registry = _registry_with_custom(handlers)
622+
result = registry.evaluate(snap)
623+
winner_step = next(
624+
s for s in result.decision_trace if s.matched and s.handler != "floor_clamp"
625+
)
626+
assert winner_step.handler == "manual_override"
627+
assert result.position == 80
628+
assert result.floor_clamp_applied is True
629+
clamp_steps = [
630+
s for s in result.decision_trace if s.handler == "floor_clamp" and s.matched
631+
]
632+
assert len(clamp_steps) == 1
633+
634+
635+
def test_floor_above_held_position_is_inert_under_manual_override() -> None:
636+
"""Floor below the cover's physical position does NOT clamp (#534 inert branch).
637+
638+
Manual override holds the cover at 85% (physical), floor is 80% — the cover
639+
already sits above the floor, so no clamp is applied.
640+
"""
641+
cover = _climate_cover(direct_sun_valid=False)
642+
snap = make_snapshot(
643+
cover=cover,
644+
manual_override_active=True,
645+
current_cover_position=85,
646+
default_position=90,
647+
direct_sun_valid=False,
648+
custom_position_sensors=[
649+
_cp_state(
650+
"binary_sensor.cp1",
651+
is_on=True,
652+
position=80,
653+
min_mode=True,
654+
sensor_name="Table",
655+
)
656+
],
657+
)
658+
handlers = [
659+
_cp_handler(1, "binary_sensor.cp1", 80),
660+
ManualOverrideHandler(),
661+
]
662+
registry = _registry_with_custom(handlers)
663+
result = registry.evaluate(snap)
664+
winner_step = next(
665+
s for s in result.decision_trace if s.matched and s.handler != "floor_clamp"
666+
)
667+
assert winner_step.handler == "manual_override"
668+
# No clamp: the held physical position (85) is already above the floor (80).
669+
assert result.floor_clamp_applied is False
670+
assert not any(
671+
s.handler == "floor_clamp" and s.matched for s in result.decision_trace
672+
)
673+
674+
589675
def test_decision_trace_does_not_mislabel_winner() -> None:
590676
"""Only the underlying handler is marked as the non-clamp winner (no double-winner)."""
591677
cover = _climate_cover(direct_sun_valid=False)

0 commit comments

Comments
 (0)