Skip to content

feat(cover,light): pluggable CoverKinematicsStrategy hierarchy and low-priority fade step routing (#730) - #739

Merged
GreenGrassBlueOcean merged 4 commits into
v2-phase1-architecturefrom
arch/issue-730-native-fades-and-cover-strategies
Oct 20, 2026
Merged

GreenGrassBlueOcean merged 4 commits into
v2-phase1-architecturefrom
arch/issue-730-native-fades-and-cover-strategies

Conversation

@GreenGrassBlueOcean

@GreenGrassBlueOcean GreenGrassBlueOcean commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Closes #730.

This PR extracts MyHOMECover kinematics into a pluggable CoverKinematicsStrategy hierarchy (custom_components/myhome/cover_strategies/) and routes intermediate software fade steps in MyHOMELight through gateway.send_fade_step(cmd) (FramePriority.LOW_FADE_STEP).


What is the problem

A. Entangled Cover Kinematics in MyHOMECover (custom_components/myhome/cover.py)

  1. Four distinct physical cover models were interleaved in a single 2,000+ line entity class:
  2. Entity-coupled timing math blocked non-linear calibrated strategies (feat: add dedicated MyHOME administration panel #374):
    • As highlighted in #739 (comment), if a strategy is only a thin facade calling back into entity._travel_for, a future CalibratedGeometryStrategy (modeling non-linear slat phase + roll diameter + lift-off gap) cannot replace the time $\leftrightarrow$ position mapping without modifying cover.py again.

B. Software Fade Step Starvation vs. Gateway Firmware NACKs (custom_components/myhome/light.py)

  1. Queue contention during software fades: During an active SoftwareFadeEngine transition, MyHOMELight._set_brightness_instant dispatches up to 25 intermediate *#1*WHERE*#1*LEVEL100*0## frames at 0.3 s intervals. Sending those intermediate steps through gateway.send(cmd) (FramePriority.HIGH_INTERACTIVE) places them at the same priority as interactive wall-switch commands and time-critical cover STOP commands (*2*0*WHERE##).
  2. Why DEFAULT_TRANSITION_MODE cannot blindly switch to "native": The firmware oracle shows the F450 dropping Dimension 1 transition-speed writes (*#1*WHERE*#1*LEVEL100*SPEED##), and legacy dimmers ignore the hardware speed byte. The oracle also has F460, F461 and MyHomeServer1 NACK them, but that NACK is an emulator artefact (see the note under the oracle table).

How is it solved

A. Pluggable CoverKinematicsStrategy Protocol & Hierarchy (custom_components/myhome/cover_strategies/)

  • Strategy-owned parameters & time $\leftrightarrow$ position contract (base.py):
    • CoverKinematicsStrategy (@runtime_checkable Protocol) and BaseCoverStrategy directly own travel_time_down, travel_time_up, min_motor_pulse, and slat_time (never reading back from entity._travel_for, entity._slat_time, or entity._min_motor_pulse).
    • MyHOMECover exposes _travel_time_down, _travel_time_up, _min_motor_pulse, and _slat_time as @property getters/setters backed by self._strategy, guaranteeing that calibration updates and future CalibratedGeometryStrategy subclasses (feat: add dedicated MyHOME administration panel #374) are never bypassed.
    • Full kinematics interface owned by the strategy:
      • position_after(start, elapsed, direction, *, entity=None, lead=None) -> int | None
      • duration_for(start, target, direction=None, *, entity=None) -> float
      • reanchor_on_stop_echo(entity, ctx, now) -> int | None
      • current_position(entity, now) -> int | None
      • curtain_travel_for(opening) -> float & lead_time_for(opening, *, entity=None) -> float
      • turn_slats_after(start_tilt, elapsed, direction) -> int, carry_overshoot_into_curtain(...) -> int, tilt_pulse_duration_for(...) -> float
      • quantize_target(current, target) -> int
  • Four concrete strategies resolved via resolve_cover_strategy() (__init__.py):
Strategy Activation (validate.py / resolve_cover_strategy) supported_features assumed_state Kinematics & Bus Behavior
SimpleUpDownStopStrategy (simple.py) strategy: simple or travel_time: 0 / None OPEN | CLOSE | STOP True current_cover_position = None (no fake 50%). Auto-promotes to TimedCurtainStrategy / VenetianTiltStrategy via _promote_to_timed_strategy() when calibrated.
TimedCurtainStrategy (timed.py) strategy: timed or travel_time > 0 (default 25 s) OPEN | CLOSE | STOP | SET_POSITION False Linear dead-reckoning with travel_time_up / travel_time_down, min_motor_pulse floor (#704/#708), and stop-echo re-anchoring (#705/#707).
VenetianTiltStrategy (venetian.py) strategy: venetian or slat_tilt: true Curtain + OPEN_TILT | CLOSE_TILT | STOP_TILT | SET_TILT_POSITION False Subclasses TimedCurtainStrategy. Shares single slat_time (#508): curtain_travel = travel_time - slat_time, slat lead-time deduction, overshoot carry into curtain, and slat stop-echo re-anchoring.
NativePositionStrategy (native.py) strategy: native or advanced: true OPEN | CLOSE | STOP | SET_POSITION False Hardware Dimension 10/11 positioning with quantize_target (MIN_POSITION_DELTA = 2, #466/#702) and duration_for = 0.0.

B. Low-Priority Software Fade Step Routing (custom_components/myhome/light.py)

  • Keeps DEFAULT_TRANSITION_MODE = TRANSITION_MODE_SOFTWARE ("software_stepped").
  • In MyHOMELight._set_brightness_instant, when self._fade_engine.is_fading is True and gateway.send_fade_step is an async coroutine function, routes intermediate fade frames through await gateway.send_fade_step(cmd) (FramePriority.LOW_FADE_STEP; a queued step is dropped only when a newer fade step or command for the same light supersedes it) while one-shot brightness commands continue using await gateway.send(cmd) (FramePriority.HIGH_INTERACTIVE).

C. Review fixes (review)

  • Contradictory config is rejected: an explicit strategy next to advanced (other than native), strategy: timed with slat_tilt or with travel_time: 0. The entity's _advanced / _slat_tilt now follow the resolved strategy, so the advanced code paths never run under a timed or simple strategy; the matching guard in TimedCurtainStrategy.current_position is gone.
  • Simple covers report closed after a closing run and open after an opening one (assumed state, as HA's optimistic covers), instead of staying open.
  • reset_cover_travel_time demotes a promoted travel_time: 0 cover back to SimpleUpDownStopStrategy; an explicit strategy: simple is never promoted, matching what a restart resolves.
  • Travel attributes come from the strategy, so a simple cover no longer shows travel_time: 25.
  • strategy and travel_time: 0 are documented in docs/configuration/covers.md.

How is it tested

  • 4,323 passed in the WSL test suite (pytest tests -q; the 784 cover / validation tests were rerun after the last edit), 100% statement coverage of cover_strategies/ and validate.py.
  • mypy --strict (0 errors via scripts/typing_ratchet.py), ruff check, scripts/verify_ha_standards.py, and scripts/sync_documentation.py --check all green.

State Diagram

Cover Strategy Resolution & Runtime Promotion Lifecycle

stateDiagram-v2
    [*] --> ResolveStrategy : MyHOMECover.__init__()

    state ResolveStrategy <<choice>>
    ResolveStrategy --> NativePositionStrategy : strategy=="native" or advanced==True
    ResolveStrategy --> VenetianTiltStrategy : strategy=="venetian" or slat_tilt==True
    ResolveStrategy --> SimpleUpDownStopStrategy : strategy=="simple" or travel_time <= 0
    ResolveStrategy --> TimedCurtainStrategy : strategy=="timed" or travel_time > 0

    state SimpleUpDownStopStrategy {
        [*] --> UncalibratedIdle : position = None, assumed_state = True
        UncalibratedIdle --> MovingUp : *2*1*WHERE## / async_open_cover()
        UncalibratedIdle --> MovingDown : *2*2*WHERE## / async_close_cover()
        MovingUp --> UncalibratedIdle : *2*0*WHERE## / async_stop_cover()
        MovingDown --> UncalibratedIdle : *2*0*WHERE## / async_stop_cover()
    }

    SimpleUpDownStopStrategy --> TimedCurtainStrategy : _promote_to_timed_strategy() (calibration saved & _slat_tilt==False)
    SimpleUpDownStopStrategy --> VenetianTiltStrategy : _promote_to_timed_strategy() (calibration saved & _slat_tilt==True)
    TimedCurtainStrategy --> VenetianTiltStrategy : _enable_slat_tilt()
Loading

Timed & Venetian Kinematics State Machine (TimedCurtainStrategy / VenetianTiltStrategy)

stateDiagram-v2
    [*] --> Idle : _start_position = pos (0..100)

    Idle --> SlatLeadPhase : Move command / bus *2*1|2*WHERE## (slat_tilt=True, lead_time_for() > 0)
    Idle --> CurtainMoving : Move command / bus *2*1|2*WHERE## (lead_time_for() == 0)
    Idle --> TiltPulseRunning : async_set_cover_tilt_position() (tilt_pulse_duration_for() >= min_motor_pulse)

    state SlatLeadPhase {
        [*] --> RotatingSlats : turn_slats_after(start_tilt, elapsed, dir)
        RotatingSlats --> LeadComplete : elapsed >= lead_time_for()
    }

    SlatLeadPhase --> CurtainMoving : Slats fully open/closed (curtain_elapsed = elapsed - lead)
    SlatLeadPhase --> StopEchoWindow : Early *2*0*WHERE## before lead completes (curtain_position unchanged)

    state CurtainMoving {
        [*] --> DeadReckoning : current_position() -> position_after(start, elapsed, dir, lead)
        DeadReckoning --> AutoStopArmed : Intermediate target (0 < target < 100, duration_for())
    }

    CurtainMoving --> StopEchoWindow : Auto-stop / async_stop_cover() sends *2*0*WHERE## (captures _pending_stop_context)
    CurtainMoving --> Idle : External *2*0*WHERE## -> _freeze_position() + _turn_slats_for_run()

    TiltPulseRunning --> Idle : Pulse auto-stop *2*0*WHERE## or carry_overshoot_into_curtain()

    state StopEchoWindow {
        [*] --> AwaitingBusStopEcho : ECHO_WINDOW (2.0 s)
        AwaitingBusStopEcho --> Reanchored : Relayed *2*0*WHERE## -> strategy.reanchor_on_stop_echo(ctx, now)
    }

    StopEchoWindow --> Idle : Position & slat tilt re-anchored to actual bus stop timestamp
Loading

Where are the traces

All three trace fixtures are checked into tests/fixtures/traces/ and replayed end-to-end through MyHOMECover in tests/test_cover_strategies.py:

# Strategy Tested Trace Fixture Path Provenance / Hardware Test Function (tests/test_cover_strategies.py) Replayed Bus Sequence & Verified Outcome
1 TimedCurtainStrategy tests/fixtures/traces/f461_live_2026-10-18/myhome_trace_F461_live_2026-10-18.json Live capture on physical BTicino F461 (fw 2.0.11, kernel 5.10.35, WHO 1013 code 134) driving physical BT-F411/2 shutter actuator at WHERE="05" (5.0 s calibrated travel time, run="run2_shutter_05_travel_and_reversal", 18 RX frames). test_replay_f461_live_shutter_05_trace_on_timed_strategy (L510–L565) Starts at 0% (closed).
1. Open pulse *2*1*05## (t=22.720s) $\rightarrow$ *2*0*05## (t=24.852s, $\Delta t = 2.132\text{ s} / 5.0\text{ s}$) $\rightarrow$ 43%.
2. Open pulse *2*1*05## (t=26.421s) $\rightarrow$ *2*0*05## (t=28.014s, $\Delta t = 1.593\text{ s} / 5.0\text{ s}$) $\rightarrow$ 75%.
3. Close reversal *2*2*05## (t=28.119s) $\rightarrow$ hardware auto-stop *2*0*05## (t=34.341s, $\Delta t = 6.222\text{ s}$) $\rightarrow$ 0% (is_closed = True).
2 NativePositionStrategy tests/fixtures/traces/issue_466/config_entry-myhome_F454_step_commands.json Live Home Assistant diagnostics bus_monitor.recent_frames from a production BTicino F454 (fw 2.0, 42 covers) at WHERE="13" (#466). test_replay_issue_466_f454_trace_on_native_strategy (L568–L613) Replays *#2*13*10*10*100*001*0## (t=1791365333.130619, Dimension 10 status: shutterStatus=10 Stop, shutterLevel=100%, priority=001, info=0) followed by *2*0*13## (t=1791365333.177027) $\rightarrow$ authoritative current_cover_position == 100, is_closed = False.
3 VenetianTiltStrategy tests/fixtures/traces/oracle_who2_venetian_tilt/myhome_trace_oracle_who2_venetian_tilt.json Synthetic fixture (its README says so: derived from the client's own slat model, so this replay checks consistency rather than hardware behaviour), built from (F454 fw 2.0.51, coso automation_tr / automation_stm + BTicino Object 218 fw 192 OWN-DEV-0045 slat timing SLATS_ROTATION_TIME_DOWN) at WHERE="31" (travel_time=22s, slat_time=2.0s). test_replay_oracle_who2_venetian_tilt_trace_on_venetian_strategy (L616–L678) Starts at position=0%, tilt=0%.
1. 1.0 s open pulse *2*1*31## (t=1792080001.0) $\rightarrow$ *2*0*31## (t=1792080002.0): rotates slats 0% -> 50% while curtain stays at 0% (absorbed by lead_time_for).
2. 1.0 s close pulse *2*2*31## (t=1792080010.0) $\rightarrow$ *2*0*31## (t=1792080011.0): rotates slats 50% -> 0% while curtain remains 0%.

(Additional reference capture cited by the Encyclopedia for WHO=1 Dimension 1 level/speed behavior: tests/fixtures/traces/issue_466/myhome_trace_MH200_f418u2_dimmer_2026-09-27T12-15-00.json, plus MyHomeServer1/LN4660M2 WHO=2 traces in tests/fixtures/traces/issue_445/).


What does the Oracle / Encyclopedia say

A. Empirical Firmware Oracle (openwebnet-mcp / own-firmware-oracle, 12 QEMU Emulated Gateways)

OpenWebNet Frame Purpose Gateways Returning ACK (*#*1##) + OUT (Bus) Gateways Returning NACK (*#*0##) + OUT (Bus) Gateways Returning NACK (*#*0##) + SILENT (Drop) Architectural Consequence for PR #739
*#1*31*#1*150*5## WHO=1 Dim 1 write (LEVEL100=150 / 50%, SPEED=5) F453, F453AV, F454, F459, H4684, MH200, MH200N, MH202 F460 (020012), F461 (020011), MyHomeServer1 (028206) F450 (020010) Native Dim 1 speed transitions are not universally accepted (F450 drops them; F460, F461, MyHomeServer1 NACK the command session). Keeping DEFAULT_TRANSITION_MODE = "software_stepped" and demoting active fade steps to FramePriority.LOW_FADE_STEP preserves transitions everywhere without starving interactive commands.
*2*1*31## / *2*0*31## WHO=2 standard UP (WHAT=1) / STOP (WHAT=0) F453, F453AV, F454, F459, H4684, MH200, MH200N, MH202 F460, F461, MyHomeServer1 (emit 24 30 ... 0d on SCS bus + *2*1000#1*31## / *2*1000#0*31##) F450 (020010) Standard UP/DOWN/STOP frames are the universal baseline for SimpleUpDownStopStrategy, TimedCurtainStrategy, and VenetianTiltStrategy. Gateways also emit *2*1000#0*WHERE## translation frames alongside *2*0*WHERE## stop echoes, handled by stop-echo re-anchoring.
*#2*31*10## WHO=2 Dim 10 status request F454 (020051), F459 (020105), MH202 (010024) F460, F461, MyHomeServer1 F450, F453, F453AV, H4684, MH200, MH200N Half of the gateway fleet (F450, F453, F453AV, H4684, MH200, MH200N) NACKs and drops Dimension 10 queries. NativePositionStrategy must only be active when advanced: true / strategy: native or when a Dimension 10 frame is actually observed on the bus.
*#2*31*#11#001*40## WHO=2 Dim 11 Go-To-Level 40% (priority=001) F454 (020051), F459 (020105), MH202 (010024) F460, F461, MyHomeServer1 (emit *#2*31*#11#001#1*40##) F450, F453, F453AV, H4684, MH200, MH200N Confirms NativePositionStrategy target writes (*#2*WHERE*#11#001*LEVEL##) are hardware-timed on F454/F459/MH202/F46x/MyHomeServer1 (duration_for() == 0.0) and unsupported on legacy MH200/MH200N/F453/H4684.

Note

The F46x NACK column is not evidence of missing support. The emulated F460, F461 and MyHomeServer1 NACK every frame above, including a plain *2*1*31## that the live F461 trace (#1) shows working. A physical F461 NACKs when no actuator answers on the bus, and the emulator has none; the same F461 ACKed the Dimension 1 write *#1*08*#1*150*0## in 177 ms (f461_live_2026-10-18/README.md). Keeping software_stepped as the default rests on the F450 and on legacy dimmers.

B. OpenWebNet Encyclopedia (OpenWebNet-Encyclopedia)

  1. WHO=1 DIMENSION 1 (functional/who-1-lighting/dimensions.md):
    • Encoding is *#1*WHERE*#1*LEVEL100*SPEED## where LEVEL100 is offset by 100 (100 = OFF, 101..199 = 1%..99%, 200 = 100%) and SPEED is 0 (last speed), 1..254 (explicit speed), or 255 (default speed).
    • Runtime captures on MH200 fw 2.1.0 + F418U2 demonstrate that the written SPEED parameter is not echoed unchanged in the following DIMENSION 1 report (a write with SPEED=0 immediately reports trailing value 5, and a later read returns 2), and DIMENSION 1 / DIMENSION 4 support varies by gateway/actuator path.
  2. WHO=2 WHAT & DIMENSION 10/11 (functional/who-2-automation/what.md, dimensions.md, README.md):
    • Base movement uses *2*WHAT*WHERE## (0 = Stop, 1 = Up, 2 = Down) with command-translation wrapper *2*1000#INNER_WHAT*WHERE##.
    • DIMENSION 10 (*#2*WHERE*10*SHUTTER_STATUS*SHUTTER_LEVEL*SHUTTER_PRIORITY*SHUTTER_INFO##) reports shutterStatus (10 = Stop, 11 = Up, 12 = Down, 13 = Step-Up, 14 = Step-Down) and shutterLevel (0..100%, 255 = unknown).
    • DIMENSION 11 (*#2*WHERE*#11#SHUTTER_PRIORITY*SHUTTER_LEVEL##, e.g. *#2*WHERE*#11#001*LEVEL##) writes an absolute target position (0..100%; 255 is never a valid target).
    • Historical BTicino touchscreen decoders (TS10_1_0_23) treat standard 3-state WHO=2 objects strictly as UP / DOWN / STOP without fabricating percentage positions when Dimension 10 is absent — matching SimpleUpDownStopStrategy.
  3. Device Model OWN-DEV-0045 (devices/definitions/own-dev-0045-shutter-actuator-bus.md — Catalogue Item 1586, Object 218 fw 192: H4661M2, LN4661M2, AM5861M2, 067557):
    • Documents separate direction calibration registers (UP_SHUTTER_TIME_MINUTES/SECONDS, DOWN_SHUTTER_TIME_MINUTES/SECONDS), minimum pulse duration registers (STOP_PULSE_DURATION, UP_OR_DOWN_PULSE_DURATION in 0.1 s increments), and Venetian slat rotation parameters (SHUTTER_TYPE = 3 "Standard with slats", SLATS_ROTATION_TIME_DOWN_H/L, SLATS_ROTATION_TIME_MIDDLE_H/L, SLATS_ROTATION_STEP_NUMBER), directly backing the parameter model in BaseCoverStrategy, TimedCurtainStrategy, and VenetianTiltStrategy.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@GreenGrassBlueOcean
GreenGrassBlueOcean force-pushed the arch/issue-730-native-fades-and-cover-strategies branch 2 times, most recently from c630e76 to 37df2a8 Compare October 17, 2026 10:40
@GreenGrassBlueOcean
GreenGrassBlueOcean force-pushed the arch/issue-731-frame-scheduler branch from 4da855e to c8b0378 Compare October 18, 2026 08:15
Base automatically changed from arch/issue-731-frame-scheduler to v2-phase1-architecture October 18, 2026 09:56
@Interstellar0verdrive

Copy link
Copy Markdown
Member

Not a review, just a note from the calibration side, since this is where the cover model is being given its shape.

Please leave room for a calibrated, non-linear strategy. The guided calibration in xtimmy86x#10 (coming to this repo through #374) produces per-cover geometry: slat phase, roll coefficients, lift-off gap and a reference travel, measured with a tape. Time and height are not proportional on these shutters: from a full close, a 40 % request on my 145 cm shutter lifts the curtain about 18 % (the first seconds open the slats), and @fedem95's 40 cm / 55 cm asymmetry on the same 50 % is the same effect seen from the other side. A linear TimedCurtainStrategy cannot express that, and it shouldn't have to. What would make the split future-proof is for the strategy to own the time↔position mapping, something like position_after(start, elapsed, direction) and duration_for(start, target, direction), rather than calling back into the entity's _travel_for. Today timed.py is a thin façade over the entity, so the kinematics are not really extracted yet; with those two methods a CalibratedGeometryStrategy could sit next to the timed one without touching cover.py again.

Two things to align before review:

@GreenGrassBlueOcean
GreenGrassBlueOcean force-pushed the arch/issue-730-native-fades-and-cover-strategies branch 2 times, most recently from 5f30c58 to bda0c54 Compare October 18, 2026 20:10
GreenGrassBlueOcean added a commit that referenced this pull request Oct 19, 2026
@GreenGrassBlueOcean
GreenGrassBlueOcean force-pushed the arch/issue-730-native-fades-and-cover-strategies branch from bda0c54 to 2be0103 Compare October 19, 2026 09:39
@GreenGrassBlueOcean GreenGrassBlueOcean changed the title feat(cover,light): default native transitions and extract cover kinematics strategies (#730) feat(cover,light): pluggable CoverKinematicsStrategy hierarchy and low-priority fade step routing (#730) Oct 19, 2026
GreenGrassBlueOcean added a commit that referenced this pull request Oct 19, 2026
@GreenGrassBlueOcean
GreenGrassBlueOcean force-pushed the arch/issue-730-native-fades-and-cover-strategies branch from 2be0103 to 587efb1 Compare October 19, 2026 09:41
@GreenGrassBlueOcean

Copy link
Copy Markdown
Contributor Author

Thanks @Interstellar0verdrive — we re-cut the branch on top of the current v2-phase1-architecture base (incorporating #508, #667, #702, #707, and #708) and restructured cover_strategies/ around all three points from your note:

1. Strategy owns position_after() & duration_for() (ready for CalibratedGeometryStrategy / #374)

  • What changed: CoverKinematicsStrategy (cover_strategies/base.py) and BaseCoverStrategy now own the time $\leftrightarrow$ position contract directly via:
    • position_after(start, elapsed, direction, *, entity=None, lead=None) -> int | None
    • duration_for(start, target, direction=None, *, entity=None) -> float
    • reanchor_on_stop_echo(entity, ctx, now) -> int | None
    • current_position(entity, now) -> int | None
  • No entity back-calls: Strategies directly own travel_time_down, travel_time_up, min_motor_pulse, and slat_time on self and never read back from entity._travel_for, entity._slat_time, or entity._min_motor_pulse. Instead, MyHOMECover exposes _travel_time_down, _travel_time_up, _min_motor_pulse, and _slat_time as @property getters/setters backed by self._strategy, and delegates _freeze_position(), current_cover_position, and _async_move_to_position() to self._strategy.position_after(...) and self._strategy.duration_for(...).
  • Why: When feat: add dedicated MyHOME administration panel #374 lands, CalibratedGeometryStrategy can subclass BaseCoverStrategy (or TimedCurtainStrategy) and override position_after() and duration_for() with the non-linear slat-phase + roll-diameter + lift-off gap curve without touching cover.py (tested in test_calibrated_geometry_strategy_plugs_in_without_cover_py_changes).

2. Re-cut on top of #702, #707, and #708 so the strategies own the real logic

3. Unified on slat_time and #508's shared slat-phase + roll geometry (tilt_time removed)

  • What changed: We removed tilt_time completely and standardized on slat_time (CONF_SLAT_TIME, clamped via clamp_slat_time to [MIN_SLAT_TIME, MAX_SLAT_TIME]).
  • Why: As you noted, total end-to-end travel_time already includes one full slat rotation (total travel = slat phase + curtain roll). VenetianTiltStrategy (cover_strategies/venetian.py) now subclasses TimedCurtainStrategy and shares the single slat_time parameter across both tilt and curtain estimation:
    • curtain_travel_for(opening) computes travel_time - slat_time, avoiding the double-counting from feat(cover): implement venetian blind slat tilt inching and clarify dimension 11 shutter level (#492) #508.
    • lead_time_for(opening, entity=self) deducts the remaining slat rotation time ((100 - curr_tilt) / 100 * slat_time opening, curr_tilt / 100 * slat_time closing) before vertical curtain travel begins.
    • turn_slats_after(), carry_overshoot_into_curtain(), tilt_pulse_duration_for(), and _reanchor_slats_on_stop_echo() all operate on the same slat_time.

4. Other scope & firmware-oracle alignments

  • Dropped ImpulseDoorStrategy / pulse_duration: Removed so this PR does not conflict with MyHOMEImpulseCover in feat(cover,lock): impulse garage/gate control, multi-factor safety approval, and intercom events #667.
  • Kept DEFAULT_TRANSITION_MODE = "software_stepped" with low-priority fade routing: Because the firmware oracle shows F450 drops Dimension 1 speed frames (*#1*WHERE*#1*LEVEL100*SPEED##) and F460, F461, and MyHomeServer1 NACK them on the command session, DEFAULT_TRANSITION_MODE stays "software_stepped", while MyHOMELight._set_brightness_instant() now routes active SoftwareFadeEngine steps through gateway.send_fade_step(cmd) (FramePriority.LOW_FADE_STEP, drop-oldest on queue saturation) so fades never starve interactive commands or cover STOP frames.

@GreenGrassBlueOcean
GreenGrassBlueOcean force-pushed the arch/issue-730-native-fades-and-cover-strategies branch from 587efb1 to 58ffd33 Compare October 19, 2026 12:33

@GreenGrassBlueOcean GreenGrassBlueOcean left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audit of 58ffd339. CI is green and the branch merges cleanly. The refactor mostly preserves existing behaviour: timed covers, the Venetian slat lead time, min_motor_pulse, stop-echo re-anchoring and the fade-step routing all match the old code. The new strategy: key and travel_time: 0 introduce three bugs, each reproduced with a throwaway test against this head.

Bugs

  1. Contradictory config is accepted. strategy: timed|simple|venetian together with advanced: true passes cover_schema. resolve_cover_strategy honours strategy, but cover.py still takes the advanced paths from self._advanced.
    • timed + advanced: Dimension 11 writes lose the #702 deadband (quantize_target(50, 51) returns 51, not 52).
    • simple + advanced: the Dimension 10 level is stored (40 in the probe) but current_cover_position returns None, and SET_POSITION is gone.
    • The _advanced guard in TimedCurtainStrategy.current_position only exists to paper over this.
  2. A simple cover never reports closed. After a bus *2*2*21## and *2*0*21##, is_closed stays False and the position stays None, so HA shows open permanently.
  3. reset_cover_travel_time does not undo the promotion. A travel_time: 0 cover that was promoted on calibration stays timed with 0.0 s travel after a reset: SET_POSITION is still advertised, duration_for returns 0.0 and position_after never moves.

Description and comment claims

  • The F46x NACKs in the oracle table are an emulator artefact. The oracle data is reproduced accurately, but the oracle also has F460/F461/MyHomeServer1 NACK a plain *2*1*31##, which the live F461 trace in this PR shows working. tests/fixtures/traces/f461_live_2026-10-18/README.md explains why: a physical F461 NACKs when no actuator answers, and it ACKed *#1*08*#1*150*0## in 177 ms. "F46x NACK Dimension 1 transitions" is therefore not evidence. Keeping software_stepped as the default is still right because of the F450 and legacy dimmers.
  • Trace #3 is synthetic. oracle_who2_venetian_tilt/README.md says it is fully synthetic, derived from the client model, so the replay is circular. It should not be presented as a firmware-oracle trace.
  • "Drop-oldest on queue saturation" does not exist. Fade steps are only dropped when superseded by a newer frame for the same target (drop_superseded). The final step of a fade also travels in the fade tier.
  • The cited test does not exist. test_calibrated_geometry_strategy_plugs_in_without_cover_py_changes is not in the PR. The test that exists overrides only travel_for_direction (still linear) and calls the strategy directly, not through the entity.
  • Some listed removals are not in the diff against the base. ImpulseDoorStrategy, tilt_time, TRANSITION_MODE_OFF and allow_multi_worker were removed from an earlier iteration of this branch, not from v2-phase1-architecture.

Nits

  • A simple cover still publishes travel_time: 25 / calibration_source: default while _travel_for() returns 0.
  • The strategies read and write entity private attributes through entity: Any; reanchor_on_stop_echo writes five of them plus _last_run. mypy cannot check those.
  • strategy and travel_time: 0 are not documented in docs/configuration/covers.md.

Fixes for the three bugs, the docs and the description follow on this branch.

where = cfg.get(CONF_WHERE)
if not where or not str(where).startswith("#"):
raise Invalid("Members can only be defined on a group cover (where must start with #)")
strategy = cfg.get(CONF_STRATEGY)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only simple + slat_tilt is rejected here. strategy: timed|simple|venetian with advanced: true, strategy: timed with slat_tilt (silently becomes venetian through _enable_slat_tilt), and strategy: timed with travel_time: 0 all pass.

Comment thread custom_components/myhome/cover.py Outdated
slat_time=raw_slat_time,
min_motor_pulse=raw_min_pulse,
)
if isinstance(self._strategy, NativePositionStrategy):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only ever turns _advanced on. With an explicit non-native strategy and advanced: true, the entity keeps the advanced code paths while the strategy is timed or simple. Deriving _advanced (and _slat_tilt) from the resolved strategy keeps the two consistent.

)
return result

def _promote_to_timed_strategy(self) -> None:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing reverses this. async_reset_travel_time (unchanged, further down) writes the YAML travel_time: 0 as 0.0 s into the promoted TimedCurtainStrategy and leaves SET_POSITION advertised. The cover should drop back to SimpleUpDownStopStrategy here.


def current_position(self, entity: Any, now: float) -> int | None:
"""Interpolate current position from start position, motor anchor, and strategy kinematics."""
if getattr(entity, "_advanced", False) is True:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A timed strategy on an advanced entity is the contradictory-config case. Once the entity flags follow the strategy, this guard is dead code.

GreenGrassBlueOcean and others added 2 commits October 20, 2026 12:12
…r closure, demote on reset (#739)

- validate: an explicit strategy cannot be combined with advanced (other
  than native), strategy: timed with slat_tilt or with travel_time: 0.
- cover: _advanced / _slat_tilt follow the resolved strategy, so the
  advanced code paths never run under a timed or simple strategy (the
  #702 deadband was lost and Dimension 10 levels were hidden); drop the
  matching guard in TimedCurtainStrategy.current_position.
- cover: an assumed-state (simple) cover is closed after a closing run
  and open after an opening one instead of staying open.
- cover: reset_cover_travel_time returns a promoted travel_time: 0 cover
  to SimpleUpDownStopStrategy; an explicit strategy: simple is never
  promoted, matching what a restart resolves.
- cover: travel attributes come from the strategy, so a simple cover
  shows no travel time.
- docs: document strategy and travel_time: 0 in covers.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_demote_to_simple_strategy's early return was never exercised, failing
the 100% coverage gate. Drop the unreachable NativePositionStrategy check
(reset returns before demotion on advanced covers) and test a reset on a
simple cover that was never promoted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@Interstellar0verdrive Interstellar0verdrive left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for re-cutting this. All three points from my note were taken up: the strategy now owns position_after / duration_for / reanchor_on_stop_echo, the #702/#707/#708 logic lives in the strategies, and slat_time is the single slat number. I checked behaviour preservation: my #508 probe set and the #702/#707/#708 cases (0.92 s / 56 %, the 0.4 s floor to 53 %, the settle at travel + 2 s, advanced 100 % as plain UP, floor 0 off, a floor-only call keeping the calibration) give identical numbers on the base and on 082f723. The fade-step tier can never sit ahead of a cover frame. As always, this is code review only.

What still stops a calibrated strategy from plugging in without touching cover.py:

  1. cover.py overrides the lead. It computes lead itself (0 unless slat_tilt) and passes it to position_after, while duration_for uses the strategy's own. With a 3 s slat phase on a cover without tilt control, set_position 0→20 stops at the right time but reports 50, and a 3 s keypad run from closed reports 30. Let the strategy own the lead everywhere.
  2. Strategies are rebuilt from fixed classes. _enable_slat_tilt, _promote_to_timed_strategy and _demote_to_simple_strategy replace a custom strategy with Timed/Venetian/Simple and drop its parameters.
  3. There are no run lifecycle hooks. A stateful model needs: run started, run ended (stop / end stop / reversal / abort), position confirmed (end stop, tape reading), position unknown until a full run, and state that survives a restart. #374's tracker is built exactly around those calls; without them it would have to stay as a second path in cover.py.
  4. The test the comment names isn't in the PR. The existing one only overrides travel_for_direction linearly and calls duration_for directly. A piecewise strategy driven through set-position, a keypad run, the stop echo and the 0/100 settle would prove the contract.

With 1–4 in place I'd be glad to approve.

…#739)

Addresses the #739 review: a calibrated strategy (#374) now plugs into
MyHOMECover without touching cover.py.

- Lead: every curtain run takes its lead from strategy.lead_time_for, not
  only covers with slat_tilt; the same lead drives the freeze, set-position
  and stop-echo paths. An auto-stop re-anchor clears it for the remaining
  segment (_move_lead), as a Venetian blind's turned slats did before.
- Transitions: with_slat_tilt / promoted / demoted let the strategy pick its
  successor. Only the built-in classes swap, carrying their timing
  parameters exactly; any other strategy keeps itself.
- Lifecycle: on_run_started, on_run_ended (stop, end_stop, takeover,
  reversal, abort) and on_position_confirmed (end stop, calibration);
  position_after may return None while the position is unknown;
  export_state / restore_state ride on the HA restore data, keyed by
  strategy name. Built-in strategies are stateless no-ops.
- tests/test_cover_strategy_contract.py drives a piecewise, stateful
  strategy (3 s slat phase without tilt control) through keypad runs,
  set-position, the stop echo, the 0/100 settle, every run end, unknown
  position and a restart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@GreenGrassBlueOcean

Copy link
Copy Markdown
Contributor Author

@Interstellar0verdrive thanks for the review. All four points are addressed in 5882b89:

  1. Lead: cover.py no longer gates the lead on slat_tilt. Every curtain run takes its lead from strategy.lead_time_for, and that same lead feeds the freeze, set-position and stop-echo paths. When an auto-stop re-anchors a run on its target, the lead is cleared for the last stretch, which a Venetian blind's turned slats used to do implicitly.

  2. Transitions: with_slat_tilt, promoted and demoted are now strategy methods. Only the built-in classes swap, and they copy their timing parameters exactly; any other strategy keeps itself.

  3. Lifecycle hooks:

    • on_run_started and on_run_ended, with reasons stop, end_stop, takeover, reversal and abort.
    • on_position_confirmed, called at the end stop and after calibration.
    • position_after may return None while the position is unknown.
    • export_state / restore_state go through Home Assistant's restore data, keyed by strategy name.

    The built-in strategies are stateless, so these are no-ops for them.

  4. End-to-end test: tests/test_cover_strategy_contract.py adds a piecewise, stateful strategy with a 3 s slat phase and no tilt control, a slow lift-off and a fast roll. It is driven through:

    • keypad runs: 3 s from closed stays at 0
    • set-position 0→20: runs 8 s and reports 20
    • the stop echo, re-anchored with the strategy's lead
    • the 0/100 settle
    • every way a run can end
    • a position that stays unknown until a full run
    • a restart

    Restoring the old lead gate makes its 3 s keypad run report 12 instead of 0.

Full suite 4,331 passed, 100% coverage on every module, mypy ratchet at 0.

@Interstellar0verdrive Interstellar0verdrive left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 5882b89. Same caveat as always: code review only, plus my own probes.

All four points are in:

  1. The lead comes from the strategy on every run. With a 3 s slat phase on a cover without tilt control, a 3 s keypad run from closed stays at 0 and set_position 0→20 reports 20.
  2. with_slat_tilt, promoted and demoted keep a custom strategy, same object and parameters.
  3. The lifecycle hooks are called on every path: motor start; stop, takeover, reversal and abort; the 0/100 settle with a confirmed end stop; calibration; restore after a restart. The calls in #374's motion tracker map onto them one to one, so the calibrated model can come in as a strategy.
  4. test_cover_strategy_contract.py drives a piecewise, stateful strategy through the entity.

Behaviour is unchanged: my #508 probe set and the #702/#707/#708 cases give the same numbers as the base.

Two nits for later: a keypad run that hits the limit switch reports stop, not end_stop (the bus can't tell), and the scope cover's run timeout sets 100/0 without confirming the position.

@GreenGrassBlueOcean
GreenGrassBlueOcean merged commit 1040503 into v2-phase1-architecture Oct 20, 2026
16 checks passed
@GreenGrassBlueOcean
GreenGrassBlueOcean deleted the arch/issue-730-native-fades-and-cover-strategies branch October 20, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants