From e839f47fdb4eadfd96b6e118e3d6d5fda112cbba Mon Sep 17 00:00:00 2001 From: hrabbach Date: Sat, 27 Jun 2026 17:19:02 +0200 Subject: [PATCH] Phase 5: code review fixes + manual quality gate (v1.3.3) Reviews every inherited module (22 findings, severity-triaged) and fixes all 15 actionable findings, then consolidates the quality gate. Fixes: - CR-01 auto-reconnect on serial connection loss (via hass.loop) - CR-02/WR-07 wrap blocking serial.Serial() opens in the executor - CR-03 use time.monotonic() for elapsed calibration timing - CR-04 log broad excepts (_LOGGER.exception) - CR-05 route legacy calibration save through cover._save_calibration() - T-05-04 fix sl-branch length guard (>= 8 -> >= 12) against truncated device_id - WR-01/02/04/08/09/11/13, IN-03 and related cleanups Tooling: - Install codespell + migrate config to pyproject.toml [tool.codespell] - Remove .pre-commit-config.yaml and the orphaned pre-commit dependency - Add CONTRIBUTING.md documenting the manual gate (pytest/ruff/mypy/codespell) - Add tests/test_quality_gate_tooling.py regression guards Gate green: 212 tests pass; ruff/mypy/codespell clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QwPMgtiypLgJ5nkR3mUGfL --- .pre-commit-config.yaml | 50 --------- CONTRIBUTING.md | 80 ++++++++++++++ README.md | 6 ++ .../schellenberg_usb/__init__.py | 5 +- custom_components/schellenberg_usb/api.py | 29 +++-- .../schellenberg_usb/config_flow.py | 42 +++++--- custom_components/schellenberg_usb/const.py | 2 +- custom_components/schellenberg_usb/cover.py | 62 ++++++----- .../schellenberg_usb/manifest.json | 3 +- .../schellenberg_usb/options_flow.py | 13 ++- .../options_flow_calibration.py | 89 +++++++++------ .../schellenberg_usb/options_flow_pairing.py | 21 +++- .../options_flow_timed_calibration.py | 16 +-- pyproject.toml | 12 ++- tests/test_api_messages.py | 12 +-- tests/test_config_flow.py | 6 +- tests/test_cover.py | 102 +++++------------- tests/test_options_flow.py | 8 +- tests/test_quality_gate_tooling.py | 43 ++++++++ tests/test_timed_cal_handler_structure.py | 11 +- tests/test_timed_calibration_flow.py | 53 ++++----- uv.lock | 92 +++------------- 22 files changed, 386 insertions(+), 371 deletions(-) delete mode 100644 .pre-commit-config.yaml create mode 100644 CONTRIBUTING.md create mode 100644 tests/test_quality_gate_tooling.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml deleted file mode 100644 index 40980f6..0000000 --- a/.pre-commit-config.yaml +++ /dev/null @@ -1,50 +0,0 @@ -repos: - - repo: https://github.com/astral-sh/uv-pre-commit - # uv version. - rev: 0.9.5 - hooks: - - id: uv-lock - - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.14.1 - hooks: - - id: ruff - args: [ --fix ] - files: ^(homeassistant|script|tests|custom_components)/.+\.py$ - - id: ruff-format - files: ^(homeassistant|script|tests|custom_components)/.+\.py$ - - repo: https://github.com/codespell-project/codespell - rev: v2.4.1 - hooks: - - id: codespell - args: - - --ignore-words-list=hass,alot,datas,dof,dur,farenheit,hist,iff,ines,ist,lightsensor,mut,nd,pres,referer,ser,serie,te,technik,ue,uint,visability,wan,wanna,withing - - --skip="./.*,*.csv,*.json" - - --quiet-level=2 - exclude_types: [ csv, json ] - - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v6.0.0 - hooks: - - id: check-executables-have-shebangs - stages: [ manual ] - - id: check-json - # These hooks require the locally installed venv - - repo: local - hooks: - - id: mypy - name: mypy - entry: uv run mypy - args: - - --pretty - - --show-error-codes - - --show-error-context - language: system - pass_filenames: true - files: ^(tests|custom_components)/.+\.py$ - - id: pytest - name: pytest - entry: uv run pytest - args: - - --no-cov - language: system - pass_filenames: false - files: ^(tests|custom_components)/.+\.(py|json|yaml)$ \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..83e66ee --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,80 @@ +# Contributing + +Thanks for helping improve the Schellenberg USB integration. This document is the +canonical guide to the local quality gate every change must pass before it is merged. + +## Development Setup + +- **Python 3.13.2+** +- **[uv](https://docs.astral.sh/uv/)** for dependency management (`uv sync` installs the + `dev` group, which pulls in both `test` and `lint`). +- **Windows contributors:** the test suite must run under **WSL2 (Ubuntu)** — see + [Why WSL for tests?](#why-wsl-for-tests) below. Linting and type-checking run natively + on Windows in a separate `.venv-win` environment. + +## Quality Gate + +There is no pre-commit hook — the gate is run manually. All four checks must pass before a +change is merged. Tests run under Linux/WSL; lint, type-check, and spell-check run natively. + +### 1. Tests (Linux / WSL) + +On Linux: + +```sh +uv run pytest -p no:cacheprovider -q +``` + +On **Windows**, run the same command inside WSL — the Home Assistant test harness needs a +Linux environment. Keep the venv on the WSL **ext4** filesystem (not `/mnt/c`, which is slow +DrvFs) so uv can hardlink from its cache and sync in seconds. The maintainer wraps this in a +small `.wsl_exec.sh` helper (which sets a clean `HOME`, points `UV_PROJECT_ENVIRONMENT` at an +ext4 venv, serializes calls with `flock`, and `cd`s into the repo) and invokes it as: + +```powershell +wsl -e env -u HOME -u WSLENV bash .wsl_exec.sh "uv run --no-sync pytest -p no:cacheprovider -q" +``` + +`--no-sync` skips uv's implicit env sync on every run; run `uv sync --frozen` explicitly only +when dependencies change. + +### 2. Lint (native Windows / `.venv-win`) + +```powershell +$env:UV_PROJECT_ENVIRONMENT = ".venv-win" +uv run ruff check custom_components/schellenberg_usb/ tests/ +uv run ruff format --check custom_components/schellenberg_usb/ tests/ +``` + +### 3. Type check (native Windows / `.venv-win`) + +```powershell +$env:UV_PROJECT_ENVIRONMENT = ".venv-win" +uv run mypy custom_components/schellenberg_usb/ tests/ +``` + +### 4. Spell check (native Windows / `.venv-win`) + +```powershell +$env:UV_PROJECT_ENVIRONMENT = ".venv-win" +uv run codespell custom_components/schellenberg_usb/ tests/ README.md CONTRIBUTING.md +``` + +codespell configuration (the ignore-words list for intentional domain terms) lives in the +`[tool.codespell]` section of `pyproject.toml`. If codespell flags a legitimate term, add +it to `ignore-words-list` there. + +## Why WSL for tests? + +The Home Assistant test harness imports the Unix-only `fcntl` module +(`homeassistant/runner.py`), so the suite cannot run on native Windows — `uv run pytest` +there fails with `ModuleNotFoundError: No module named 'fcntl'`. Running the tests inside +WSL (or on any Linux/macOS host) avoids this. Lint, type-check, and spell-check have no +such constraint and run natively for speed. + +## Commit Conventions + +- Keep commits focused — one logical change per commit. +- Use conventional-commit prefixes (`feat:`, `fix:`, `docs:`, `chore:`, `refactor:`, + `test:`, `style:`). +- Run the full quality gate before pushing. diff --git a/README.md b/README.md index 46078fd..bbac0ff 100644 --- a/README.md +++ b/README.md @@ -224,3 +224,9 @@ Once a timed motor has been calibrated, the position slider in the Home Assistan 3. Schedule a stop command after the computed fraction of the full-travel time has elapsed. The integration tracks position by dead-reckoning — it uses the calibrated open and close times to estimate where the shutter is. If the motor is ever moved outside of Home Assistant (e.g., by a physical remote), the tracked position will drift until the next full-travel recalibration. + +--- + +## Contributing + +Contributions are welcome. See [CONTRIBUTING.md](CONTRIBUTING.md) for the development setup and the local quality gate (tests, lint, type-check, and spell-check) every change must pass. diff --git a/custom_components/schellenberg_usb/__init__.py b/custom_components/schellenberg_usb/__init__.py index 9cbf5b3..7b9f5df 100644 --- a/custom_components/schellenberg_usb/__init__.py +++ b/custom_components/schellenberg_usb/__init__.py @@ -151,9 +151,7 @@ async def _on_entry_updated( return new_ignore = updated_entry.options.get(CONF_IGNORE_UNKNOWN, False) if api_instance.ignore_unknown != new_ignore: - _LOGGER.debug( - "Live-applying ignore_unknown=%s to running API", new_ignore - ) + _LOGGER.debug("Live-applying ignore_unknown=%s to running API", new_ignore) api_instance.ignore_unknown = new_ignore entry.add_update_listener(_on_entry_updated) @@ -175,5 +173,6 @@ async def async_unload_entry( if unload_ok: api: SchellenbergUsbApi = entry.runtime_data await api.disconnect() + _SETUP_CALLBACKS.pop(entry.entry_id, None) return unload_ok diff --git a/custom_components/schellenberg_usb/api.py b/custom_components/schellenberg_usb/api.py index 3fd381e..47be551 100644 --- a/custom_components/schellenberg_usb/api.py +++ b/custom_components/schellenberg_usb/api.py @@ -97,8 +97,8 @@ async def connect(self) -> None: ( self._transport, self._protocol, - ) = await serial_asyncio.create_serial_connection( - self.hass.loop, + ) = await serial_asyncio.create_serial_connection( # type: ignore[assignment] + asyncio.get_running_loop(), lambda: SchellenbergProtocol(self._handle_message, self), self.port, baudrate=112500, @@ -151,7 +151,9 @@ async def connect(self) -> None: ) self._is_connecting = False # Always retry after 5 seconds - self.hass.loop.call_later(5, lambda: self.hass.create_task(self.connect())) + asyncio.get_running_loop().call_later( + 5, lambda: self.hass.async_create_task(self.connect()) + ) @callback def _handle_message(self, message: str) -> None: @@ -220,7 +222,10 @@ def _handle_message(self, message: str) -> None: # 00BE = 2 bytes to ignore (address prefix) # XXXXXX = 3 bytes device ID (the actual device ID we want) # Rest = can be ignored - if message.startswith("sl") and len(message) >= 8: + # Guard: slice [6:12] requires len >= 12 (end index = 12). The previous + # >= 8 guard was a defect (Pattern 2 / T-05-04) — on an 8-11 char frame + # the slice silently returns a truncated/empty device_id. + if message.startswith("sl") and len(message) >= 12: # Extract the device ID: skip "sl" (2 chars) + "00BE" (4 chars) = 6 chars # Then take the next 6 characters (3 bytes as hex) = 6 chars device_id = message[6:12] @@ -291,8 +296,7 @@ def _handle_message(self, message: str) -> None: # "Ignore unknown signals" hub option is on — demote the # unknown-device line to DEBUG to keep logs quiet (SIG-01). _LOGGER.debug( - "Ignoring signal from unknown device %s " - "(enum=%s, cmd=%s)", + "Ignoring signal from unknown device %s (enum=%s, cmd=%s)", device_id, device_enum, command, @@ -376,7 +380,7 @@ async def pair_device_and_wait(self) -> tuple[str, str] | None: ) # Create a future to wait for device ID first - self._pairing_future = self.hass.loop.create_future() + self._pairing_future = asyncio.get_running_loop().create_future() try: # Send sp command to enter pairing/listening mode (like C# does) @@ -524,7 +528,7 @@ async def verify_device(self) -> bool: return False _LOGGER.debug("Verifying Schellenberg USB device") - self._verify_future = self.hass.loop.create_future() + self._verify_future = asyncio.get_running_loop().create_future() try: # Send the verification command @@ -695,7 +699,7 @@ async def get_device_id(self) -> str | None: return None _LOGGER.debug("Requesting device ID") - self._device_id_future = self.hass.loop.create_future() + self._device_id_future = asyncio.get_running_loop().create_future() try: # Send the request command @@ -780,3 +784,10 @@ def connection_lost(self, exc: Exception | None) -> None: """Called when the connection is lost.""" _LOGGER.warning("Serial port connection lost: %s", exc) self.api.update_connection_status(False) + # Schedule a reconnect attempt so a runtime USB blip recovers + # automatically. Use hass.loop (not asyncio.get_running_loop()) because + # this transport callback can be invoked synchronously — e.g. in tests — + # where no running loop is present; hass.loop is always the correct loop. + self.api.hass.loop.call_later( + 5, lambda: self.api.hass.async_create_task(self.api.connect()) + ) diff --git a/custom_components/schellenberg_usb/config_flow.py b/custom_components/schellenberg_usb/config_flow.py index 1e01ec0..f015f0b 100644 --- a/custom_components/schellenberg_usb/config_flow.py +++ b/custom_components/schellenberg_usb/config_flow.py @@ -84,10 +84,13 @@ async def async_step_user(self, user_input: dict | None = None) -> ConfigFlowRes if user_input is not None: port = user_input[CONF_SERIAL_PORT] try: - # Quick, blocking sanity check that the port is reachable. - serial_conn = serial.Serial(port) + # Run blocking serial open in the executor to avoid blocking the + # HA event loop (CR-02 — serial.Serial() can block for 100-500ms). + def _open_serial(p: str) -> None: + conn = serial.Serial(p) + conn.close() - serial_conn.close() + await self.hass.async_add_executor_job(_open_serial, port) # Use the port path as the unique ID when set up manually. await self.async_set_unique_id(port, raise_on_progress=False) @@ -99,8 +102,10 @@ async def async_step_user(self, user_input: dict | None = None) -> ConfigFlowRes except serial.SerialException: errors["base"] = "cannot_connect" _LOGGER.error("Failed to connect to serial port %s", port) - except Exception: + except Exception: # noqa: BLE001 errors["base"] = "unknown" + # HA config-flow must surface 'unknown' to the user rather than + # crashing the flow; broad catch is intentional here (RESEARCH Pitfall 7). _LOGGER.exception("An unexpected error occurred") return self._form_schema(errors, default_port="/dev/ttyUSB0") @@ -148,8 +153,13 @@ async def async_step_usb_confirm( if user_input is not None: port = user_input[CONF_SERIAL_PORT] try: - serial_conn = serial.Serial(port) - serial_conn.close() + # Run blocking serial open in the executor to avoid blocking the + # HA event loop (CR-02 — serial.Serial() can block for 100-500ms). + def _open_serial(p: str) -> None: + conn = serial.Serial(p) + conn.close() + + await self.hass.async_add_executor_job(_open_serial, port) # unique_id was already set in async_step_usb(), re-assert and create the entry await self.async_set_unique_id( @@ -164,8 +174,10 @@ async def async_step_usb_confirm( except serial.SerialException: errors["base"] = "cannot_connect" _LOGGER.error("Failed to connect to serial port %s", port) - except Exception: + except Exception: # noqa: BLE001 errors["base"] = "unknown" + # HA config-flow must surface 'unknown' to the user rather than + # crashing the flow; broad catch is intentional here (RESEARCH Pitfall 7). _LOGGER.exception("An unexpected error occurred during USB confirm") # Mark as confirm-only so the UI shows a simple confirmation experience @@ -282,12 +294,8 @@ async def async_step_manual_add( if not errors: # Resolve mode — BooleanSelector returns a real Python bool - is_bidirectional: bool = bool( - user_input.get(CONF_BIDIRECTIONAL, True) - ) - device_name = ( - user_input.get("device_name") or f"Blind {device_enum}" - ) + is_bidirectional: bool = bool(user_input.get(CONF_BIDIRECTIONAL, True)) + device_name = user_input.get("device_name") or f"Blind {device_enum}" self._pending_device_enum = device_enum self._pending_device_name = device_name self._pending_is_bidirectional = is_bidirectional @@ -307,9 +315,7 @@ async def async_step_manual_add( unique_id=device_enum, ) # Timed: advance to initial-position step - _LOGGER.debug( - "Timed motor %s: advancing to position step", device_enum - ) + _LOGGER.debug("Timed motor %s: advancing to position step", device_enum) return await self.async_step_manual_position() return self.async_show_form( @@ -477,6 +483,10 @@ async def async_step_reconfigure( device_enum = subentry.data.get("device_enum") if not device_id: return self.async_abort(reason="device_not_found") + if not device_enum: + # WR-13: device_enum=None would produce malformed protocol command + # f"ss{None}9{CMD_DOWN}0000" sent to the USB stick. + return self.async_abort(reason="device_not_found") # Route by motor type (CTRL-05 zero-regression requirement): # - bidirectional motors → legacy event-based CalibrationFlowHandler diff --git a/custom_components/schellenberg_usb/const.py b/custom_components/schellenberg_usb/const.py index 54904a4..ba1e8bd 100644 --- a/custom_components/schellenberg_usb/const.py +++ b/custom_components/schellenberg_usb/const.py @@ -108,7 +108,7 @@ # Timed calibration guards (D-08 / D-09) CAL_MAX_TRAVEL_TIME = 120 # seconds — reject "walked away" runs (D-08) -CAL_MIN_TRAVEL_TIME = 2 # seconds — reject double-press/misfire (D-09) +CAL_MIN_TRAVEL_TIME = 2 # seconds — reject double-press/misfire (D-09) # Manual-add device mode flag (stored in subentry.data) CONF_BIDIRECTIONAL = "bidirectional" # bool; False = timed/non-bidirectional diff --git a/custom_components/schellenberg_usb/cover.py b/custom_components/schellenberg_usb/cover.py index 3c7b024..7a5ed7a 100644 --- a/custom_components/schellenberg_usb/cover.py +++ b/custom_components/schellenberg_usb/cover.py @@ -1,4 +1,5 @@ """Cover platform for Schellenberg USB.""" + from __future__ import annotations import asyncio @@ -167,8 +168,11 @@ async def async_setup_entry( ) if existing_entity_id: - entry_entity = entity_registry.entities[existing_entity_id] - if entry_entity.config_subentry_id != subentry.subentry_id: + entry_entity = entity_registry.async_get(existing_entity_id) + if ( + entry_entity is not None + and entry_entity.config_subentry_id != subentry.subentry_id + ): _LOGGER.info( "Updating existing cover entity %s to subentry %s", existing_entity_id, @@ -222,11 +226,8 @@ class SchellenbergCover(CoverEntity, RestoreEntity): _attr_should_poll = False _unrecorded_attributes = frozenset({"mode", "calibrated"}) - _attr_supported_features = ( - CoverEntityFeature.OPEN - | CoverEntityFeature.CLOSE - | CoverEntityFeature.STOP - | CoverEntityFeature.SET_POSITION + _BASE_FEATURES = ( + CoverEntityFeature.OPEN | CoverEntityFeature.CLOSE | CoverEntityFeature.STOP ) def __init__( @@ -327,6 +328,21 @@ def entity_registry_enabled_default(self) -> bool: """Return if entity should be enabled by default.""" return True + @property + def supported_features(self) -> CoverEntityFeature: + """Return supported features, adding SET_POSITION only when usable. + + For timed (non-bidirectional) motors, SET_POSITION is only meaningful + once calibration data is available. Advertising it on uncalibrated + motors shows a position slider in HA's UI that silently does nothing + (IN-03) — confusing users. Re-evaluation happens on + _handle_calibration_completed via async_write_ha_state(). + """ + features = self._BASE_FEATURES + if self._is_bidirectional or self._is_calibrated: + features = features | CoverEntityFeature.SET_POSITION + return features + @property def extra_state_attributes(self) -> dict[str, Any]: """Return device-specific state attributes.""" @@ -337,9 +353,7 @@ def extra_state_attributes(self) -> dict[str, Any]: attrs["calibrated"] = self._is_calibrated return attrs - def _restore_position_from_last_state( - self, last_state: Any - ) -> None: + def _restore_position_from_last_state(self, last_state: Any) -> None: """Restore cover position from a HA last-known state. Contains the generic recorded-position restore logic: raw_position @@ -369,9 +383,7 @@ def _restore_position_from_last_state( restored_position = 0 if restored_position is not None: - self._attr_current_cover_position = max( - 0, min(100, restored_position) - ) + self._attr_current_cover_position = max(0, min(100, restored_position)) self._attr_is_closed = self._attr_current_cover_position == 0 _LOGGER.debug( "Restored position for %s (%s) to %d%% (raw=%s)", @@ -398,16 +410,14 @@ async def async_added_to_hass(self) -> None: self._attr_current_cover_position = 100 self._attr_is_closed = False _LOGGER.debug( - "Timed motor %s was opening at restart," - " snapping to 100%%", + "Timed motor %s was opening at restart, snapping to 100%%", self._attr_name, ) elif last_state.state == "closing": self._attr_current_cover_position = 0 self._attr_is_closed = True _LOGGER.debug( - "Timed motor %s was closing at restart," - " snapping to 0%%", + "Timed motor %s was closing at restart, snapping to 0%%", self._attr_name, ) else: @@ -596,7 +606,9 @@ def _handle_event(self, event: str) -> None: self._target_position = None else: - _LOGGER.debug("Device %s received unknown event: %s", self._attr_name, event) + _LOGGER.debug( + "Device %s received unknown event: %s", self._attr_name, event + ) self.async_write_ha_state() @@ -712,9 +724,7 @@ def _update_position(self) -> None: else: return - self._attr_current_cover_position = max( - 0, min(100, int(round(new_pos))) - ) + self._attr_current_cover_position = max(0, min(100, int(round(new_pos)))) self._attr_is_closed = self._attr_current_cover_position == 0 _LOGGER.debug( @@ -725,9 +735,7 @@ def _update_position(self) -> None: travel_time, ) - async def async_open_cover( - self, target: int | None = None, **kwargs: Any - ) -> None: + async def async_open_cover(self, target: int | None = None, **kwargs: Any) -> None: """Open the cover. ``target`` is the partial-move target for a set-position driven @@ -749,9 +757,7 @@ async def async_open_cover( self.async_write_ha_state() await self._api.control_blind(self._device_enum, CMD_UP) - async def async_close_cover( - self, target: int | None = None, **kwargs: Any - ) -> None: + async def async_close_cover(self, target: int | None = None, **kwargs: Any) -> None: """Close cover. ``target`` is the partial-move target for a set-position driven @@ -829,4 +835,4 @@ async def async_set_cover_position(self, **kwargs: Any) -> None: ) await self.async_close_cover(target=target_position) # The position tracking loop will automatically send the stop command - # when the target position is reached. \ No newline at end of file + # when the target position is reached. diff --git a/custom_components/schellenberg_usb/manifest.json b/custom_components/schellenberg_usb/manifest.json index 1ec38b0..f6199b9 100644 --- a/custom_components/schellenberg_usb/manifest.json +++ b/custom_components/schellenberg_usb/manifest.json @@ -4,6 +4,7 @@ "codeowners": ["@hrabbach"], "config_flow": true, "documentation": "https://github.com/hrabbach/schellenberg_usb", + "homeassistant": "2024.11.0", "integration_type": "hub", "iot_class": "local_push", "issue_tracker": "https://github.com/hrabbach/schellenberg_usb/issues", @@ -15,5 +16,5 @@ "manufacturer": "van ooijen" } ], - "version": "1.3.2" + "version": "1.3.3" } diff --git a/custom_components/schellenberg_usb/options_flow.py b/custom_components/schellenberg_usb/options_flow.py index 8cf74a6..ed4279a 100644 --- a/custom_components/schellenberg_usb/options_flow.py +++ b/custom_components/schellenberg_usb/options_flow.py @@ -40,14 +40,21 @@ async def async_step_init( new_ignore = user_input.get(CONF_IGNORE_UNKNOWN, False) if new_port != current_port: try: - serial_conn = serial.Serial(new_port) - serial_conn.close() + # Run blocking serial open in the executor to avoid blocking + # the HA event loop (WR-07 / CR-02 pattern). + def _open_serial(p: str) -> None: + conn = serial.Serial(p) + conn.close() + + await self.hass.async_add_executor_job(_open_serial, new_port) except serial.SerialException: _LOGGER.error( "Failed to open serial port %s during options save", new_port ) self._errors["base"] = "cannot_connect" - except Exception: + except Exception: # noqa: BLE001 + # HA options flow must surface 'unknown' to the user rather than + # crashing the flow; broad catch is intentional (RESEARCH Pitfall 7). _LOGGER.exception("Unexpected error validating port %s", new_port) self._errors["base"] = "unknown" else: diff --git a/custom_components/schellenberg_usb/options_flow_calibration.py b/custom_components/schellenberg_usb/options_flow_calibration.py index 928d6fa..e6db326 100644 --- a/custom_components/schellenberg_usb/options_flow_calibration.py +++ b/custom_components/schellenberg_usb/options_flow_calibration.py @@ -225,8 +225,8 @@ async def async_step_calibration_open_instruction( last_step=False, ) - # Start timing the open movement - self._calibration_start_time = time.time() + # Start timing the open movement (before await — CR-03: use monotonic clock) + self._calibration_start_time = time.monotonic() # Wait for device to stop moving stop_ok = await self._wait_for_stop_event() @@ -243,13 +243,17 @@ async def async_step_calibration_open_instruction( ) # Record the open time - self._open_time = time.time() - self._calibration_start_time + self._open_time = time.monotonic() - self._calibration_start_time _LOGGER.debug("Calibration open_time: %s seconds", self._open_time) # Move to close instruction step return await self.async_step_calibration_close_instruction() except Exception: # noqa: BLE001 + # Broad catch intentional: unexpected errors (asyncio cancellation, + # dispatcher internals) must set errors["base"] and return the form + # rather than crash the flow. Log for diagnostics (CR-04). + _LOGGER.exception("Calibration open step failed unexpectedly") errors["base"] = "unknown" return self.flow.async_show_form( step_id="calibration_open_instruction", @@ -298,8 +302,8 @@ async def async_step_calibration_close_instruction( last_step=False, ) - # Start timing the close movement - self._calibration_start_time = time.time() + # Start timing the close movement (before await — CR-03: use monotonic clock) + self._calibration_start_time = time.monotonic() # Wait for device to stop moving stop_ok = await self._wait_for_stop_event() @@ -316,13 +320,17 @@ async def async_step_calibration_close_instruction( ) # Record the close time - self._close_time = time.time() - self._calibration_start_time + self._close_time = time.monotonic() - self._calibration_start_time _LOGGER.debug("Calibration close_time: %s seconds", self._close_time) # Move to completion step return await self.async_step_calibration_complete() except Exception: # noqa: BLE001 + # Broad catch intentional: unexpected errors (asyncio cancellation, + # dispatcher internals) must set errors["base"] and return the form + # rather than crash the flow. Log for diagnostics (CR-04). + _LOGGER.exception("Calibration close step failed unexpectedly") errors["base"] = "unknown" return self.flow.async_show_form( step_id="calibration_close_instruction", @@ -397,14 +405,15 @@ async def _wait_for_movement_start(self, event_type: str) -> bool: return False device_id = self._selected_device["id"] self._start_event = asyncio.Event() - loop = asyncio.get_event_loop() # Set up listener for movement start events + # HA's async_dispatcher_connect callbacks run in the event loop thread, + # so a direct .set() is correct; call_soon_threadsafe is not needed (WR-08). def handle_device_event(command: str) -> None: """Handle device event.""" if command == event_type: if self._start_event: - loop.call_soon_threadsafe(self._start_event.set) + self._start_event.set() # Subscribe to device events self._event_listener_unsub = async_dispatcher_connect( @@ -439,14 +448,15 @@ async def _wait_for_stop_event(self) -> bool: return False device_id = self._selected_device["id"] self._stop_event = asyncio.Event() - loop = asyncio.get_event_loop() # Set up listener for stop events + # HA's async_dispatcher_connect callbacks run in the event loop thread, + # so a direct .set() is correct; call_soon_threadsafe is not needed (WR-08). def handle_device_event(command: str) -> None: """Handle device event.""" if command == EVENT_STOPPED: if self._stop_event: - loop.call_soon_threadsafe(self._stop_event.set) + self._stop_event.set() # Subscribe to device events self._event_listener_unsub = async_dispatcher_connect( @@ -470,37 +480,56 @@ def handle_device_event(command: str) -> None: self._stop_event = None async def _save_calibration_data(self, open_time: float, close_time: float) -> None: - """Save calibration times to device storage and set cover position. + """Save calibration times to cover calibration store and set cover position. + + Uses cover._save_calibration() (same store key as cover.py reads from) + to fix the CR-05 key mismatch where the old code wrote to STORAGE_KEY + ('schellenberg_usb_devices') while cover.py reads from '_CAL_STORE_KEY' + ('schellenberg_usb_calibration'). After calibration completes, the device is in fully closed position, so we update the cover entity position to 0. """ - storage: Store = Store(self.flow.hass, STORAGE_VERSION, STORAGE_KEY) - stored_data = await storage.async_load() or {"devices": []} + if self._selected_device is None: + return + + # Resolve the hub entry_id regardless of flow type + # (OptionsFlow has config_entry; ConfigSubentryFlow has _get_entry()) + hub_entry = ( + getattr(self.flow, "config_entry", None) + or getattr(self.flow, "_get_entry", lambda: None)() + ) + if hub_entry is None: + _LOGGER.error( + "CR-05: Cannot resolve hub entry from flow %s — calibration not saved", + type(self.flow).__name__, + ) + return - # Find and update the device - if self._selected_device is not None: - for device in stored_data.get("devices", []): - if device["id"] == self._selected_device["id"]: - device[CONF_OPEN_TIME] = round(open_time, 2) - device[CONF_CLOSE_TIME] = round(close_time, 2) - break + # Lazy import to avoid a cover<->flow import cycle at module load + # (same pattern as options_flow_timed_calibration.py line 324). + from .cover import _save_calibration # noqa: PLC0415 - await storage.async_save(stored_data) + await _save_calibration( + self.flow.hass, + hub_entry.entry_id, + self._selected_device["id"], + round(open_time, 2), + round(close_time, 2), + ) # Send signal to notify entities that calibration has been completed. # Explicit final_position=0: legacy flow ends on a close run (D-14 / # REVIEW-1). The handler default already covers the 3-arg case, but # passing 0 explicitly documents intent and guards future refactors. - if self._selected_device is not None: - async_dispatcher_send( - self.flow.hass, - SIGNAL_CALIBRATION_COMPLETED, - self._selected_device["id"], - round(open_time, 2), - round(close_time, 2), - 0, - ) + async_dispatcher_send( + self.flow.hass, + SIGNAL_CALIBRATION_COMPLETED, + self._selected_device["id"], + round(open_time, 2), + round(close_time, 2), + 0, + ) def enable_subentry_creation( self, diff --git a/custom_components/schellenberg_usb/options_flow_pairing.py b/custom_components/schellenberg_usb/options_flow_pairing.py index f986174..0b7889c 100644 --- a/custom_components/schellenberg_usb/options_flow_pairing.py +++ b/custom_components/schellenberg_usb/options_flow_pairing.py @@ -13,7 +13,16 @@ class PairingFlowHandler: - """Handle pairing options flow steps.""" + """Handle pairing options flow steps. + + LEGACY: This handler is unreachable in the current UI path. The active + pairing flow is SchellenbergPairingSubentryFlow in config_flow.py. + This class is retained only because CalibrationFlowHandler references + get_last_paired_device_id() via getattr() fallback. The async_step_name_device + 'handle_new_device_no_reload' branch is dead — __init__.py never registers + that key (WR-09). Do NOT delete this file until CalibrationFlowHandler's + async_step_calibration_after_pairing is also removed or rerouted. + """ def __init__(self, flow: OptionsFlow) -> None: """Initialize the pairing flow handler.""" @@ -71,9 +80,11 @@ async def async_step_name_device( # User provided a name or left it empty device_name = user_input.get(CONF_DEVICE_NAME) or f"Blind {self._device_id}" - # Call the handle_new_device_no_reload function to save without reloading + # WR-09: 'handle_new_device_no_reload' is never registered by __init__.py, + # so this branch has always been dead. Abort with a descriptive reason + # instead of silently completing with no-op persistence. hass = self.flow.hass - handle_new_device_no_reload = hass.data["schellenberg_usb"].get( + handle_new_device_no_reload = hass.data.get("schellenberg_usb", {}).get( "handle_new_device_no_reload" ) if handle_new_device_no_reload: @@ -83,6 +94,10 @@ async def async_step_name_device( # Reload to create the entity (wait for it to complete) entry = self.flow.config_entry await hass.config_entries.async_reload(entry.entry_id) + else: + # Dead path: handler not registered — abort to avoid silent no-op. + # Use the active SchellenbergPairingSubentryFlow in config_flow.py instead. + return self.flow.async_abort(reason="not_supported") # Pairing complete - end the flow return self.flow.async_create_entry(title="", data={}) diff --git a/custom_components/schellenberg_usb/options_flow_timed_calibration.py b/custom_components/schellenberg_usb/options_flow_timed_calibration.py index 66844af..b8cd4a1 100644 --- a/custom_components/schellenberg_usb/options_flow_timed_calibration.py +++ b/custom_components/schellenberg_usb/options_flow_timed_calibration.py @@ -121,9 +121,7 @@ async def async_step_timed_cal_close( device_enum = self._selected_device.get("enum", "") self._close_start_time = time.monotonic() # D-07 — BEFORE await await api.control_blind(device_enum, CMD_DOWN) # D-04 close-first - _LOGGER.debug( - "Timed calibration: close command sent to %s", device_enum - ) + _LOGGER.debug("Timed calibration: close command sent to %s", device_enum) return self.flow.async_show_form( step_id="timed_cal_close", data_schema=vol.Schema({}), @@ -196,9 +194,7 @@ async def async_step_timed_cal_open( device_enum = self._selected_device.get("enum", "") self._open_start_time = time.monotonic() # D-07 — BEFORE await await api.control_blind(device_enum, CMD_UP) # D-04 open-second - _LOGGER.debug( - "Timed calibration: open command sent to %s", device_enum - ) + _LOGGER.debug("Timed calibration: open command sent to %s", device_enum) return self.flow.async_show_form( step_id="timed_cal_open", data_schema=vol.Schema({}), @@ -243,9 +239,7 @@ async def async_step_timed_cal_open( ) self._open_time = round(elapsed, 2) - _LOGGER.debug( - "Timed calibration: open_time recorded as %s s", self._open_time - ) + _LOGGER.debug("Timed calibration: open_time recorded as %s s", self._open_time) return await self.async_step_timed_cal_confirm() async def async_step_timed_cal_confirm( @@ -283,9 +277,7 @@ async def async_step_timed_cal_confirm( return self.flow.async_show_form( step_id="timed_cal_confirm", - data_schema=vol.Schema( - {vol.Optional("redo", default=False): bool} - ), + data_schema=vol.Schema({vol.Optional("redo", default=False): bool}), description_placeholders={ "device_name": self._selected_device["name"], "open_time": f"{self._open_time:.2f}", diff --git a/pyproject.toml b/pyproject.toml index 25799c9..d9f8b3a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,8 +29,8 @@ test = [ ] lint = [ + "codespell~=2.4.1", "mypy~=1.18.2", - "pre-commit~=4.5.0", "ruff~=0.14.1", "types-PyYAML~=6.0.12.20250915", ] @@ -71,4 +71,12 @@ warn_redundant_casts = true warn_unused_configs = true [tool.ruff.lint.pycodestyle] -max-line-length = 80 \ No newline at end of file +max-line-length = 80 + +[tool.codespell] +# Migrated from the deleted hook-config YAML so the manual gate +# (`uv run codespell ...`) stays green. Domain terms that look like typos +# but are intentional (HA APIs, abbreviations, German tech terms). +ignore-words-list = "hass,alot,datas,dof,dur,farenheit,hist,iff,ines,ist,lightsensor,mut,nd,pres,referer,ser,serie,te,technik,ue,uint,visability,wan,wanna,withing" +skip = "./.*,*.csv,*.json" +quiet-level = 2 diff --git a/tests/test_api_messages.py b/tests/test_api_messages.py index a254c87..472892a 100644 --- a/tests/test_api_messages.py +++ b/tests/test_api_messages.py @@ -266,9 +266,9 @@ async def test_handle_message_unknown_device_warning_default( api._handle_message("ss99UNKNOWNZZZZ01PP00") # Must have at least one WARNING for the unknown device - assert any( - r.levelno == logging.WARNING for r in caplog.records - ), "Expected WARNING log for unknown device when ignore_unknown is False" + assert any(r.levelno == logging.WARNING for r in caplog.records), ( + "Expected WARNING log for unknown device when ignore_unknown is False" + ) @pytest.mark.asyncio @@ -289,9 +289,9 @@ async def test_handle_message_unknown_device_debug_when_ignored( for r in caplog.records if "UNKNOWN" in r.message or "unknown" in r.message.lower() ), "Expected DEBUG log for ignored unknown device" - assert not any( - r.levelno == logging.WARNING for r in caplog.records - ), "Expected no WARNING when ignore_unknown is True" + assert not any(r.levelno == logging.WARNING for r in caplog.records), ( + "Expected no WARNING when ignore_unknown is True" + ) @pytest.mark.asyncio diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index f8b7126..70c3a70 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -204,7 +204,7 @@ async def test_manual_add_invalid_enum( assert result["type"] == "form", ( f"Expected form for input {bad_input!r}, got {result['type']!r}" ) - assert result["errors"].get("device_enum") == "invalid_enum_format", ( + assert (result["errors"] or {}).get("device_enum") == "invalid_enum_format", ( f"Expected invalid_enum_format for input {bad_input!r}, " f"got {result['errors']!r}" ) @@ -230,7 +230,7 @@ async def test_manual_add_duplicate_enum( } ) assert result["type"] == "form" - assert result["errors"].get("device_enum") == "duplicate_enum" + assert (result["errors"] or {}).get("device_enum") == "duplicate_enum" @pytest.mark.asyncio @@ -413,7 +413,7 @@ async def test_manual_add_enum_case_normalized( } ) assert result["type"] == "form" - assert result["errors"].get("device_enum") == "duplicate_enum", ( + assert (result["errors"] or {}).get("device_enum") == "duplicate_enum", ( "Expected duplicate_enum: '1a'.upper() == '1A' which already exists" ) diff --git a/tests/test_cover.py b/tests/test_cover.py index 67f7fcb..6b7a308 100644 --- a/tests/test_cover.py +++ b/tests/test_cover.py @@ -722,9 +722,7 @@ async def test_cover_initial_position_from_subentry( cover.hass = hass with patch.object(cover, "async_get_last_state", return_value=None): - with patch( - "custom_components.schellenberg_usb.cover.async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover, "async_write_ha_state"): await cover.async_added_to_hass() @@ -749,9 +747,7 @@ async def test_cover_initial_position_clamped( cover_clamped.hass = hass with patch.object(cover_clamped, "async_get_last_state", return_value=None): - with patch( - "custom_components.schellenberg_usb.cover.async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover_clamped, "async_write_ha_state"): await cover_clamped.async_added_to_hass() @@ -772,9 +768,7 @@ async def test_cover_initial_position_clamped( last_state = State("cover.restored_cover", "open", {"current_position": 50}) with patch.object(cover_restored, "async_get_last_state", return_value=last_state): - with patch( - "custom_components.schellenberg_usb.cover.async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover_restored, "async_write_ha_state"): await cover_restored.async_added_to_hass() @@ -854,7 +848,9 @@ async def test_timed_motor_position_loop_clears_flags( f"Expected position=50 after reaching target, got {cover._attr_current_cover_position}" ) # Task should be done - assert loop_task.done(), "Expected position loop task to be done after target reached" + assert loop_task.done(), ( + "Expected position loop task to be done after target reached" + ) # --------------------------------------------------------------------------- @@ -969,9 +965,7 @@ async def test_timed_stop_freezes_at_estimate( assert cover._attr_is_closing is False frozen_pos = cover._attr_current_cover_position assert frozen_pos is not None - assert 0 < frozen_pos < 100, ( - f"Expected mid estimate, got {frozen_pos}" - ) + assert 0 < frozen_pos < 100, f"Expected mid estimate, got {frozen_pos}" _async_mock(mock_api.control_blind).assert_awaited_once_with( cover._device_enum, CMD_STOP ) @@ -1303,9 +1297,7 @@ async def test_timed_set_position_noop_when_uncalibrated( cover.hass = hass cover._attr_current_cover_position = 50 - with patch.object( - cover, "async_open_cover", new_callable=AsyncMock - ) as mock_open: + with patch.object(cover, "async_open_cover", new_callable=AsyncMock) as mock_open: with patch.object( cover, "async_close_cover", new_callable=AsyncMock ) as mock_close: @@ -1341,14 +1333,9 @@ async def test_timed_restart_opening_snaps_to_100( ) cover.hass = hass - last_state = State( - "cover.timed_motor", "opening", {"current_position": 60} - ) + last_state = State("cover.timed_motor", "opening", {"current_position": 60}) with patch.object(cover, "async_get_last_state", return_value=last_state): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover, "async_write_ha_state"): await cover.async_added_to_hass() @@ -1375,14 +1362,9 @@ async def test_timed_restart_closing_snaps_to_0( ) cover.hass = hass - last_state = State( - "cover.timed_motor", "closing", {"current_position": 40} - ) + last_state = State("cover.timed_motor", "closing", {"current_position": 40}) with patch.object(cover, "async_get_last_state", return_value=last_state): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover, "async_write_ha_state"): await cover.async_added_to_hass() @@ -1574,9 +1556,7 @@ async def test_set_position_unlocked_after_calibration_drives_to_midpoint( cmd_up_calls = [c for c in calls if c.args[1] == CMD_UP] cmd_stop_calls = [c for c in calls if c.args[1] == CMD_STOP] - assert len(cmd_up_calls) >= 1, ( - f"Expected CMD_UP call; got calls: {calls}" - ) + assert len(cmd_up_calls) >= 1, f"Expected CMD_UP call; got calls: {calls}" assert len(cmd_stop_calls) >= 1, ( f"Expected CMD_STOP at midpoint; got calls: {calls}" ) @@ -1584,8 +1564,7 @@ async def test_set_position_unlocked_after_calibration_drives_to_midpoint( f"Expected position 50, got {cover._attr_current_cover_position}" ) assert cover._target_position is None, ( - f"Expected _target_position=None after completion, " - f"got {cover._target_position}" + f"Expected _target_position=None after completion, got {cover._target_position}" ) @@ -1665,16 +1644,9 @@ async def test_timed_restart_idle_restores_position( ) cover_zero.hass = hass - last_state_zero = State( - "cover.timed_motor", "closed", {"current_position": 0} - ) - with patch.object( - cover_zero, "async_get_last_state", return_value=last_state_zero - ): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + last_state_zero = State("cover.timed_motor", "closed", {"current_position": 0}) + with patch.object(cover_zero, "async_get_last_state", return_value=last_state_zero): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover_zero, "async_write_ha_state"): await cover_zero.async_added_to_hass() @@ -1694,18 +1666,13 @@ async def test_timed_restart_idle_restores_position( ) cover_thirty.hass = hass - last_state_thirty = State( - "cover.timed_motor_30", "open", {"current_position": 30} - ) + last_state_thirty = State("cover.timed_motor_30", "open", {"current_position": 30}) with patch.object( cover_thirty, "async_get_last_state", return_value=last_state_thirty, ): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover_thirty, "async_write_ha_state"): await cover_thirty.async_added_to_hass() @@ -1733,10 +1700,7 @@ async def test_timed_restart_no_prior_state_uses_initial_position( cover.hass = hass with patch.object(cover, "async_get_last_state", return_value=None): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover, "async_write_ha_state"): await cover.async_added_to_hass() @@ -1765,10 +1729,7 @@ async def test_timed_restart_no_prior_no_initial_defaults_to_100( cover.hass = hass with patch.object(cover, "async_get_last_state", return_value=None): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover, "async_write_ha_state"): await cover.async_added_to_hass() @@ -1861,9 +1822,7 @@ def _record_stop() -> None: stop_order.append("stop_tracking") cover_stop._attr_current_cover_position = 50 # mock freeze - with patch.object( - cover_stop, "_stop_position_tracking", side_effect=_record_stop - ): + with patch.object(cover_stop, "_stop_position_tracking", side_effect=_record_stop): with patch.object(cover_stop, "_update_position"): with patch.object(cover_stop, "async_write_ha_state"): await cover_stop.async_stop_cover() @@ -1905,9 +1864,7 @@ async def test_bidir_set_position_not_gated( assert cover._is_bidirectional is True assert cover._is_calibrated is False - with patch.object( - cover, "async_open_cover", new_callable=AsyncMock - ) as mock_open: + with patch.object(cover, "async_open_cover", new_callable=AsyncMock) as mock_open: await cover.async_set_cover_position(**{ATTR_POSITION: 80}) # Gate must NOT have fired — open was called @@ -1935,14 +1892,9 @@ async def test_bidir_restore_not_snapped_to_endstop( ) cover.hass = hass - last_state = State( - "cover.bidir_cover", "opening", {"current_position": 60} - ) + last_state = State("cover.bidir_cover", "opening", {"current_position": 60}) with patch.object(cover, "async_get_last_state", return_value=last_state): - with patch( - "custom_components.schellenberg_usb.cover" - ".async_dispatcher_connect" - ): + with patch("custom_components.schellenberg_usb.cover.async_dispatcher_connect"): with patch.object(cover, "async_write_ha_state"): await cover.async_added_to_hass() @@ -2132,9 +2084,7 @@ def _discard_task(coro: Any) -> MagicMock: return MagicMock() with patch.object(live_cover, "async_write_ha_state"): - with patch.object( - hass, "async_create_task", side_effect=_discard_task - ): + with patch.object(hass, "async_create_task", side_effect=_discard_task): await handler._emit_calibration_signal() # The live instance was updated by the dispatched signal. diff --git a/tests/test_options_flow.py b/tests/test_options_flow.py index 99f379e..cfb099e 100644 --- a/tests/test_options_flow.py +++ b/tests/test_options_flow.py @@ -63,9 +63,7 @@ async def test_toggle_saves_to_options( # the property reads `self.handler`. handler.handler = mock_hub_entry.entry_id - with patch.object( - hass.config_entries, "async_schedule_reload" - ) as mock_reload: + with patch.object(hass.config_entries, "async_schedule_reload") as mock_reload: result = await handler.async_step_init( user_input={ CONF_SERIAL_PORT: "/dev/ttyUSB0", # unchanged @@ -109,9 +107,9 @@ async def test_options_form_shows_toggle_default_off( assert result["type"] == "form" schema = result["data_schema"] + assert schema is not None schema_keys = { - (k.schema if hasattr(k, "schema") else k): v - for k, v in schema.schema.items() + (k.schema if hasattr(k, "schema") else k): v for k, v in schema.schema.items() } assert "ignore_unknown" in schema_keys, ( "Expected ignore_unknown field in the options form schema" diff --git a/tests/test_quality_gate_tooling.py b/tests/test_quality_gate_tooling.py new file mode 100644 index 0000000..80ab49e --- /dev/null +++ b/tests/test_quality_gate_tooling.py @@ -0,0 +1,43 @@ +"""Regression guards for the Phase 5 quality-gate tooling invariants. + +These tests lock in the outcomes of Phase 5 (TOOL-01, TOOL-02): the unused +pre-commit config stays removed, codespell stays configured in pyproject.toml, +and CONTRIBUTING.md keeps documenting the manual gate. They protect the +"single documented quality path" from silent regression in later phases. +""" + +from __future__ import annotations + +from pathlib import Path + +import tomllib + +REPO_ROOT = Path(__file__).resolve().parent.parent + + +def test_pre_commit_config_removed() -> None: + """TOOL-01: .pre-commit-config.yaml must not be reintroduced.""" + assert not (REPO_ROOT / ".pre-commit-config.yaml").exists() + + +def test_pre_commit_dependency_removed() -> None: + """TOOL-01: the orphaned pre-commit lint dependency stays gone.""" + pyproject = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text()) + lint = pyproject["dependency-groups"]["lint"] + assert not any(dep.startswith("pre-commit") for dep in lint) + + +def test_codespell_configured() -> None: + """TOOL-02: codespell config lives in pyproject [tool.codespell].""" + pyproject = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text()) + codespell = pyproject["tool"]["codespell"] + assert codespell["ignore-words-list"] + lint = pyproject["dependency-groups"]["lint"] + assert any(dep.startswith("codespell") for dep in lint) + + +def test_contributing_documents_full_gate() -> None: + """TOOL-01/TOOL-02: CONTRIBUTING.md documents all four gate tools.""" + text = (REPO_ROOT / "CONTRIBUTING.md").read_text(encoding="utf-8") + for tool in ("pytest", "ruff", "mypy", "codespell"): + assert tool in text, f"CONTRIBUTING.md must document {tool}" diff --git a/tests/test_timed_cal_handler_structure.py b/tests/test_timed_cal_handler_structure.py index 418aaa0..e537d09 100644 --- a/tests/test_timed_cal_handler_structure.py +++ b/tests/test_timed_cal_handler_structure.py @@ -6,8 +6,6 @@ from __future__ import annotations -import pytest - def test_module_imports() -> None: """Handler module and class must be importable (D-01, CAL-01).""" @@ -48,9 +46,7 @@ def test_handler_methods_exist() -> None: "_emit_calibration_signal", ] for method in required: - assert hasattr(TimedCalibrationFlowHandler, method), ( - f"Missing method: {method}" - ) + assert hasattr(TimedCalibrationFlowHandler, method), f"Missing method: {method}" def test_no_cmd_stop_in_module() -> None: @@ -60,11 +56,6 @@ def test_no_cmd_stop_in_module() -> None: as a call argument. Docstring references to CMD_STOP are exempt — the important invariant is that no executable code references the constant. """ - from custom_components.schellenberg_usb.const import CMD_STOP - from custom_components.schellenberg_usb.options_flow_timed_calibration import ( - TimedCalibrationFlowHandler, - ) - # CMD_STOP must not be imported into the timed calibration module namespace. import custom_components.schellenberg_usb.options_flow_timed_calibration as m diff --git a/tests/test_timed_calibration_flow.py b/tests/test_timed_calibration_flow.py index 79c9c5a..f560c5e 100644 --- a/tests/test_timed_calibration_flow.py +++ b/tests/test_timed_calibration_flow.py @@ -76,9 +76,7 @@ def _make_timed_handler( "type": "form", "step_id": kwargs.get("step_id"), "errors": kwargs.get("errors", {}), - "description_placeholders": kwargs.get( - "description_placeholders", {} - ), + "description_placeholders": kwargs.get("description_placeholders", {}), } ) mock_flow.async_abort = MagicMock( @@ -147,9 +145,7 @@ async def test_timed_cal_precondition_shows_form( ) as mock_step: result = await handler.async_step_reconfigure(None) - assert result["type"] == "form", ( - f"Expected form, got {result['type']!r}" - ) + assert result["type"] == "form", f"Expected form, got {result['type']!r}" assert result["step_id"] == "timed_cal_precondition", ( f"Expected timed_cal_precondition, got {result.get('step_id')!r}" ) @@ -193,7 +189,7 @@ async def test_timed_cal_close_too_short( assert result["type"] == "form" assert result["step_id"] == "timed_cal_close" - assert result["errors"].get("base") == "timed_cal_too_short" + assert (result["errors"] or {}).get("base") == "timed_cal_too_short" # Start time must be reset so the next visit restarts the timer assert handler._close_start_time is None @@ -215,7 +211,7 @@ async def test_timed_cal_close_too_long( assert result["type"] == "form" assert result["step_id"] == "timed_cal_close" - assert result["errors"].get("base") == "timed_cal_too_long" + assert (result["errors"] or {}).get("base") == "timed_cal_too_long" assert handler._close_start_time is None @@ -245,7 +241,7 @@ async def test_timed_cal_guard_reshow_does_not_redrive( # Guard submit — must NOT re-send CMD_DOWN result = await handler.async_step_timed_cal_close(user_input={}) - assert result["errors"].get("base") == "timed_cal_too_short" + assert (result["errors"] or {}).get("base") == "timed_cal_too_short" assert api.control_blind.await_count == count_after_drive, ( "control_blind must NOT be awaited again on guard re-show (REVIEW-3)" ) @@ -305,7 +301,6 @@ async def test_timed_cal_happy_path_reaches_confirm( Pins: D-04 (close-then-open order), D-10 (confirm before save). """ handler = _make_timed_handler(hass, mock_hub_entry) - api = mock_hub_entry.runtime_data # Drive close step await handler.async_step_timed_cal_close(user_input=None) @@ -317,13 +312,13 @@ async def test_timed_cal_happy_path_reaches_confirm( async def capture_open( user_input: dict | None = None, - ) -> dict: + ) -> dict: # type: ignore[return] """Capture the open step result.""" result = await TimedCalibrationFlowHandler.async_step_timed_cal_open( handler, user_input ) captured["open_result"] = result - return result + return result # type: ignore[return-value] with patch.object(handler, "async_step_timed_cal_open", side_effect=capture_open): # Submit close end-press (advances to open step via internal call) @@ -336,16 +331,14 @@ async def capture_open( async def intercept_confirm( user_input: dict | None = None, - ) -> dict: + ) -> dict: # type: ignore[return] """Call confirm with user_input=None (show screen).""" - return await original_confirm(user_input=None) + return await original_confirm(user_input=None) # type: ignore[return-value] with patch.object( handler, "async_step_timed_cal_confirm", side_effect=intercept_confirm ): - result_close_submit = await handler.async_step_timed_cal_close( - user_input={} - ) + await handler.async_step_timed_cal_close(user_input={}) # After valid close, open step is called internally, then confirm is shown # The result from async_step_timed_cal_close (after guard passes) is the @@ -354,7 +347,6 @@ async def intercept_confirm( # Reset and do it manually handler2 = _make_timed_handler(hass, mock_hub_entry) - api2 = mock_hub_entry.runtime_data # Step 1: close first visit (sends CMD_DOWN, shows form) await handler2.async_step_timed_cal_close(user_input=None) @@ -371,8 +363,9 @@ async def intercept_confirm( result_confirm_form = await handler2.async_step_timed_cal_open(user_input={}) assert result_confirm_form["type"] == "form" assert result_confirm_form["step_id"] == "timed_cal_confirm" - assert "close_time" in result_confirm_form["description_placeholders"] - assert "open_time" in result_confirm_form["description_placeholders"] + placeholders = result_confirm_form["description_placeholders"] or {} + assert "close_time" in placeholders + assert "open_time" in placeholders @pytest.mark.asyncio @@ -391,23 +384,17 @@ async def test_timed_cal_confirm_emits_signal_with_100( "custom_components.schellenberg_usb" ".options_flow_timed_calibration.async_dispatcher_send" ) as mock_send: - result = await handler.async_step_timed_cal_confirm( - user_input={"redo": False} - ) + result = await handler.async_step_timed_cal_confirm(user_input={"redo": False}) assert result["type"] == "abort" assert result["reason"] == "reconfigure_successful" mock_send.assert_called_once() call_args = mock_send.call_args[0] - assert call_args[1] == SIGNAL_CALIBRATION_COMPLETED, ( - "Signal name mismatch" - ) + assert call_args[1] == SIGNAL_CALIBRATION_COMPLETED, "Signal name mismatch" assert call_args[2] == "DEV1A" # device_id - assert call_args[3] == 20.3 # open_time - assert call_args[4] == 18.5 # close_time - assert call_args[5] == 100, ( - "final_position must be 100 for timed flow (D-14)" - ) + assert call_args[3] == 20.3 # open_time + assert call_args[4] == 18.5 # close_time + assert call_args[5] == 100, "final_position must be 100 for timed flow (D-14)" @pytest.mark.asyncio @@ -428,9 +415,7 @@ async def test_timed_cal_redo_returns_to_precondition( "custom_components.schellenberg_usb" ".options_flow_timed_calibration.async_dispatcher_send" ) as mock_send: - result = await handler.async_step_timed_cal_confirm( - user_input={"redo": True} - ) + result = await handler.async_step_timed_cal_confirm(user_input={"redo": True}) # No signal emitted on redo mock_send.assert_not_called() diff --git a/uv.lock b/uv.lock index a5e2fdc..479499f 100644 --- a/uv.lock +++ b/uv.lock @@ -628,15 +628,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/ae/3a/dbeec9d1ee0844c679f6bb5d6ad4e9f198b1224f4e7a32825f47f6192b0c/cffi-2.0.0-cp314-cp314t-win_arm64.whl", hash = "sha256:0a1527a803f0a659de1af2e1fd700213caba79377e27e4693648c2923da066f9", size = 184195, upload-time = "2025-09-08T23:23:43.004Z" }, ] -[[package]] -name = "cfgv" -version = "3.4.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/11/74/539e56497d9bd1d484fd863dd69cbbfa653cd2aa27abfe35653494d85e94/cfgv-3.4.0.tar.gz", hash = "sha256:e52591d4c5f5dead8e0f673fb16db7949d2cfb3f7da4582893288f0ded8fe560", size = 7114, upload-time = "2023-08-12T20:38:17.776Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/c5/55/51844dd50c4fc7a33b653bfaba4c2456f06955289ca770a5dbd5fd267374/cfgv-3.4.0-py2.py3-none-any.whl", hash = "sha256:b7265b1f29fd3316bfcd2b330d63d024f2bfd8bcb8b0272f8e19a504856c48f9", size = 7249, upload-time = "2023-08-12T20:38:16.269Z" }, -] - [[package]] name = "charset-normalizer" version = "3.4.4" @@ -714,6 +705,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/98/78/01c019cdb5d6498122777c1a43056ebb3ebfeef2076d9d026bfe15583b2b/click-8.3.1-py3-none-any.whl", hash = "sha256:981153a64e25f12d547d3426c367a4857371575ee7ad18df2a6183ab0545b2a6", size = 108274, upload-time = "2025-11-15T20:45:41.139Z" }, ] +[[package]] +name = "codespell" +version = "2.4.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/2d/9d/1d0903dff693160f893ca6abcabad545088e7a2ee0a6deae7c24e958be69/codespell-2.4.2.tar.gz", hash = "sha256:3c33be9ae34543807f088aeb4832dfad8cb2dae38da61cac0a7045dd376cfdf3", size = 352058, upload-time = "2026-03-05T18:10:42.936Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/42/a1/52fa05533e95fe45bcc09bcf8a503874b1c08f221a4e35608017e0938f55/codespell-2.4.2-py3-none-any.whl", hash = "sha256:97e0c1060cf46bd1d5db89a936c98db8c2b804e1fdd4b5c645e82a1ec6b1f886", size = 353715, upload-time = "2026-03-05T18:10:41.398Z" }, +] + [[package]] name = "colorama" version = "0.4.6" @@ -874,15 +874,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cc/41/2d2baa32486a2f3f8248fbc855670caa861e66e017988c74a3f234664d3a/dbus_fast-2.44.5-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:2979c4b90596ddb72d2056de7cb47a2994143e805844732d96bcd882731b936b", size = 960019, upload-time = "2025-10-04T20:15:40.435Z" }, ] -[[package]] -name = "distlib" -version = "0.4.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/96/8e/709914eb2b5749865801041647dc7f4e6d00b549cfe88b65ca192995f07c/distlib-0.4.0.tar.gz", hash = "sha256:feec40075be03a04501a973d81f633735b4b69f98b05450592310c0f401a4e0d", size = 614605, upload-time = "2025-07-17T16:52:00.465Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/33/6b/e0547afaf41bf2c42e52430072fa5658766e3d65bd4b03a563d1b6336f57/distlib-0.4.0-py2.py3-none-any.whl", hash = "sha256:9659f7d87e46584a30b5780e43ac7a2143098441670ff0a49d5f9034c54a6c16", size = 469047, upload-time = "2025-07-17T16:51:58.613Z" }, -] - [[package]] name = "distro" version = "1.9.0" @@ -1409,15 +1400,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/af/cf/ef5cc94b1ed4e1ab8a15c17937c876b9733154a746c78f4c06c2336a05e5/huggingface_hub-1.2.1-py3-none-any.whl", hash = "sha256:8c74a41a16156337dfa1090873ca11f8c1d7b6efcbac9f6673d008a740207e6a", size = 520930, upload-time = "2025-12-05T15:11:20.045Z" }, ] -[[package]] -name = "identify" -version = "2.6.15" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/ff/e7/685de97986c916a6d93b3876139e00eef26ad5bbbd61925d670ae8013449/identify-2.6.15.tar.gz", hash = "sha256:e4f4864b96c6557ef2a1e1c951771838f4edc9df3a72ec7118b338801b11c7bf", size = 99311, upload-time = "2025-10-02T17:43:40.631Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/0f/1c/e5fd8f973d4f375adb21565739498e2e9a1e54c858a97b9a8ccfdc81da9b/identify-2.6.15-py2.py3-none-any.whl", hash = "sha256:1181ef7608e00704db228516541eb83a88a9f94433a8c80bb9b5bd54b1d81757", size = 99183, upload-time = "2025-10-02T17:43:39.137Z" }, -] - [[package]] name = "idna" version = "3.11" @@ -1836,15 +1818,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/79/7b/2c79738432f5c924bef5071f933bcc9efd0473bac3b4aa584a6f7c1c8df8/mypy_extensions-1.1.0-py3-none-any.whl", hash = "sha256:1be4cccdb0f2482337c4743e60421de3a356cd97508abadd57d47403e94f5505", size = 4963, upload-time = "2025-04-22T14:54:22.983Z" }, ] -[[package]] -name = "nodeenv" -version = "1.9.1" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/43/16/fc88b08840de0e0a72a2f9d8c6bae36be573e475a6326ae854bcc549fc45/nodeenv-1.9.1.tar.gz", hash = "sha256:6ec12890a2dab7946721edbfbcd91f3319c6ccc9aec47be7c7e6b7011ee6645f", size = 47437, upload-time = "2024-06-04T18:44:11.171Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/d2/1d/1b658dbd2b9fa9c4c9f32accbfc0205d532c8c6194dc0f2a4c0428e7128a/nodeenv-1.9.1-py2.py3-none-any.whl", hash = "sha256:ba11c9782d29c27c70ffbdda2d7415098754709be8a7056d79a737cd901155c9", size = 22314, upload-time = "2024-06-04T18:44:08.352Z" }, -] - [[package]] name = "numpy" version = "2.3.2" @@ -2057,15 +2030,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/b9/a5/f9f143b420e53a296869636d1c3bdc144be498ca3136a113f52b53ea2b02/pipdeptree-2.26.1-py3-none-any.whl", hash = "sha256:3849d62a2ed641256afac3058c4f9b85ac4a47e9d8c991ee17a8f3d230c5cffb", size = 32802, upload-time = "2025-04-20T03:27:40.413Z" }, ] -[[package]] -name = "platformdirs" -version = "4.5.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/61/33/9611380c2bdb1225fdef633e2a9610622310fed35ab11dac9620972ee088/platformdirs-4.5.0.tar.gz", hash = "sha256:70ddccdd7c99fc5942e9fc25636a8b34d04c24b335100223152c2803e4063312", size = 21632, upload-time = "2025-10-08T17:44:48.791Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/73/cb/ac7874b3e5d58441674fb70742e6c374b28b0c7cb988d37d991cde47166c/platformdirs-4.5.0-py3-none-any.whl", hash = "sha256:e578a81bb873cbb89a41fcc904c7ef523cc18284b7e3b3ccf06aca1403b7ebd3", size = 18651, upload-time = "2025-10-08T17:44:47.223Z" }, -] - [[package]] name = "pluggy" version = "1.6.0" @@ -2088,22 +2052,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/94/7c/535646d75a1c510065169ea65693613c7a6bc64491bea13e7dad4f028ff3/polyfactory-3.1.0-py3-none-any.whl", hash = "sha256:78171232342c25906d542513c9f00ebf41eadec2c67b498490a577024dd7e867", size = 61836, upload-time = "2025-11-25T08:10:14.893Z" }, ] -[[package]] -name = "pre-commit" -version = "4.5.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "cfgv" }, - { name = "identify" }, - { name = "nodeenv" }, - { name = "pyyaml" }, - { name = "virtualenv" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/f4/9b/6a4ffb4ed980519da959e1cf3122fc6cb41211daa58dbae1c73c0e519a37/pre_commit-4.5.0.tar.gz", hash = "sha256:dc5a065e932b19fc1d4c653c6939068fe54325af8e741e74e88db4d28a4dd66b", size = 198428, upload-time = "2025-11-22T21:02:42.304Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/5d/c4/b2d28e9d2edf4f1713eb3c29307f1a63f3d67cf09bdda29715a36a68921a/pre_commit-4.5.0-py2.py3-none-any.whl", hash = "sha256:25e2ce09595174d9c97860a95609f9f852c0614ba602de3561e267547f2335e1", size = 226429, upload-time = "2025-11-22T21:02:40.836Z" }, -] - [[package]] name = "propcache" version = "0.4.1" @@ -2941,12 +2889,12 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "codespell" }, { name = "croniter" }, { name = "freezegun" }, { name = "mock-open" }, { name = "mypy" }, { name = "polyfactory" }, - { name = "pre-commit" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, @@ -2957,8 +2905,8 @@ dev = [ { name = "watchdog" }, ] lint = [ + { name = "codespell" }, { name = "mypy" }, - { name = "pre-commit" }, { name = "ruff" }, { name = "types-pyyaml" }, ] @@ -2980,12 +2928,12 @@ requires-dist = [{ name = "pyserial-asyncio-fast", specifier = "==0.16" }] [package.metadata.requires-dev] dev = [ + { name = "codespell", specifier = "~=2.4.1" }, { name = "croniter", specifier = "~=6.0.0" }, { name = "freezegun" }, { name = "mock-open", specifier = "~=1.4.0" }, { name = "mypy", specifier = "~=1.18.2" }, { name = "polyfactory", specifier = "~=3.1.0" }, - { name = "pre-commit", specifier = "~=4.5.0" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, @@ -2996,8 +2944,8 @@ dev = [ { name = "watchdog", specifier = "~=6.0.0" }, ] lint = [ + { name = "codespell", specifier = "~=2.4.1" }, { name = "mypy", specifier = "~=1.18.2" }, - { name = "pre-commit", specifier = "~=4.5.0" }, { name = "ruff", specifier = "~=0.14.1" }, { name = "types-pyyaml", specifier = "~=6.0.12.20250915" }, ] @@ -3381,20 +3329,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/92/05/8b4632c4c793b58bfed5ec8353c3f25818c025a6393ceb91f23a91094408/uv-0.9.6-py3-none-win_arm64.whl", hash = "sha256:166175ba952d2ad727e1dbd57d7cfc1782dfe7b8d79972174a46a7aa33ddceec", size = 19919992, upload-time = "2025-10-29T19:40:44.187Z" }, ] -[[package]] -name = "virtualenv" -version = "20.35.3" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "distlib" }, - { name = "filelock" }, - { name = "platformdirs" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/a4/d5/b0ccd381d55c8f45d46f77df6ae59fbc23d19e901e2d523395598e5f4c93/virtualenv-20.35.3.tar.gz", hash = "sha256:4f1a845d131133bdff10590489610c98c168ff99dc75d6c96853801f7f67af44", size = 6002907, upload-time = "2025-10-10T21:23:33.178Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/27/73/d9a94da0e9d470a543c1b9d3ccbceb0f59455983088e727b8a1824ed90fb/virtualenv-20.35.3-py3-none-any.whl", hash = "sha256:63d106565078d8c8d0b206d48080f938a8b25361e19432d2c9db40d2899c810a", size = 5981061, upload-time = "2025-10-10T21:23:30.433Z" }, -] - [[package]] name = "voluptuous" version = "0.15.2"