Refactor: extract input mode subclasses and add tests Chore/refactor codebase#45
Closed
clintongormley wants to merge 29 commits intoSese-Schneider:mainfrom
Closed
Refactor: extract input mode subclasses and add tests Chore/refactor codebase#45clintongormley wants to merge 29 commits intoSese-Schneider:mainfrom
clintongormley wants to merge 29 commits intoSese-Schneider:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds the main integration entry point that handles config entry setup, unload, and options updates. Forwards setup to the cover platform. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ry flows - Add config flow strings for setup wizard - Add options flow strings for default timing settings - Add config subentries for cover management with add/reconfigure flows - Add selector translations for device type and input mode - Include translations for travel timing and advanced settings sections - Preserve existing service translations for set_known_position and set_known_tilt_position Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implement three flow handlers for UI-based configuration: - CoverTimeBasedConfigFlow: creates the integration entry - CoverTimeBasedOptionsFlow: edits integration-level default timing - CoverTimeBasedSubentryFlow: adds/reconfigures individual cover entities Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add three functions to support creating cover entities from config entry subentries alongside the existing YAML-based async_setup_platform: - async_setup_entry: iterates subentries to create entities and registers custom services - _get_subentry_value: resolves config values with priority from subentry data, entry options, then schema defaults - _entity_from_subentry: constructs a CoverTimeBased entity from a config subentry, handling both switch and cover device types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop the subentry/defaults pattern in favor of a simpler approach: each cover is its own config entry with all settings in one form. - Single page with name, entities, input mode, timing, and a collapsible advanced section - Options flow for reconfiguration with same form - Human-friendly labels and descriptions - No shared defaults — each cover is self-contained Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Step 1: device type (radio) + input mode (radio) - Step 2: entity fields shown conditionally based on device type, pulse time only shown for pulse/toggle modes - Remove name field (handled by helper framework) - Same two-step flow for options/reconfigure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Name field to step 1, move tilt fields into collapsible "Tilt settings" section, move pulse_time to Advanced section, add full translations to translations/en.json, and move description below the name field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add deprecation warning log and repair issue when covers are configured via YAML, directing users to recreate via the UI helpers page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use Platform enum in __init__.py - Add type annotations to async_setup_entry (HomeAssistant, ConfigEntry, AddEntitiesCallback) - Move CONF_DEVICE_TYPE/DEVICE_TYPE_* constants to cover.py to avoid magic strings - Remove misleading `adv = d` alias in _build_details_schema - Add tilt time pair validation (both or neither must be set) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set up pytest test infrastructure with shared fixtures (make_hass, make_cover) and write characterization tests that document the exact relay command sequences for each input mode (switch, pulse, toggle, wrapped cover). Also add toggle behavior tests covering same-direction stop, idle stop guard, and direction change scenarios. These tests serve as a safety net before refactoring. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move CoverTimeBased from cover.py to cover_base.py and add abstract _send_open, _send_close, _send_stop methods. Refactor _async_handle_command to dispatch to these abstract methods while keeping state setting and async_write_ha_state in the dispatcher. Update tests to use a concrete CoverTimeBasedTest subclass that implements the original relay logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Create five subclasses of CoverTimeBased, each implementing the _send_open/_send_close/_send_stop abstract methods for a specific input mode: - WrappedCoverTimeBased: delegates to cover.open/close/stop_cover - SwitchCoverTimeBased: abstract mid-level base for switch-controlled covers - SwitchModeCover: latching relay mode (switches stay on during movement) - PulseModeCover: momentary pulse mode (brief pulse then switch off) - ToggleModeCover: toggle mode (re-press direction to stop, overrides async_close/open/stop_cover for same-direction-stops-movement behavior) All 54 tests pass (25 original + 29 new subclass tests). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…guards from base - Add _create_cover_from_options() factory in cover.py that creates the correct subclass (WrappedCoverTimeBased, SwitchModeCover, PulseModeCover, ToggleModeCover) based on options dict - Update async_setup_entry and devices_from_config to use the factory - Clean up CoverTimeBased base class: remove device-specific params (switch entity IDs, input_mode, pulse_time, cover_entity_id) from constructor - each subclass now only accepts what it needs - Remove toggle-specific guards from base class (async_close_cover, async_open_cover, async_stop_cover, set_known_position, set_known_tilt_position) - toggle behavior now lives in ToggleModeCover - Add stop-before-direction-change to base class async_close_cover and async_open_cover (benefits all modes, not just toggle) - Move CONF_* constants to cover_base.py to eliminate deferred import in extra_state_attributes - Add -> None return type annotations to all _send_open/_send_close/ _send_stop implementations - Update all test factories to use new constructor signatures - Remove CoverTimeBasedTest from conftest; make_cover now uses the factory to create real subclasses Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…t_position Use _async_handle_command(SERVICE_STOP_COVER) instead of bare _send_stop() to ensure self._state is set and async_write_ha_state() is called. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract _start_movement() helper to replace 6 duplicated startup-delay patterns - Unify async_close_cover/async_open_cover into _async_move_to_endpoint(target) - Unify async_close_cover_tilt/async_open_cover_tilt into _async_move_tilt_to_endpoint(target) - Extract _handle_pre_movement_checks() and _is_movement_too_short() from set_position/set_tilt_position - Fix startup delay conflict check ordering: check before position check so direction reversal during startup delay is properly cancelled Reduces cover_base.py from 1057 to 767 lines (27% reduction). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
63 new tests covering: - Travel/tilt endpoint movement (close, open, direction changes) - Tilt-travel coupling (both directions, with/without coupling) - set_position and set_tilt_position (direction, already-at-target, endpoints) - Min movement time filtering (short moves, endpoints always allowed) - Startup delay (creation, same-direction dedup, direction-change cancellation) - Relay delay at endpoints (auto-stop, mid-point stop, cancellation) - Stop cover, set_known_position/tilt, properties, tilt constraints Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract _calc_coupled_target() and _begin_movement() in cover_base.py to eliminate repeated coupled-calculator and start-movement patterns across _async_move_to_endpoint, _async_move_tilt_to_endpoint, set_position, and set_tilt_position - Simplify devices_from_config() in cover.py with data-driven loop over _TIMING_DEFAULTS, extract _get_value() and _resolve_input_mode() - Apply ruff formatting fixes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ning Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes "Task was destroyed but it is pending" warnings in CI by cancelling any leftover _startup_delay_task when tests finish. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
clintongormley
force-pushed
the
chore/refactor-codebase
branch
from
c67ecd5 to
38cf3da
Compare
3 tasks
The pytest-homeassistant-custom-component verify_cleanup fixture detects pending tasks before our make_cover teardown runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
I'm closing this PR as the work I've been doing to add UI configuration and calibration has largely made the changes in #44 redundant. I'll open a bigger PR later. |
Owner
|
@clintongormley sorry for not getting to review now, looking forward for the big combined PR! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds tests giving almost complete coverage of the codebase. The code has been refactored to reduce deduplication and to make it easier to reason about. The
covervsswitchvspulsevstoggleinput modes are now handled by sub-classes rather than long if-then chains.Also formatted the whole codebase with ruff to make future diffs less fragile.
Please merge #44 first as this PR is based on that branch. The diff will be easier to reason about once #44 is merged.
Summary
CoverTimeBasedbase class tocover_base.pywith abstract_send_open/_send_close/_send_stopmethodsSwitchModeCover,PulseModeCover,ToggleModeCover,WrappedCoverTimeBased_calc_coupled_target,_begin_movement) to reduce duplication across movement methodsdevices_from_configwith data-driven config extraction loopTest plan
pytest tests/ -v)ruff check .andruff format .clean🤖 Generated with Claude Code
Relates #42