diff --git a/.gitattributes b/.gitattributes index c6b7e4d..e21ea57 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,5 +1,5 @@ # Every text file is stored and checked out with LF. This is the native # means for the problem a hand-written guard used to watch: a generator run # on Windows would otherwise rewrite whole files to CRLF and bury the real -# change in the diff (audit VA-11 / ledger L-006). +# change in the diff. * text=auto eol=lf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2baf8cc..4303f2d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,7 @@ permissions: jobs: check: runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v7 - uses: actions/setup-python@v7 @@ -39,6 +40,7 @@ jobs: # install. minimum-home-assistant: runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v7 - uses: actions/setup-python@v7 @@ -54,12 +56,14 @@ jobs: hassfest: runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v7 - uses: home-assistant/actions/hassfest@master hacs: runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v7 - uses: hacs/action@main diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 687f25a..45a6471 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,13 +10,14 @@ permissions: jobs: # The tag alone would publish whatever check.sh --release passes; hassfest, - # HACS and the minimum-HA run only ever saw the branch (audit A13-02). + # HACS and the minimum-HA run only ever saw the branch. compatibility: uses: ./.github/workflows/ci.yml release: needs: compatibility runs-on: ubuntu-latest + timeout-minutes: 120 permissions: contents: write steps: @@ -24,20 +25,24 @@ jobs: with: fetch-depth: 0 # the history main has, for the check below # Tags come from main only. One pushed from a branch before its merge - # published code main does not contain, to every HACS user (A18-31). + # published code main does not contain, to every HACS user. - name: Release only what main contains run: git merge-base --is-ancestor "$GITHUB_SHA" origin/main - uses: actions/setup-python@v7 with: python-version: "3.14" - - name: Install dependencies - run: pip install --group dev - - name: Full gates including mutation run - run: bash scripts/check.sh --release + # Before the long gate: a wrong tag or a missing changelog section + # fails in seconds instead of after the mutation run. - name: Verify tag matches manifest version run: | VERSION=$(python3 -c "import json;print(json.load(open('custom_components/wattpilot/manifest.json'))['version'])") [ "v$VERSION" = "$GITHUB_REF_NAME" ] || { echo "tag $GITHUB_REF_NAME != manifest v$VERSION"; exit 1; } + - name: Take the release notes from the changelog + run: python3 scripts/release_notes.py "${GITHUB_REF_NAME#v}" >release-notes.md + - name: Install dependencies + run: pip install --group dev + - name: Full gates including mutation run + run: bash scripts/check.sh --release - name: Say whether this tag is a pre-release id: kind # Anything that is not exactly vX.Y.Z is a candidate (v0.1.2b1), and @@ -50,8 +55,6 @@ jobs: echo "prerelease=true" >>"$GITHUB_OUTPUT" fi - - name: Take the release notes from the changelog - run: python3 scripts/release_notes.py "${GITHUB_REF_NAME#v}" >release-notes.md - name: Create GitHub release uses: softprops/action-gh-release@v3 env: diff --git a/CHANGELOG.md b/CHANGELOG.md index ccd0a1e..3ac57ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,20 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed + +- Setting up or reloading the charger is a little faster: the integration + reads its description of the charger's settings once per Home Assistant + start instead of every time. +- The log is quieter. A connection, a refused password and a reconnect + attempt that stalls are logged at debug level only; what you need to act + on still reaches you as a repair notice, a password request or a setup + error. + A message from the charger that keeps failing is warned about once per + connection, with the line it failed at. +- A charger that accepts the connection but never completes it is retried + ever more slowly, as one that does not answer at all already was. + ### Fixed - A Wattpilot Home on firmware 43.5 no longer reports its ID-chip list but @@ -43,6 +57,18 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The diagnostics download also removes a Wi-Fi key sent as text, the Wi-Fi network name in `wss`, the load balancing members and the last ID chip read. +- An entry from the earlier cloud integration, made local with Reconfigure, + now checks that the charger at its address is still its own. It never + did, so another Wattpilot with the same password at that address would + have taken over its entities and history. +- Reauthentication and Reconfigure name a different charger at the address + as such. When its password differed too, they reported a wrong password. +- Installing a firmware version the charger does not offer, for example + through a typo in an action, is refused with an error. It was sent to the + charger as is. +- Unloading or reloading no longer fails when closing the connection + fails. The entry then refused every reload until Home Assistant + restarted. ## [0.3.0] - 2026-10-21 diff --git a/NOTICE b/NOTICE index f7912bf..8984db8 100644 --- a/NOTICE +++ b/NOTICE @@ -10,8 +10,8 @@ Protocol knowledge and property semantics build on prior work: which carries its MIT licence text, and is maintained here since. Only the modules this integration uses came along (client, auth, definition, models, exceptions, utils and the API definition resource); mqtt, shell and - discovery did not. Changes against 1.4.0 are marked in the source with a - `wattpilot:` comment. + discovery did not. It has been changed throughout since: utils was dropped, + and the connection handling moved out of the client into connection.py. - ruaan-deysel/ha-wattpilot (Apache-2.0) — the integration this project replaces. No source code was copied from the Apache-2.0 project; only interoperability facts (property keys, enum values, unique-id suffixes) diff --git a/custom_components/wattpilot/__init__.py b/custom_components/wattpilot/__init__.py index 9b17989..ffcc498 100644 --- a/custom_components/wattpilot/__init__.py +++ b/custom_components/wattpilot/__init__.py @@ -10,7 +10,7 @@ from homeassistant import config_entries from homeassistant.const import CONF_NAME, EVENT_HOMEASSISTANT_STOP from homeassistant.core import callback -from homeassistant.exceptions import ConfigEntryError, HomeAssistantError +from homeassistant.exceptions import ConfigEntryError from homeassistant.helpers import config_validation as cv from homeassistant.helpers.dispatcher import async_dispatcher_connect @@ -86,7 +86,7 @@ def _replacements(self) -> list[tuple[str, str]]: def _serial_masks(entries: list[ConfigEntry]) -> list[tuple[str, str]]: # A serial is all digits; an entry awaiting it is keyed on a name or - # address, which got masked in every other title as well (R1-17). + # address, which got masked in every other title as well. return [ (entry.unique_id, mask_serial(entry.unique_id)) for entry in entries @@ -95,8 +95,8 @@ def _serial_masks(entries: list[ConfigEntry]) -> list[tuple[str, str]]: def _address_title_masks(entries: list[ConfigEntry]) -> list[tuple[str, str]]: - # A fork entry titled by its address put it into every setup message - # (RF-P-02). An address literal, unlike a name, means nothing else. + # A fork entry titled by its address put it into every setup message. + # An address literal, unlike a name, means nothing else. return [(entry.title, "
") for entry in entries if _is_address(entry.title)] @@ -112,7 +112,7 @@ async def async_setup(hass: HomeAssistant, _config: ConfigType) -> bool: """Register integration services and mask serials in setup messages.""" async_setup_services(hass) # Never removed: the setups the stop cancels log their title after the - # stop event, and lost the mask with it (R1-05). + # stop event, and lost the mask with it. serial_mask = _SerialMask(hass) for logger in _ENTRY_LOGGERS: logger.addFilter(serial_mask) @@ -149,23 +149,18 @@ async def async_setup_entry(hass: HomeAssistant, entry: WattpilotConfigEntry) -> # Home Assistant calls async_unload_entry only for an entry that finished # setup, so every failure below has to hand them back itself -- otherwise # the entry sits in SETUP_ERROR with the connection still open, and the - # next retry puts a second hub on the same dispatcher signals (audit - # VA-02, measured through the config entry manager). + # next retry puts a second hub on the same dispatcher signals (measured + # through the config entry manager). # # BaseException, not Exception: Home Assistant cancels a setup that runs # too long, and CancelledError is not an Exception -- the narrower catch - # handed nothing back on that path (audit A11-05). + # handed nothing back on that path. try: await _finish_setup(hass, entry, hub) except BaseException: # A failed disconnect must not replace the error that ended setup -- - # that one says what the user has to fix (audit A14-03). - try: - await hub.async_shutdown() - except HomeAssistantError: - _LOGGER.debug( - "Ignoring error while closing after failed setup", exc_info=True - ) + # that one says what the user has to fix; async_shutdown keeps it out. + await hub.async_shutdown() raise return True @@ -180,7 +175,7 @@ def _reload_when_firmware_changes( their description tables at setup, and an entity freezes the device's sw_version when it is built. So a firmware jump leaves the newly supported entities missing and the device page showing the old version - until someone reloads by hand (audit VA-05). A reload redoes both. + until someone reloads by hand. A reload redoes both. """ known = hub.firmware @@ -209,7 +204,9 @@ async def _finish_setup( Split out so its caller can put a single cleanup guard around all of it. """ serial = hub.serial - if entry.unique_id != serial: + # Also when the serial already matches: confirming it is what ends the + # permission, or the client keeps being built without an identity check. + if entry.unique_id != serial or awaits_serial(entry): await _adopt_serial(hass, entry, serial) await async_migrate_legacy_unique_ids( hass, entry, serial, hub.variant, hub.properties @@ -283,7 +280,7 @@ async def _remove_ignored_card(hass: HomeAssistant, serial: str) -> None: Drop an ignored card for the charger this entry is about to own. It only kept the card from coming back, which the entry does now; left - in place it would share this entry's unique_id (audit A18-07). + in place it would share this entry's unique_id. """ for ignored in hass.config_entries.async_entries(DOMAIN): if ( @@ -299,7 +296,7 @@ def _close_open_cards(hass: HomeAssistant, serial: str) -> None: Close the discovery cards still offering the charger just adopted. One opened before the entry knew its serial stays up otherwise, and - ignoring it makes Home Assistant replace the entry (audit A18-01). + ignoring it makes Home Assistant replace the entry. """ flows = hass.config_entries.flow for card in flows.async_progress_by_handler( @@ -353,8 +350,8 @@ async def async_migrate_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: CONF_AWAITING_SERIAL: True, } # Without a name the device took the host name, and every entity the - # fork lacked got the serial in its ID (RF-P-04); a cloud entry too, - # once Reconfigure made it local (PC5-R-04). + # fork lacked got the serial in its ID; a cloud entry too, + # once Reconfigure made it local. new_data[CONF_NAME] = new_device_name( hass, preferred=old.get("friendly_name"), exclude=entry.entry_id ) diff --git a/custom_components/wattpilot/api/__init__.py b/custom_components/wattpilot/api/__init__.py index 684f4a7..3a73f75 100644 --- a/custom_components/wattpilot/api/__init__.py +++ b/custom_components/wattpilot/api/__init__.py @@ -8,8 +8,8 @@ integration uses were taken over; mqtt, shell and discovery were not, and with them neither aiomqtt, prompt-toolkit nor pydantic. -Changes against 1.4.0 carry a `wattpilot:` comment naming the finding, so -the adopted parts stay distinguishable from ours. +It has been changed throughout since, so the changes are no longer marked +one by one. """ from .client import Wattpilot, mask_serial diff --git a/custom_components/wattpilot/api/client.py b/custom_components/wattpilot/api/client.py index 192fc9c..dcf2292 100644 --- a/custom_components/wattpilot/api/client.py +++ b/custom_components/wattpilot/api/client.py @@ -4,7 +4,6 @@ import asyncio import contextlib -import inspect import json import logging from collections.abc import Callable @@ -42,8 +41,7 @@ _NEVER_ACKNOWLEDGED = frozenset({"rst"}) CLOUD_API_BASE_URL = "https://app.wattpilot.io/app" -type PropertyCallback = Callable[[str, Any], Any] -type MessageCallback = Callable[[dict[str, Any]], Any] +type PropertyCallback = Callable[[str, Any], None] def _plain_json(value: Any) -> Any: # noqa: ANN401 -- recurses over untyped JSON @@ -54,7 +52,7 @@ def _plain_json(value: Any) -> Any: # noqa: ANN401 -- recurses over untyped JSO SimpleNamespace per nested object, because the message handlers read fields by attribute. Everything downstream -- entities, diagnostics, the redaction that walks dictionaries and lists -- expects plain containers, - and a namespace slipped past all of it (audit A11-03). + and a namespace slipped past all of it. """ if isinstance(value, SimpleNamespace): return {key: _plain_json(item) for key, item in vars(value).items()} @@ -64,7 +62,7 @@ def _plain_json(value: Any) -> Any: # noqa: ANN401 -- recurses over untyped JSO def _status_object(msg: SimpleNamespace) -> SimpleNamespace: - """Return a status frame's status; anything else completed it unread (finding 6).""" + """Return a status frame's status; anything else completed it unread.""" if isinstance(status := getattr(msg, "status", None), SimpleNamespace): return status msg_text = "Status frame without a status object, skipped as unreadable" @@ -73,18 +71,18 @@ def _status_object(msg: SimpleNamespace) -> SimpleNamespace: def _correlation_key(request_id: object) -> str: """ - wattpilot: one spelling for a request id, whatever the wire used. + Return one spelling for a request id, whatever the wire used. Commands are sent with an integer; the protocol's own examples answer with a string. Comparing them raw left the caller waiting for an answer - that had already arrived (audit A11-01). + that had already arrived. """ return str(request_id) def mask_serial(serial: str | None) -> str: """ - wattpilot: a serial fit for the log, which users attach to public issues. + Return a serial fit for the log, which users attach to public issues. Only the last digits stay readable; a serial this short is masked whole. """ @@ -97,7 +95,7 @@ def mask_serial(serial: str | None) -> str: def _message_type(raw: str) -> str: - """wattpilot: the frame's type, for logging without its payload.""" + """Return the frame's type, for logging without its payload.""" try: parsed = json.loads(raw) except ValueError: @@ -148,6 +146,14 @@ def _as_float(value: Any, name: str) -> float: # noqa: ANN401 -- a value to wri raise PropertyError(msg) +def _as_text(value: Any, name: str) -> str: # noqa: ANN401 -- a value to write + """Return *value* if it is text already: str(5) is no text the caller meant.""" + if isinstance(value, str): + return value + msg = f"Cannot convert {value!r} to str for property '{name}'" + raise PropertyError(msg) + + def _parse_number(text: str) -> int | float | str: """Read a number from text, or give the text back unchanged.""" for parse in (int, float): @@ -205,9 +211,9 @@ def __init__( # noqa: PLR0913 -- every connection knob it has reconnect_delay_max=reconnect_delay_max, ) self._request_id = 0 - # Pending setValue commands, so a rejection reaches its caller (VA-03). + # Pending setValue commands, so a rejection reaches its caller. # Keyed by string: ids go out as int, but answers may carry them as - # strings, and an unnormalized lookup missed those (audit A11-01). + # strings, and an unnormalized lookup missed those. self._pending_commands: dict[str, asyncio.Future[None]] = {} self.command_timeout = 10.0 self._all_props: dict[str, Any] = {} @@ -221,7 +227,6 @@ def __init__( # noqa: PLR0913 -- every connection knob it has # Callbacks self._property_callbacks: list[PropertyCallback] = [] - self._message_callbacks: list[MessageCallback] = [] # ---- Context manager ---- @@ -240,20 +245,20 @@ async def connect(self) -> None: """Open the WebSocket and authenticate.""" await self._connection.open() # Same cleanup scope as the handshake: a cancellation between the two - # left reader and socket alive (audit A12-03). + # left reader and socket alive. try: await self._load_api_definition() except BaseException: await self._connection.close() raise - _LOGGER.info("Connected to Wattpilot %s", mask_serial(self._device.serial)) + _LOGGER.debug("Connected to Wattpilot %s", mask_serial(self._device.serial)) async def disconnect(self) -> None: """Close the WebSocket connection.""" await self._connection.close() def _fail_pending_commands(self, reason: str) -> None: - """wattpilot: never leave a caller awaiting an answer that cannot come.""" + """Never leave a caller awaiting an answer that cannot come.""" for request_id, future in list(self._pending_commands.items()): if not future.done(): future.set_exception( @@ -293,6 +298,10 @@ def firmware(self) -> str | None: """Return the charger's firmware version, if known.""" return self._firmware + def get_property(self, name: str, default: Any = None) -> Any: # noqa: ANN401 -- a charger value + """Return one property, without the copy all_properties makes.""" + return self._all_props.get(name, default) + @property def all_properties(self) -> dict[str, Any]: """Return a copy of every property seen so far.""" @@ -306,8 +315,8 @@ def properties_initialized(self) -> bool: # Device info @property - def variant(self) -> str | None: - """Device variant (e.g. ``'11kW'``, ``'22kW'``).""" + def variant(self) -> object: + """Maximum-current variant as the charger sends it: the integer 11 or 22.""" return self._all_props.get("var") @property @@ -343,7 +352,7 @@ async def set_property(self, name: str, value: Any) -> None: # noqa: ANN401 -- if name in _NEVER_ACKNOWLEDGED: await self._send(message, secure=secure) return - # Wait for the answer, not just the send (audit VA-03). + # Wait for the answer, not just the send. await self._send_command(message, secure=secure) async def set_next_trip( @@ -367,7 +376,7 @@ async def set_next_trip( # No DST hour. Measured 2026-09-06, summer time, `tds` = 1: 07:30 set # in the app reads back as 27000, not 30600; adding the hour made it - # come back as 08:30 (audit A12-06). + # come back as 08:30. await self.set_property("ftt", timestamp) async def set_next_trip_energy(self, energy_wh: float) -> None: @@ -440,12 +449,12 @@ async def install_firmware_update( version = versions[0] # Counted from here: a close in the wait for the reboot ended that - # wait as if the charger had gone, and the update reconnected (RF-L-01). + # wait as if the charger had gone, and the update reconnected. closes = self._connection.close_count await self.set_property("oct", version) # A monotonic deadline: a sum of sleeps never counted the reconnect - # waits and overran the two-minute budget by half an hour (A12-08). + # waits and overran the two-minute budget by half an hour. loop = asyncio.get_running_loop() deadline = loop.time() + timeout @@ -456,7 +465,7 @@ async def install_firmware_update( msg = "Charger did not disconnect for firmware update" raise WattpilotConnectionError(msg) - # No disconnect() of its own: it counted as an unload (PC-01). + # No disconnect() of its own: it counted as an unload. await self._reconnect_before(deadline, closes) async def _reconnect_before(self, deadline: float, closes: int) -> None: @@ -466,10 +475,10 @@ async def _reconnect_before(self, deadline: float, closes: int) -> None: Every attempt is bounded by what is left of the budget, and so is the pause between them. Both used to be free: the caller's timeout was a sum of sleeps, and a single attempt could sit in a full - authentication wait without spending any of it (audit A12-08). + authentication wait without spending any of it. """ loop = asyncio.get_running_loop() - # Another close (an unload) ends the wait; reconnecting outlived it (R1-11). + # Another close (an unload) ends the wait; reconnecting outlived it. while self._connection.close_count == closes: remaining = deadline - loop.time() if remaining <= 0: @@ -481,8 +490,8 @@ async def _reconnect_before(self, deadline: float, closes: int) -> None: break await asyncio.sleep(min(_REBOOT_RETRY_PAUSE, remaining)) - # Giving up without a reader left the updated charger unreached - # (A18-06); a failed attempt may have stopped the one there was. + # Giving up without a reader left the updated charger unreached; + # a failed attempt may have stopped the one there was. self._connection.resume_reconnecting() msg = "Timeout reconnecting after firmware update" raise WattpilotConnectionError(msg) @@ -494,7 +503,7 @@ async def _reconnect_within(self, remaining: float) -> bool: await self.connect() except Exception: if self.refusal is not None: - # Retried, it ended up reported as a timeout (R1-13). + # Retried, it ended up reported as a timeout. raise return False return True @@ -505,7 +514,7 @@ async def _load_api_definition(self) -> None: """ Read the API definition into the cache, off the event loop. - wattpilot: the definition is an 88 kB YAML file, and the lazy path + The definition is a large YAML file, and the lazy path below used to read it during the first write -- blocking the caller's event loop at a moment nobody chose. Loading it while connecting keeps that read in a thread, and leaves the lazy path as the @@ -552,7 +561,7 @@ def _coerce_to_json_type( case "float": return _as_float(value, name) case "string": - return str(value) + return _as_text(value, name) case _: return value @@ -568,35 +577,17 @@ def unsubscribe() -> None: return unsubscribe - def on_message(self, callback: MessageCallback) -> Callable[[], None]: - """Register a raw-message callback. Returns an unsubscribe function.""" - self._message_callbacks.append(callback) - - def unsubscribe() -> None: - """Remove the callback registered above.""" - self._message_callbacks.remove(callback) - - return unsubscribe - # ---- Internal: message loop ---- # One branch per message type the charger sends; the dispatch is the # function's whole content. The complexity ratchet in tests/ freezes # the number so it cannot creep upward unnoticed. - async def _handle_message(self, raw: str) -> None: # noqa: C901 - # wattpilot: type only, never the payload (audit VA-01). Frames + async def _handle_message(self, raw: str) -> None: + # Type only, never the payload. Frames # carry Wi-Fi passwords, cloud tokens and OCPP keys, and DEBUG # logging for this integration is one toggle away in the UI. _LOGGER.debug("Message received: %s (%d bytes)", _message_type(raw), len(raw)) msg = json.loads(raw) - - # Fire raw message callbacks - for cb in self._message_callbacks: - if inspect.iscoroutinefunction(cb): - await cb(msg) - else: - cb(msg) - msg_type = msg.get("type", "") ns = json.loads(raw, object_hook=lambda d: SimpleNamespace(**d)) @@ -638,7 +629,7 @@ async def _identity_accepted(self, serial: str) -> bool: Hello comes on every connection, reconnects included, so the check lives here: DHCP can hand the address to another device, and a - reconnect kept the entry pointed at whatever answered (audit A11-02). + reconnect kept the entry pointed at whatever answered. """ known = self._device.serial if not known or known == serial: @@ -652,7 +643,7 @@ async def _identity_accepted(self, serial: str) -> bool: async def _on_auth_required(self, msg: SimpleNamespace) -> None: if hasattr(msg, "hash"): if msg.hash not in AuthHashType: - # Unanswered, this was a timeout with no cause (A18-18). + # Unanswered, this was a timeout with no cause. msg_text = f"Unsupported password hash: {msg.hash}" await self._connection.reject(WattpilotConnectionError(msg_text)) return @@ -675,8 +666,8 @@ def _on_auth_success(self, _msg: SimpleNamespace) -> None: async def _on_auth_error(self, msg: SimpleNamespace) -> None: error_msg = getattr(msg, "message", "Unknown auth error") - _LOGGER.error("Authentication failed: %s", error_msg) - # Rejected, not failed: after authSuccess the socket stayed open (PC2-02). + _LOGGER.debug("Authentication failed: %s", error_msg) + # Rejected, not failed: after authSuccess the socket stayed open. await self._connection.reject(AuthenticationError(error_msg)) def _on_full_status(self, msg: SimpleNamespace) -> None: @@ -689,23 +680,29 @@ def _on_delta_status(self, msg: SimpleNamespace) -> None: self._apply_status(status) def _apply_status(self, status: object) -> None: - # wattpilot: one property at a time. A value this client cannot + # One property at a time. A value this client cannot # read cost the rest of its frame, and in the last chunk of the - # first full status the status never completed (audit A18-16). + # first full status the status never completed. # A status that is no object (`null`) has nothing to apply. for key, value in getattr(status, "__dict__", {}).items(): try: self._update_property(key, value) - except ValueError, TypeError, LookupError, AttributeError, RecursionError: - _LOGGER.warning("Ignoring an unreadable value for %s", key) + except ( + ValueError, + TypeError, + LookupError, + AttributeError, + RecursionError, + ) as exc: + self._connection.skip(f"the value of {key}", exc) def _on_response(self, msg: SimpleNamespace) -> None: - # wattpilot: resolve the waiting command (audit VA-03). An answer + # Resolve the waiting command. An answer # without a waiter -- or without an id at all -- is still applied # and logged. request_id = getattr(msg, "requestId", None) future = self._pending_commands.get(_correlation_key(request_id)) - # Truthy, not only a JSON true (L-039): the Home sends true, the Flex + # Truthy, not only a JSON true: the Home sends true, the Flex # was never measured, and refusing a real acceptance fails every write. if msg.success: # Nothing in the protocol promises a status on every acceptance. @@ -729,9 +726,9 @@ def _update_property( name: str, value: Any, # noqa: ANN401 -- charger values are dynamically typed ) -> None: - # wattpilot: every status frame arrives here, from the full replay, + # Every status frame arrives here, from the full replay, # a delta and a command response alike -- so this is the one place - # that has to turn the wire shape into plain JSON (audit A11-03). + # that has to turn the wire shape into plain JSON. value = _plain_json(value) self._all_props[name] = value @@ -741,26 +738,19 @@ def _update_property( case "fwv": self._firmware = value - # Fire property callbacks for cb in self._property_callbacks: - if inspect.iscoroutinefunction(cb): - task = asyncio.ensure_future(cb(name, value)) - task.add_done_callback( - lambda t: t.exception() if not t.cancelled() else None - ) - else: - cb(name, value) + cb(name, value) async def _update_hashed_password(self) -> None: """ Hash the password for this device, off the event loop. - PBKDF2 measured 110-116 ms per handshake on the event loop (A11-08). + PBKDF2 measured 110-116 ms per handshake on the event loop. """ if not self._password or not self._device.serial: return inputs = (self._password, self._device.serial, self._auth_hash_type) - # wattpilot: the result never changes for the same inputs, and + # The result never changes for the same inputs, and # recomputing it delayed every reconnect's auth reply by that much. if inputs == self._hashed_from: return @@ -771,7 +761,7 @@ async def _send_command( self, message: dict[str, Any], *, secure: bool = False ) -> None: """ - wattpilot: send a command and wait for its acknowledgement. + Send a command and wait for its acknowledgement. Added here rather than in the caller so every command path gets it. A rejection and a silence are both errors -- reporting success for @@ -797,7 +787,7 @@ async def _send(self, message: dict[str, Any], *, secure: bool = False) -> None: if secure: message = sign_secured_message(message, self._hashed_password) - # wattpilot: same reason as on receive -- a secured message carries + # Same reason as on receive -- a secured message carries # its signature, and setValue carries whatever is being written. _LOGGER.debug("Sending: %s (key=%s)", message.get("type"), message.get("key")) await self._connection.send(json.dumps(message)) diff --git a/custom_components/wattpilot/api/connection.py b/custom_components/wattpilot/api/connection.py index 15f79d1..b0751db 100644 --- a/custom_components/wattpilot/api/connection.py +++ b/custom_components/wattpilot/api/connection.py @@ -18,7 +18,7 @@ import websockets import websockets.asyncio.client -from .exceptions import WattpilotConnectionError, describe_error +from .exceptions import WattpilotConnectionError, describe_error, where_raised if TYPE_CHECKING: from collections.abc import AsyncIterator, Awaitable, Callable, Coroutine @@ -58,11 +58,12 @@ def __init__( # noqa: PLR0913 -- every connection knob the client offers self._auto_reconnect = auto_reconnect self._reconnect_delay_min = reconnect_delay_min self._reconnect_delay_max = reconnect_delay_max + self._reconnect_delay = reconnect_delay_min self.socket: websockets.asyncio.client.ClientConnection | None = None self.message_loop_task: asyncio.Task[None] | None = None self._readiness_guard: asyncio.Task[None] | None = None - # wattpilot: one field for every reason this connection cannot be + # One field for every reason this connection cannot be # used -- rejected credentials or the wrong charger answering. A # second flag beside it would be a second place to forget. self.fatal_error: WattpilotError | None = None @@ -70,11 +71,12 @@ def __init__( # noqa: PLR0913 -- every connection knob the client offers self._authenticated = False self._initialized = False self._streaming_full_status = False + self._skipped: set[str] = set() self._authenticated_event = asyncio.Event() self._initialized_event = asyncio.Event() self._disconnected_event = asyncio.Event() self._disconnected_event.set() - # One transition at a time (audits A13-01, A14-01, A15-01): opens + # One transition at a time: opens # share one attempt, close() cancels it rather than waiting it out, # and an open() during a close() waits for a fresh connection. self._opening: asyncio.Task[None] | None = None @@ -138,12 +140,13 @@ def receive_delta_status(self) -> None: self.mark_initialized() def mark_initialized(self) -> None: - """Record that the complete property snapshot has arrived.""" + """Record the full snapshot; a stalled socket keeps the backoff growing.""" self._initialized = True + self._reconnect_delay = self._reconnect_delay_min self._initialized_event.set() def fail(self, error: WattpilotError) -> None: - """Record the refusal, sign out at once and wake open()'s waits (PC3-01/02).""" + """Record the refusal, sign out at once and wake open()'s waits.""" self.fatal_error = error self._set_authenticated(value=False) self._authenticated_event.set() @@ -162,11 +165,11 @@ async def open(self) -> None: # Every close in progress, not just the first: one can start as another ends. while self._closing is not None and not self._closing.done(): await asyncio.wait({self._closing}) - self._stopped = False # not in the attempt, which undid a close (PC-01) + self._stopped = False # not in the attempt, which undid a close attempt = self._opening = self._ongoing(self._opening, self._open) try: # Not shielded: a caller Home Assistant gives up on takes the - # attempt down with it, socket and reader included (A11-05). + # attempt down with it, socket and reader included. await attempt except asyncio.CancelledError: if _cancelled_by_caller(attempt): @@ -181,13 +184,13 @@ async def _open(self) -> None: await self._finish_initialization() return # A reader still sitting out its retry backoff owns a socket; - # replacing it without stopping it left two readers racing (A12-01). + # replacing it without stopping it left two readers racing. if had_reader: await self._close() await self._open_socket() except BaseException: # That reader was the one retrying; failed, timed out or cancelled, - # nothing reached the charger again (A18-05, R1-12). + # nothing reached the charger again. if had_reader: self.resume_reconnecting() raise @@ -218,7 +221,7 @@ async def _open_socket(self) -> None: def resume_reconnecting(self) -> None: """Restart the reconnect loop after a failed attempt -- not after a refusal.""" - # Nor after a close: an unload in a firmware wait was overridden (R1-11). + # Nor after a close: an unload in a firmware wait was overridden. if self._stopped or self.fatal_error is not None or self._reader_owned(): return if self._auto_reconnect: @@ -240,13 +243,14 @@ def begin(self) -> None: Reset everything that belongs to a single connection. Every path that opens a socket comes through here; carrying the - initialized flag over let open() return before the new snapshot - (audit A11-07). The property cache is deliberately kept: clearing it - would blank every entity for the length of the outage. + initialized flag over let open() return before the new snapshot. + The property cache is deliberately kept: clearing it would blank + every entity for the length of the outage. """ self._cancel_readiness_guard() self._initialized = False self._streaming_full_status = False + self._skipped.clear() self._initialized_event.clear() self._authenticated_event.clear() self.fatal_error = None @@ -258,7 +262,7 @@ def begin(self) -> None: async def close(self) -> None: """Stop the reader and close the socket, however either of them ended.""" # Visible before anything is awaited: while it cancelled the pending - # attempt unannounced, a new open() started and was torn down (A16-01). + # attempt unannounced, a new open() started and was torn down. self._stopped = True self.close_count += 1 self._closing = self._ongoing(self._closing, self._cancel_and_close) @@ -297,15 +301,15 @@ async def _close(self) -> None: if task is not None: task.cancel() # wait(), not await: catching the reader's cancellation also - # swallowed a close aimed at the caller (RF-L-02). + # swallowed a close aimed at the caller. await asyncio.wait({task}) if not task.cancelled() and (err := task.exception()) is not None: - # A failed loop re-raised here, before the socket closed - # (A11-06). No traceback: it can quote the address (PC-02). + # A failed loop re-raised here, before the socket closed. + # No traceback: it can quote the address. _LOGGER.debug("Message loop ended: %s", describe_error(err)) finally: # Reset even when close() raises, or `connected` stays True with - # the reader already cancelled (audit VA-04). + # the reader already cancelled. try: if self.socket is not None: await self.socket.close() @@ -342,13 +346,9 @@ async def _guard_readiness(self) -> None: """ Give up on a reconnected socket that never finishes the handshake. - open() can wait for readiness inline because a separate task does the - reading. After an automatic reconnect the reader is this very loop, so - the same wait would deadlock -- nothing would read the frames it waits - for. Watching from the side instead: a peer whose socket stays up - while its application handshake stalls used to hold the integration - unavailable for good, with no further retry (audit A12-04). Closing - the socket ends the frame iteration, and the loop tries again. + After an automatic reconnect the reader is this very loop, so waiting + inline as open() does would deadlock. Watching from the side, closing + the socket ends the frame iteration and the loop tries again. """ try: await asyncio.wait_for( @@ -356,7 +356,7 @@ async def _guard_readiness(self) -> None: self.connect_timeout + self.init_timeout, ) except TimeoutError: - _LOGGER.warning("Reconnected socket never became ready; closing it") + _LOGGER.debug("Reconnected socket never became ready; closing it") if self.socket is not None: await self.socket.close() @@ -367,7 +367,7 @@ async def _cleanup_on_failure(self) -> AsyncIterator[None]: BaseException, not Exception: Home Assistant cancels a setup that takes too long, and CancelledError is not an Exception -- the - narrower catch left both running behind it (audit A11-05). + narrower catch left both running behind it. """ try: yield @@ -384,7 +384,7 @@ async def _read_frames(self) -> None: async for frame in self.socket: if self.fatal_error is not None: # Frames buffered before a refusal were still applied: the - # wrong device's amp reached the cache (audit A12-02). + # wrong device's amp reached the cache. _LOGGER.debug("Dropping a frame that arrived after the refusal") break try: @@ -393,12 +393,18 @@ async def _read_frames(self) -> None: except Exception as exc: # noqa: BLE001 -- one frame, not the connection # A failing frame (bad JSON, not UTF-8, nested past the recursion # limit) ended the reader; connection errors end the loop itself. - cause = type(exc).__name__ - _LOGGER.warning("Skipping a frame (%d bytes): %s", len(frame), cause) + self.skip(f"a frame ({len(frame)} bytes)", exc) + + def skip(self, subject: str, error: Exception) -> None: + """Warn once per connection and cause: a defect can break every frame.""" + cause = f"{type(error).__name__} at {where_raised(error)}" + level = logging.DEBUG if cause in self._skipped else logging.WARNING + self._skipped.add(cause) + _LOGGER.log(level, "Skipping %s: %s", subject, cause) async def _message_loop(self) -> None: # Started without a socket (resume_reconnecting), it begins in its backoff. - reconnect_delay = self._reconnect_delay_min + self._reconnect_delay = self._reconnect_delay_min try: while True: try: @@ -414,24 +420,19 @@ async def _message_loop(self) -> None: # Retrying cannot help against rejected credentials or a # charger that is not ours; both would loop forever. if self.fatal_error is not None: - _LOGGER.error("Not reconnecting: %s", self.fatal_error) + _LOGGER.debug("Not reconnecting: %s", self.fatal_error) break - _LOGGER.debug("Reconnecting in %.0fs...", reconnect_delay) - await asyncio.sleep(reconnect_delay) + delay = self._reconnect_delay + _LOGGER.debug("Reconnecting in %.0fs...", delay) + await asyncio.sleep(delay) + self._reconnect_delay = min(delay * 2, self._reconnect_delay_max) try: self.begin() self.socket = await websockets.asyncio.client.connect(self._url) self._readiness_guard = asyncio.create_task(self._guard_readiness()) - reconnect_delay = self._reconnect_delay_min except (OSError, websockets.exceptions.WebSocketException) as exc: - reconnect_delay = min( - reconnect_delay * 2, self._reconnect_delay_max - ) - cause = describe_error(exc) # Once per cycle while the charger is away: DEBUG, not WARNING. - _LOGGER.debug( - "Reconnect failed: %s, retry in %.0fs", cause, reconnect_delay - ) + _LOGGER.debug("Reconnect failed: %s", describe_error(exc)) finally: self._set_authenticated(value=False) diff --git a/custom_components/wattpilot/api/definition.py b/custom_components/wattpilot/api/definition.py index a10294d..3415df9 100644 --- a/custom_components/wattpilot/api/definition.py +++ b/custom_components/wattpilot/api/definition.py @@ -2,6 +2,7 @@ from __future__ import annotations +import functools import importlib.resources as import_resources import logging import pkgutil @@ -19,8 +20,6 @@ class ApiDefinition: """Parsed representation of ``wattpilot.yaml``.""" - config: dict[str, Any] = field(default_factory=dict) - messages: dict[str, dict[str, Any]] = field(default_factory=dict) properties: dict[str, dict[str, Any]] = field(default_factory=dict) @@ -57,6 +56,14 @@ def _add_unique(d: dict[str, Any], k: str, v: Any) -> dict[str, Any]: # noqa: A return d +# The C loader parses the file eight times faster where PyYAML was built with +# it; both are safe loaders. +_LOADER = getattr(yaml, "CSafeLoader", yaml.SafeLoader) + + +# Once per process: every client parsed it again, a setup probe included, and +# nothing changes it at runtime. Its readers only read. +@functools.cache def load_api_definition() -> ApiDefinition: """ Load and parse ``wattpilot.yaml`` from package resources. @@ -90,10 +97,8 @@ def load_api_definition() -> ApiDefinition: api_def = ApiDefinition() try: - api_def.config = validate_api_definition(yaml.safe_load(raw_text or "{}")) - api_def.messages = {m["key"]: m for m in api_def.config["messages"]} - - for p in api_def.config["properties"]: + config = validate_api_definition(yaml.load(raw_text or "{}", Loader=_LOADER)) # noqa: S506 -- a safe loader + for p in config["properties"]: api_def.properties = _add_unique(api_def.properties, p["key"], p) except yaml.YAMLError as exc: diff --git a/custom_components/wattpilot/api/exceptions.py b/custom_components/wattpilot/api/exceptions.py index 61a99e4..2a4f9c4 100644 --- a/custom_components/wattpilot/api/exceptions.py +++ b/custom_components/wattpilot/api/exceptions.py @@ -1,13 +1,17 @@ -"""Exception hierarchy for the wattpilot-api library.""" +"""Exceptions of the Wattpilot API client.""" from __future__ import annotations import errno import socket +import traceback +from pathlib import Path + +_INTEGRATION = str(Path(__file__).parent.parent) class WattpilotError(Exception): - """Base exception for all wattpilot-api errors.""" + """Base exception of the Wattpilot API client.""" class WattpilotConnectionError(WattpilotError): @@ -25,7 +29,7 @@ class AuthenticationError(WattpilotError): class PropertyError(WattpilotError): - """Raised when a property operation fails (unknown key, read-only, etc.).""" + """Raised when a value would change on conversion, or no update is offered.""" class CommandError(WattpilotError): @@ -43,7 +47,7 @@ def __init__(self, message: str, *, found: str) -> None: def describe_error(error: BaseException) -> str: """ - wattpilot: an error fit for the log, which users attach to public issues. + Return an error fit for the log, which users attach to public issues. Our own messages are written to carry no identifier. A socket error's text quotes the charger's address and port, so anything else is named @@ -59,3 +63,18 @@ def describe_error(error: BaseException) -> str: if code is None: return type(error).__name__ return f"{type(error).__name__} ({errno.errorcode.get(code, code)})" + + +def where_raised(error: BaseException) -> str: + """ + Return the integration's own line an error passed last, as file:line. + + Enough to find a defect from a log line; the message is left out, as it + can quote what the charger sent. + """ + own = [ + frame + for frame in traceback.extract_tb(error.__traceback__) + if frame.filename.startswith(_INTEGRATION) + ] + return f"{Path(own[-1].filename).name}:{own[-1].lineno}" if own else "?" diff --git a/custom_components/wattpilot/api/models.py b/custom_components/wattpilot/api/models.py index cd2f21e..ed9e751 100644 --- a/custom_components/wattpilot/api/models.py +++ b/custom_components/wattpilot/api/models.py @@ -3,58 +3,7 @@ from __future__ import annotations from dataclasses import dataclass -from enum import IntEnum, StrEnum - - -class CarStatus(IntEnum): - """Car connection / charging status.""" - - NO_CAR = 1 - CHARGING = 2 - READY = 3 - COMPLETE = 4 - - -class AccessState(IntEnum): - """Access / lock state.""" - - OPEN = 0 - WAIT = 1 - - -class ErrorState(IntEnum): - """Device error state.""" - - UNKNOWN = 0 - IDLE = 1 - CHARGING = 2 - WAIT_CAR = 3 - COMPLETE = 4 - ERROR = 5 - - -class CableLockMode(IntEnum): - """Cable lock behaviour.""" - - NORMAL = 0 - AUTO_UNLOCK = 1 - ALWAYS_LOCK = 2 - - -class ForceState(IntEnum): - """Force charging state.""" - - NEUTRAL = 0 - OFF = 1 - ON = 2 - - -class PhaseSwitchMode(IntEnum): - """Phase switching mode.""" - - AUTO = 0 - FORCE_1 = 1 - FORCE_3 = 2 +from enum import StrEnum class AuthHashType(StrEnum): diff --git a/custom_components/wattpilot/config_flow.py b/custom_components/wattpilot/config_flow.py index 7a2fd78..3b1f9ff 100644 --- a/custom_components/wattpilot/config_flow.py +++ b/custom_components/wattpilot/config_flow.py @@ -2,7 +2,6 @@ from __future__ import annotations -import logging from ipaddress import IPv4Address, ip_address from typing import TYPE_CHECKING, Any @@ -18,8 +17,8 @@ from homeassistant.data_entry_flow import AbortFlow from homeassistant.exceptions import ( ConfigEntryAuthFailed, + ConfigEntryError, ConfigEntryNotReady, - HomeAssistantError, ) from homeassistant.helpers.selector import ( NumberSelector, @@ -49,8 +48,6 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.helpers.service_info.zeroconf import ZeroconfServiceInfo -_LOGGER = logging.getLogger(__name__) - USER_SCHEMA = probatio.Schema( {probatio.Required("host"): str, probatio.Required("password"): str} ) @@ -106,6 +103,18 @@ def _address_update(known: ConfigEntry | None, ipv4: str) -> dict[str, str] | No return {"host": ipv4} +# What the form says for each way a probe fails. The hub raises +# ConfigEntryError only for another charger's serial. A ValueError is an +# address that cannot be built (an unbracketed IPv6 address, a bad port, an +# empty label), which the form showed as "Unknown error". +PROBE_ERRORS: dict[type[Exception], dict[str, str]] = { + ConfigEntryError: {"base": "wrong_device"}, + ConfigEntryAuthFailed: {"base": "invalid_auth"}, + ConfigEntryNotReady: {"base": "cannot_connect"}, + ValueError: {"host": "invalid_host"}, +} + + class WattpilotConfigFlow(ConfigFlow, domain=DOMAIN): """Handle the local setup, reauth and reconfigure flows.""" @@ -120,59 +129,63 @@ def async_get_options_flow(_entry: ConfigEntry) -> WattpilotOptionsFlow: return WattpilotOptionsFlow() async def _async_validate( - self, host: str, password: str + self, host: str, password: str, serial: str | None = None ) -> tuple[dict[str, str], str]: - """ - Try to connect; return (errors, serial). - - The probe connection is always closed before returning -- on the - success path as well as every failure path -- via the `finally`, - which is the one point every path through this function converges - on, rather than repeating the shutdown call in each branch. - """ - hub = WattpilotHub.create_local(self.hass, "config_flow", host, password) + """Try to connect; return (errors, serial). The probe is always closed.""" + hub = WattpilotHub.create_local( + self.hass, "config_flow", host, password, serial=serial + ) try: await hub.async_connect() - except ConfigEntryAuthFailed: - return {"base": "invalid_auth"}, "" - except ConfigEntryNotReady: - return {"base": "cannot_connect"}, "" - except ValueError: - # An unbracketed IPv6 address, a bad port, an empty label: building - # the address raised, and the form showed "Unknown error" (A18-21). - return {"host": "invalid_host"}, "" + except tuple(PROBE_ERRORS) as err: + return next( + form for kind, form in PROBE_ERRORS.items() if isinstance(err, kind) + ), "" finally: - # The probe connection is disposable: a failure closing it must - # not replace the validation result with a disconnect error. - try: - await hub.async_shutdown() - except HomeAssistantError: - _LOGGER.debug( - "Ignoring error while closing the probe connection", - exc_info=True, - ) + # Disposable: a failure closing it never replaces the result. + await hub.async_shutdown() return {}, hub.serial - @staticmethod - def _is_another_charger(entry: ConfigEntry, serial: str) -> bool: - """Whether the connected charger contradicts the entry's own identity.""" - return not awaits_serial(entry) and serial != entry.unique_id - def _identity_error(self, entry: ConfigEntry, serial: str) -> str | None: """ Name what is wrong with moving this entry onto that charger, if anything. An entry still awaiting its serial accepts any charger -- but not one that has an entry of its own: the setup after the flow would - refuse it as a duplicate, with host and password already overwritten - (audit A18-22). + refuse it as a duplicate, with host and password already overwritten. """ - if self._is_another_charger(entry, serial): + if not awaits_serial(entry) and serial != entry.unique_id: return "wrong_device" if serial_taken_by_other_entry(self.hass, entry, serial): return "charger_has_entry" return None + async def _async_validate_for( + self, entry: ConfigEntry, host: str, password: str + ) -> dict[str, str]: + """ + Probe for an existing entry, which only its own charger may take. + + A known serial is checked before the password is sent, as setup does: + another charger is the wrong device, not a wrong password. + """ + known = None if awaits_serial(entry) else entry.unique_id + errors, serial = await self._async_validate(host, password, known) + identity = None if errors else self._identity_error(entry, serial) + return {"base": identity} if identity else errors + + def _create_local_entry(self, host: str, password: str) -> ConfigFlowResult: + name = new_device_name(self.hass) + return self.async_create_entry( + title=name, + data={ + CONF_NAME: name, + CONF_CONNECTION_TYPE: CONNECTION_LOCAL, + "host": host, + "password": password, + }, + ) + @staticmethod def _local_entry_data( entry: ConfigEntry, user_input: dict[str, Any] @@ -207,15 +220,8 @@ async def async_step_user( # must not stop adding it by hand; it goes once the entry exists. await self.async_set_unique_id(serial, raise_on_progress=False) self._abort_if_unique_id_configured() - name = new_device_name(self.hass) - return self.async_create_entry( - title=name, - data={ - CONF_NAME: name, - CONF_CONNECTION_TYPE: CONNECTION_LOCAL, - "host": user_input["host"], - "password": user_input["password"], - }, + return self._create_local_entry( + user_input["host"], user_input["password"] ) return self.async_show_form( step_id="user", data_schema=USER_SCHEMA, errors=errors @@ -248,9 +254,9 @@ def _abort_if_awaiting_entry_at(self, host: str) -> None: Abort for an entry at this address that has yet to learn its serial. Keyed on its address until it connects, it was not found by the - serial, and an entry made from a card or the form locked it out - (R1-14, PC-04). shortcut: the address is compared as written, so a - host name against an IP goes unnoticed; resolve both if that occurs. + serial, and an entry made from a card or the form locked it out. + shortcut: the address is compared as written, so a host name + against an IP goes unnoticed; resolve both if that occurs. """ awaiting = entry_awaiting_serial_at(self.hass, host) if awaiting is None: @@ -279,15 +285,8 @@ async def async_step_zeroconf_confirm( # card was open, and a second entry with it would make Home # Assistant delete the first. self._abort_if_unique_id_configured() - name = new_device_name(self.hass) - return self.async_create_entry( - title=name, - data={ - CONF_NAME: name, - CONF_CONNECTION_TYPE: CONNECTION_LOCAL, - "host": self._discovered_host, - "password": user_input["password"], - }, + return self._create_local_entry( + self._discovered_host, user_input["password"] ) return self.async_show_form( step_id="zeroconf_confirm", @@ -303,7 +302,7 @@ async def async_step_ignore(self, user_input: dict[str, Any]) -> ConfigFlowResul A fork entry learns its serial only once it connects, so a card for the same charger can still be open. Home Assistant resolves an ignored card with that unique_id by replacing the entry with it, - entities and entity ids gone (audit A18-01). + entities and entity ids gone. """ await self.async_set_unique_id(user_input["unique_id"], raise_on_progress=False) self._abort_if_unique_id_configured() @@ -322,14 +321,9 @@ async def async_step_reauth_confirm( errors: dict[str, str] = {} entry = self._get_reauth_entry() if user_input is not None: - errors, serial = await self._async_validate( - entry.data["host"], user_input["password"] + errors = await self._async_validate_for( + entry, entry.data["host"], user_input["password"] ) - # The check reconfigure makes: without it a password accepted by - # whatever answers at the address was written in and reported as - # success, leaving the entry broken (audit A12-07). - if not errors and (identity := self._identity_error(entry, serial)): - errors = {"base": identity} if not errors: return self.async_update_reload_and_abort( entry, data_updates={"password": user_input["password"]} @@ -358,11 +352,9 @@ async def async_step_reconfigure( errors: dict[str, str] = {} entry = self._get_reconfigure_entry() if user_input is not None: - errors, serial = await self._async_validate( - user_input["host"], user_input["password"] + errors = await self._async_validate_for( + entry, user_input["host"], user_input["password"] ) - if not errors and (identity := self._identity_error(entry, serial)): - errors = {"base": identity} if not errors: return self.async_update_reload_and_abort( entry, data=self._local_entry_data(entry, user_input) diff --git a/custom_components/wattpilot/const.py b/custom_components/wattpilot/const.py index bead44a..c688029 100644 --- a/custom_components/wattpilot/const.py +++ b/custom_components/wattpilot/const.py @@ -31,11 +31,11 @@ CONF_UPDATE_INTERVAL = "update_interval" DEFAULT_UPDATE_INTERVAL = 5 -# Set by the fork migration, cleared the first time setup reads the serial. -# Its presence is the only thing that makes adopting a *different* unique_id -# legitimate: a fork entry is keyed on the IP and has yet to learn its -# serial. Without it, a serial that disagrees with the entry means the -# address now belongs to another charger. +# Set by the fork migration on a local entry, whose unique_id is whatever +# the fork keyed it on (an address, a name, or nothing), and set again by +# Reconfigure on any entry that still awaits its serial; removed when setup +# adopts the serial. One of the shapes awaits_serial() accepts: without one, +# a serial that disagrees with the entry means another charger. CONF_AWAITING_SERIAL = "awaiting_serial" PLATFORMS: list[Platform] = [ @@ -60,10 +60,11 @@ def awaits_serial(entry: ConfigEntry) -> bool: one has never verified against a live local connection. This is the single condition under which a serial that disagrees with - the entry is an upgrade rather than a different charger. Both callers - that decide identity -- setup adopting a serial, and the reconfigure - flow deciding whether to reject one -- have to agree on it, or one of - them locks the user out of a state the other happily creates. + the entry is an upgrade rather than a different charger. Every place + that decides identity has to agree on it, or one of them locks the user + out of a state another happily creates: which serial setup and the + reauth and reconfigure probes check, setup adopting a serial, and those + flows rejecting one. """ return ( entry.unique_id is None @@ -103,7 +104,7 @@ def serial_taken_by_other_entry( A second entry for the same charger would fight over the device. An ignored card is not one: it holds the serial only to keep the card from coming back, and counting it locked the fork entry out of its own - charger for good (audit A18-07). Setup and the reconfigure and reauth + charger for good. Setup and the reconfigure and reauth flows decide on this together, like on awaits_serial. """ return any( @@ -152,7 +153,7 @@ def reload_failed_entry( Home Assistant reloads only a loaded or retrying entry after a discovery update: one that met another charger at its old address and failed stayed - failed with the right address stored (reaudit RF-E-01). Only a change + failed with the right address stored. Only a change reloads, or every announcement would. """ if entry is None or not updates or entry.state is not ConfigEntryState.SETUP_ERROR: diff --git a/custom_components/wattpilot/descriptions.py b/custom_components/wattpilot/descriptions.py index 76578db..ce8c132 100644 --- a/custom_components/wattpilot/descriptions.py +++ b/custom_components/wattpilot/descriptions.py @@ -43,8 +43,8 @@ from collections.abc import Callable, Mapping, Sequence # Charger hardware variants, as the device reports them in "var". The value -# is compared as a string because the real device sends the integer 11 while -# the vendor client annotates the property as str. +# is compared as a string: the constraints below are written as text, the +# device sends the integer 11. VARIANT_11 = "11" VARIANT_22 = "22" @@ -996,7 +996,7 @@ def _seconds_setting(key: str, charger_key: str) -> WattpilotNumberEntityDescrip entity_category=EntityCategory.CONFIG, # No MONETARY device class: it requires an ISO 4217 currency code and # "ct" is not one, so Home Assistant would be told a currency that - # does not exist (audit VA-15). Converting instead is not available + # does not exist. Converting instead is not available # either -- "awc" selects the market, so the currency is not fixed at # description time. The device stores this in cent (vendor schema: # "awattarMaxPrice in ct") and we pass it through unscaled; labelling @@ -1448,7 +1448,7 @@ class WattpilotButtonEntityDescription( # datetime.time conversion mirrors the fork's runtime ChargerTime exactly. # Writes go through hub.async_set_next_trip rather than a plain property # write, and send the time unchanged: the charger applies no daylight-saving -# correction of its own (measured, audit A12-06). +# correction of its own. @dataclass(frozen=True, kw_only=True) class WattpilotTimeEntityDescription(WattpilotDescriptionMixin, TimeEntityDescription): """Time description.""" diff --git a/custom_components/wattpilot/diagnostics.py b/custom_components/wattpilot/diagnostics.py index 799e5b6..6d62add 100644 --- a/custom_components/wattpilot/diagnostics.py +++ b/custom_components/wattpilot/diagnostics.py @@ -5,11 +5,12 @@ function scripts/anonymize_probe.py uses to build the committed test fixture, so the two can never drift apart. That covers device-reported values (credential keys dropped, "cci"/"cards" neutralized instead of -dropped outright, MAC addresses and network fields scrubbed regardless of -key name). TO_REDACT stays a small hand-picked list here only for -entry.data: the connection settings the user typed in and what the entry -stores about the charger (device name, a cloud entry's serial), which are -not device-reported properties and so are out of scope for redaction.py. +dropped outright, MAC and IP literals and the serial replaced wherever +they appear, network fields such as ip, gw and ssid by name at any depth). +TO_REDACT stays a small hand-picked list here only for entry.data: the +connection settings the user typed in and what the entry stores about the +charger (device name, a cloud entry's serial), which are not +device-reported properties and so are out of scope for redaction.py. """ from __future__ import annotations @@ -27,7 +28,7 @@ from .hub import WattpilotConfigEntry # Config-entry fields -- see module docstring. A carried-over device name is -# whatever the fork's user called the charger (README audit RM2-01). +# whatever the fork's user called the charger. TO_REDACT = {"password", "host", CONF_NAME, "serial"} diff --git a/custom_components/wattpilot/entity.py b/custom_components/wattpilot/entity.py index 19d9207..291c4d5 100644 --- a/custom_components/wattpilot/entity.py +++ b/custom_components/wattpilot/entity.py @@ -60,7 +60,7 @@ async def async_added_to_hass(self) -> None: # A sentinel, not None: "the charger has not sent this" and "the # charger sent null" are different answers, and for `trx` null is # the enum's own "No Transaction". Skipping it left the entity at - # unknown until the charger repeated the field (audit VA-13). + # unknown until the charger repeated the field. initial = self._hub.get_property(description.charger_key, _NOT_SENT) if initial is not _NOT_SENT: self._apply_value(initial) diff --git a/custom_components/wattpilot/hub.py b/custom_components/wattpilot/hub.py index f0cf289..a618710 100644 --- a/custom_components/wattpilot/hub.py +++ b/custom_components/wattpilot/hub.py @@ -41,7 +41,7 @@ # The client's own errors, raw websockets errors it does not wrap, and socket # failures. A write the charger refused (CommandError) lands here too: either # way the action did not take effect. Raised `from None`: a setup traceback -# shows the cause, and a refused connect quotes the address (R1-01). +# shows the cause, and a refused connect quotes the address. _WRITE_ERRORS = (WattpilotError, WebSocketException, OSError) _REFUSAL_ISSUES = ("wrong_charger", "charger_refused") # also translation keys @@ -70,9 +70,9 @@ def __init__( self._cancel_timer: CALLBACK_TYPE | None = None self._pending: dict[str, Any] = {} self._cancel_flush: CALLBACK_TYPE | None = None - # Set by async_shutdown, cleared by async_connect: see `available`. + # Set by async_disconnect, cleared by async_connect: see `available`. self._torn_down = False - # Set once by async_shutdown: an unloaded hub never comes back (A15-02). + # Set once by async_shutdown: an unloaded hub never comes back. self._retired = False @classmethod @@ -130,8 +130,7 @@ def start_dispatch(self) -> None: are forwarded, on purpose: an entity subscribes before it reads its initial value, so starting early cannot lose a push, while starting late loses any push that lands between an entity's read and this call - (see async_setup_entry). Availability signalling does not run until - this runs. Idempotent: reconnect_charger calls it again. + (see async_setup_entry). Idempotent: reconnect_charger calls it again. """ if self._unsubscribe_properties is not None: return @@ -178,23 +177,24 @@ async def async_reconnect(self) -> None: async def async_shutdown(self) -> None: """ - Close for good: unload, failed setup, a config-flow probe. + Close for good: unload, failed setup, a config-flow probe, HA stop. - A reconnect already under way loses its connection attempt to the - close; one that starts later is refused. + A reconnect under way loses its attempt to the close, a later one is + refused. A failure to close is only logged: no caller can act on it, + and a failed unload kept the entry from reloading until a restart. """ self._retired = True self._clear_refusals() # this hub's connection; a new setup finds out afresh - await self.async_disconnect() + try: + await self.async_disconnect() + except HomeAssistantError: + _LOGGER.debug("Ignoring error while closing for good", exc_info=True) async def async_disconnect(self) -> None: """ Stop dispatching and close the connection; a reconnect may follow. - Handle cleanup (unsubscribe, cancel timer) runs first and - unconditionally, before the I/O call that can fail -- a failed - disconnect must still leave the hub's callbacks fully torn down, - not half-alive. + Callbacks go first, so that a failed close cannot leave them half alive. """ self._torn_down = True if self._unsubscribe_properties is not None: @@ -262,10 +262,10 @@ def available(self) -> bool: """Return whether the charger is currently reachable.""" # Not the client's flag alone: a close that raised leaves `connected` # True with the loop behind it gone, and once this hub has torn itself - # down no update reaches an entity again (audit VA-04). Nor before the - # full status (A18-20) -- unless no outage was seen: a reconnect + # down no update reaches an entity again. Nor before the + # full status -- unless no outage was seen: a reconnect # between two checks replays it into entities still shown, and they - # flapped (R1-04). + # flapped. return ( not self._torn_down and bool(self.charger.connected) @@ -282,7 +282,7 @@ def properties(self) -> dict[str, Any]: def get_property(self, key: str, default: Any = None) -> Any: # noqa: ANN401 """Return one charger property, or default if not seen yet.""" - return self.charger.all_properties.get(key, default) + return self.charger.get_property(key, default) # ---- push handling ---- @@ -297,7 +297,7 @@ def _handle_property_change(self, key: str, value: Any) -> None: # noqa: ANN401 self._hass, signal_property(self._entry_id, key), value ) # Not buffered: the first push after the full status (the next delta, - # about a second later) announces a recovery (R1-23). + # about a second later) announces a recovery. self._update_availability(available=self.available) @callback @@ -323,7 +323,7 @@ def _check_availability(self, _now: datetime) -> None: # Not reauth: a new password would not bring this charger back. self._report_refusal("wrong_charger", {"serial": self.serial}) elif refusal is not None: - # A sign-in scheme the client does not know (reaudit R1-13). + # A sign-in scheme the client does not know. self._report_refusal("charger_refused", {"error": describe_error(refusal)}) def _clear_refusals(self) -> None: @@ -350,8 +350,8 @@ def _update_availability(self, *, available: bool, expected: bool = False) -> No never_reported = self._last_available is None self._last_available = available if available: - self._flush_pending() # the values before the announcement (A18-20) - self._clear_refusals() # a reconnect left them up beside it (RF-L-03) + self._flush_pending() # the values before the announcement + self._clear_refusals() # a reconnect left them up beside it self._log_availability( available=available, expected=expected, never_reported=never_reported ) diff --git a/custom_components/wattpilot/number.py b/custom_components/wattpilot/number.py index b862483..596f5de 100644 --- a/custom_components/wattpilot/number.py +++ b/custom_components/wattpilot/number.py @@ -61,7 +61,7 @@ def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- dynamically shape return try: self._attr_native_value = float(value) / self.entity_description.wire_scale - # OverflowError: an int past the float range (PC6-02). + # OverflowError: an int past the float range. except TypeError, ValueError, OverflowError: self._attr_native_value = None diff --git a/custom_components/wattpilot/quality_scale.yaml b/custom_components/wattpilot/quality_scale.yaml index b146f18..1fa73a3 100644 --- a/custom_components/wattpilot/quality_scale.yaml +++ b/custom_components/wattpilot/quality_scale.yaml @@ -48,13 +48,13 @@ rules: comment: | The options flow's one parameter (update interval) is documented in the README's Configuration section. It was exempt while there was no options - flow -- adding one made the exemption wrong (audit VA-09). + flow -- adding one made the exemption wrong. docs-installation-parameters: done docs-triggers: status: exempt comment: | The integration provides no triggers. The rule was missing from this - file while it claimed to list every rule (audit VA-09). + file while it claimed to list every rule. docs-conditions: status: exempt comment: The integration provides no conditions. Added with docs-triggers. diff --git a/custom_components/wattpilot/readings.py b/custom_components/wattpilot/readings.py index b7a0038..35651dd 100644 --- a/custom_components/wattpilot/readings.py +++ b/custom_components/wattpilot/readings.py @@ -77,7 +77,7 @@ def price_list(value: Any) -> list[dict[str, Any]]: # noqa: ANN401 -- dynamical def _price_entry(entry: Any) -> dict[str, Any] | None: # noqa: ANN401 -- charger payload - """One price; None for an entry without usable unix times (RF-D-03).""" + """One price; None for an entry without usable unix times.""" try: return { "start": dt_util.utc_from_timestamp(namespace_get(entry, "start")), diff --git a/custom_components/wattpilot/redaction.py b/custom_components/wattpilot/redaction.py index 723176d..b7d823d 100644 --- a/custom_components/wattpilot/redaction.py +++ b/custom_components/wattpilot/redaction.py @@ -50,7 +50,7 @@ # host/hsta/hsts/fwan: hostnames the bundled definition documents as # "Wattpilot_". None of them is populated on firmware 42.5, which is # exactly why they had no decision -- the register only knew the fields this -# one device happened to report (audit A11-03). +# one device happened to report. REPLACE: dict[str, Any] = { "sse": "123456", "fna": "Wattpilot", @@ -65,14 +65,14 @@ "wae": True, # Free text nobody bounds: a load group named after a place, a WiFi error # quoting a network name. Empty on the reference device, which proves - # nothing about the next (audit A17-02). + # nothing about the next. "log": "", "wsm": "", # Readable on the reference device, a TLS error; on a Flex on Ethernet # it quotes the charger's address. "cle": "", # Flex free text nothing documents: an OCPP fallback card and tariff - # text, an OTA field. Placeholders on the one Flex seen (audit A18-15). + # text, an OTA field. Placeholders on the one Flex seen. "ocppfd": "", "ocpptt": "", "otaif": "", @@ -141,7 +141,7 @@ # rules only know the keys someone has seen, and a Flex on Ethernet reported # a dozen nobody had. ipaddress decides, so times and versions survive. # A full stop may end the IPv4 address, a label may precede the IPv6 one: -# "Connect to
." and "inet6:
" got through (R1-07). +# "Connect to
." and "inet6:
" got through. IPV4_CANDIDATE = re.compile(r"(? Any: # noqa: ANN401 -- untyped JS def _sanitized_cards(value: Any) -> Any: # noqa: ANN401 -- untyped JSON # A card that is not a dict used to pass through as-is, which for a bare # string means the identifier itself survived. An unknown shape is - # exactly the case we cannot judge, so it is dropped (VA-12) -- and so + # exactly the case we cannot judge, so it is dropped -- and so # is an outer value that is not a list, which used to abort the whole - # diagnostics download (A13-05). + # diagnostics download. if not isinstance(value, list): return UNEXPECTED_SHAPE return [_sanitized_card(i, card) for i, card in enumerate(value)] @@ -287,7 +287,7 @@ def _sanitized_companion(value: Any, serial: str | None) -> Any: # noqa: ANN401 if not isinstance(value, dict): # An unconditional update() raised on anything else, which took the # whole diagnostics download with it -- the one thing a user needs - # when reporting a problem (VA-12). + # when reporting a problem. return UNEXPECTED_SHAPE companion = scrub(value, serial) for field, replacement in _COMPANION_STAND_INS.items(): @@ -310,7 +310,7 @@ def sanitize_property(key: str, value: Any, serial: str | None = None) -> Any: # a private IP is not, and nothing in the value tells them apart, so # it always goes. Not a NET_FIELDS entry, which matches by name at any # depth while `smd` carries a numeric `ts` of its own -- a timestamp, - # which must survive -- and only for a string (audit VA-07). + # which must survive -- and only for a string. return _stand_in(value, TIME_SERVER_REPLACEMENT) replacement = _replacement_for(key, value) if replacement is not _KEEP: diff --git a/custom_components/wattpilot/registry_migration.py b/custom_components/wattpilot/registry_migration.py index d38788e..adc2f8e 100644 --- a/custom_components/wattpilot/registry_migration.py +++ b/custom_components/wattpilot/registry_migration.py @@ -157,10 +157,10 @@ def _migrate(entity_entry: er.RegistryEntry) -> dict[str, str] | None: # history, entity id and customisations, and the one kept is # already bound. Whether that history still matters is the # owner's call, so nothing is deleted or renamed -- but it is - # reported instead of only logged (audit VA-06). + # reported instead of only logged. collisions.append(entity_entry.entity_id) - # Not the entity ID, here or below: it can carry the serial - # (R1-02). The repair issue names it; that is not logged. + # Not the entity ID, here or below: it can carry the serial. + # The repair issue names it; that is not logged. _LOGGER.warning( "Not migrating a %s entity for %s: one already exists", entity_entry.domain, @@ -184,7 +184,7 @@ def _collision_issue_id(entry: ConfigEntry) -> str: def async_clear_collisions(hass: HomeAssistant, entry: ConfigEntry) -> None: - """Drop the notice with the entry it is about (audit A18-23).""" + """Drop the notice with the entry it is about.""" ir.async_delete_issue(hass, DOMAIN, _collision_issue_id(entry)) @@ -197,7 +197,7 @@ def _report_collisions( return registry = er.async_get(hass) # Registry ids, not entity IDs: a listed entity renamed was still named - # by its old ID, and deleting it then left the notice up (RF-E-02). + # by its old ID, and deleting it then left the notice up. listed = {found.id for name in collisions if (found := registry.async_get(name))} @callback @@ -206,7 +206,7 @@ def _is_removal_or_rename(data: er.EventEntityRegistryUpdatedData) -> bool: @callback def _recount(_event: Event[er.EventEntityRegistryUpdatedData]) -> None: - # Deleting what the notice lists left it up until a reload (R1-19). + # Deleting what the notice lists left it up until a reload. entries = (registry.async_get(registry_id) for registry_id in listed) _show_collisions(hass, entry, {found.entity_id for found in entries if found}) diff --git a/custom_components/wattpilot/select.py b/custom_components/wattpilot/select.py index a3a4efb..1b95515 100644 --- a/custom_components/wattpilot/select.py +++ b/custom_components/wattpilot/select.py @@ -90,7 +90,7 @@ def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- dynamically shape def _report_unknown(self, value: Any, *, known: bool) -> None: # noqa: ANN401 -- charger payload # Unknown alone told nobody (#25). Once until a known value: a NaN # never equals itself, and one unknown value after another tells - # nothing new (F31-03). A preset select sees a current between its + # nothing new. A preset select sees a current between its # presets as a matter of course, so only a fixed table reports. if known: self._unknown_reported = False diff --git a/custom_components/wattpilot/sensor.py b/custom_components/wattpilot/sensor.py index b219a6b..9d89632 100644 --- a/custom_components/wattpilot/sensor.py +++ b/custom_components/wattpilot/sensor.py @@ -91,7 +91,7 @@ def _sensor_class( def _expects_number(description: WattpilotSensorEntityDescription) -> bool: - """Home Assistant's rule: such a sensor's state must be a number (K-A).""" + """Home Assistant's rule: such a sensor's state must be a number.""" if description.device_class in NON_NUMERIC_DEVICE_CLASSES or description.enum: return False numeric_marks = ( @@ -111,7 +111,7 @@ class WattpilotSensor(WattpilotEntity, SensorEntity): # readings that differ in nearly every update while charging: history # of either would only fill the recorder. _unrecorded_attributes = frozenset({"prices", "readings", *NRG_ATTRIBUTES}) - # Held against, not an unknown a null left on display (RF-D-02). + # Held against, not an unknown a null left on display. _last_number: float | None = None def __init__(self, *args: Any) -> None: # noqa: ANN401 -- forwards WattpilotEntity's args unchanged @@ -146,10 +146,10 @@ def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- dynamically shape # the platform; keep the previous state. # Shape and error type, never the value: this sink is not the # diagnostics download and nothing redacts it, so a card list - # logged with %r put holder names into the log (audit A12-05). + # logged with %r put holder names into the log. # The exception's own message can quote the value too, so only # its type is reported. The sensor by its key: the entity ID - # carries the serial (R1-02). + # carries the serial. _LOGGER.warning( "%s: unexpected %s for %s (%s)", description.key, @@ -237,7 +237,7 @@ def _apply_card(self, value: Any, index: int) -> None: # noqa: ANN401 -- charge absent slot is the entity having nothing to show, not a malformed push: indexing past the end raised, and the generic handler then logged the whole card list with %r -- holder names included, in a - sink nothing redacts (audit A12-05). + sink nothing redacts. """ if index >= len(value): self._attr_native_value = None @@ -309,7 +309,7 @@ async def async_added_to_hass(self) -> None: """Seed the hold against a step back with the last reported total.""" # In memory only, the hold ended with the entity: a reload in the # window after a charger restart -- the one a firmware update - # triggers -- reported the lower total (audit A18-25). + # triggers -- reported the lower total. last = await self.async_get_last_sensor_data() if last is not None and isinstance(last.native_value, (int, float)): self._attr_native_value = self._restored_total = last.native_value @@ -321,7 +321,7 @@ def _should_suppress_decrease( ) -> bool: restored, self._restored_total = self._restored_total, None # A restored total can be another charger's: a replacement that took - # over this entity ID was held at the old total for years (R1-03). + # over this entity ID was held at the old total for years. # Only a step back as small as a restart's is held. if restored is not None and value < restored * _RESET_THRESHOLD_RATIO: return False @@ -348,7 +348,7 @@ class WattpilotAveragePowerSensor(WattpilotSensor): def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- charger payload # tpa keeps the last session's average after charging, and threshold - # automations kept firing on it for hours (audit A18-13). + # automations kept firing on it for hours. live = total_power( self._hub.get_property(self.entity_description.companion_key) # type: ignore[arg-type] ) @@ -361,11 +361,11 @@ def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- charger payload def _show_live(self, value: Any, live: float | None) -> None: # noqa: ANN401 -- charger payload if not live or value is None: # Unknown, whichever reading is: no telling a stale average, and - # which one is stale stays known (PC5-R-03, PC6-03, PC7-01). + # which one is stale stays known. super()._apply_value(None) return if value == self._stale: - self._attr_native_value = 0 # until this session's own average (RF-D-04) + self._attr_native_value = 0 # until this session's own average return self._stale = None super()._apply_value(value) diff --git a/custom_components/wattpilot/services.py b/custom_components/wattpilot/services.py index f163fb9..d253018 100644 --- a/custom_components/wattpilot/services.py +++ b/custom_components/wattpilot/services.py @@ -64,7 +64,7 @@ async def set_goe_cloud(call: ServiceCall) -> None: # Neither the key nor the URL belongs here: even a prefix and # suffix narrow the key, and the vendor URL embeds the full # serial. Both are readable from the charger for anyone who - # needs them (audit VA-14). + # needs them. _LOGGER.info("go-e cloud API enabled") else: await hub.async_disable_cloud_api() diff --git a/custom_components/wattpilot/strings.json b/custom_components/wattpilot/strings.json index a7b15fc..5517776 100644 --- a/custom_components/wattpilot/strings.json +++ b/custom_components/wattpilot/strings.json @@ -73,6 +73,9 @@ "entry_unloaded": { "message": "The charger's entry was unloaded. Reload it to reconnect." }, + "firmware_not_offered": { + "message": "The charger does not offer firmware {version}" + }, "firmware_update_failed": { "message": "Firmware update failed: {error}" }, diff --git a/custom_components/wattpilot/time.py b/custom_components/wattpilot/time.py index 741bd6f..482daf6 100644 --- a/custom_components/wattpilot/time.py +++ b/custom_components/wattpilot/time.py @@ -54,7 +54,7 @@ def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- dynamically shape return try: seconds = int(value) % _SECONDS_PER_DAY - # OverflowError: infinity (PC6-02). + # OverflowError: infinity. except TypeError, ValueError, OverflowError: self._attr_native_value = None return diff --git a/custom_components/wattpilot/translations/de.json b/custom_components/wattpilot/translations/de.json index b2af00f..c11a308 100644 --- a/custom_components/wattpilot/translations/de.json +++ b/custom_components/wattpilot/translations/de.json @@ -73,6 +73,9 @@ "entry_unloaded": { "message": "Der Eintrag des Ladegeräts wurde entladen. Zum Wiederverbinden den Eintrag neu laden." }, + "firmware_not_offered": { + "message": "Das Ladegerät bietet die Firmware {version} nicht an" + }, "firmware_update_failed": { "message": "Firmware-Update fehlgeschlagen: {error}" }, diff --git a/custom_components/wattpilot/translations/en.json b/custom_components/wattpilot/translations/en.json index a7b15fc..5517776 100644 --- a/custom_components/wattpilot/translations/en.json +++ b/custom_components/wattpilot/translations/en.json @@ -73,6 +73,9 @@ "entry_unloaded": { "message": "The charger's entry was unloaded. Reload it to reconnect." }, + "firmware_not_offered": { + "message": "The charger does not offer firmware {version}" + }, "firmware_update_failed": { "message": "Firmware update failed: {error}" }, diff --git a/custom_components/wattpilot/update.py b/custom_components/wattpilot/update.py index 6aa0c12..08266d7 100644 --- a/custom_components/wattpilot/update.py +++ b/custom_components/wattpilot/update.py @@ -6,10 +6,11 @@ from homeassistant.components.update import UpdateEntity, UpdateEntityFeature from homeassistant.core import callback +from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers.dispatcher import async_dispatcher_connect from packaging.version import InvalidVersion, Version -from .const import signal_property +from .const import DOMAIN, signal_property from .descriptions import ( UPDATE_DESCRIPTIONS, WattpilotUpdateEntityDescription, @@ -71,7 +72,7 @@ def _newest_version(raw_versions: list[str]) -> str | None: def _version(value: Any) -> str | None: # noqa: ANN401 -- charger payload """Return a version string, or None for anything else.""" # Home Assistant compares versions through a cache, and anything - # unhashable raised there (post-check PC7-02). + # unhashable raised there. return value if isinstance(value, str) and value else None @@ -79,6 +80,7 @@ class WattpilotUpdate(WattpilotEntity, UpdateEntity): """Firmware update driven by onv (available) and fwv (installed).""" entity_description: WattpilotUpdateEntityDescription + _offered: tuple[str, ...] = () # until the charger sends onv _attr_supported_features = ( UpdateEntityFeature.INSTALL | UpdateEntityFeature.SPECIFIC_VERSION ) @@ -113,6 +115,7 @@ def _apply_value(self, value: Any) -> None: # noqa: ANN401 -- dynamically shape ) entries = value if isinstance(value, list) else [value] raw_versions = [entry for entry in entries if _version(entry)] + self._offered = tuple(raw_versions) self._attr_latest_version = ( _newest_version(raw_versions) or self._attr_installed_version ) @@ -123,7 +126,15 @@ async def async_install( backup: bool, # noqa: ARG002, FBT001 -- HA's UpdateEntity base signature; called by keyword **_kwargs: Any, # noqa: ANN401 -- HA's UpdateEntity base signature requires **kwargs: Any ) -> None: - """Install the given version, defaulting to the displayed latest.""" + """Install an offered version, defaulting to the displayed latest.""" if version is None: version = self.latest_version + # The latest falls back to the installed version when nothing is + # offered, and the charger takes whatever it is sent. + if version not in self._offered: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="firmware_not_offered", + translation_placeholders={"version": str(version)}, + ) await self._hub.async_install_firmware(version) diff --git a/pyproject.toml b/pyproject.toml index 34b4221..74561ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,8 +3,7 @@ name = "ha-wattpilot" # Kept equal to custom_components/wattpilot/manifest.json, which is the # version Home Assistant and HACS actually read; a guard enforces it. This # file is tooling config, but two version numbers with no rule between them -# drift, and the release gate only ever compared the tag to the manifest -# (audit VA-17). +# drift, and the release gate only ever compared the tag to the manifest. version = "0.3.0" description = "Home Assistant integration for Fronius Wattpilot EV chargers" readme = "README.md" @@ -43,6 +42,8 @@ testpaths = ["tests"] asyncio_mode = "auto" asyncio_default_fixture_loop_scope = "function" addopts = ["-q"] +# A hung test fails on its own instead of holding the run until CI gives up. +timeout = 120 # Ours only: a deprecation inside Home Assistant or a library is theirs to # fix, and would turn the suite red with no change here. filterwarnings = [ @@ -51,11 +52,10 @@ filterwarnings = [ ] # The marker names what the test is, and `-m e2e` still selects it. It is no # longer deselected by default: it drives a full HA setup against the fake -# charger, not real hardware, and costs 3.3 s -- cheap enough that hiding it -# behind a release-only gate only meant finding those regressions late -# (audit VA-16). +# charger, not real hardware, and costs seconds -- cheap enough that hiding it +# behind a release-only gate only meant finding those regressions late. markers = [ - "e2e: drives a complete Home Assistant setup (fake charger, not hardware)", + "e2e: a complete Home Assistant setup or pytest run (fake charger, not hardware)", "logs_identifiers: the test logs a charger identifier on purpose", ] diff --git a/scripts/anonymize_probe.py b/scripts/anonymize_probe.py index 5ceee20..24ed9fe 100644 --- a/scripts/anonymize_probe.py +++ b/scripts/anonymize_probe.py @@ -43,7 +43,7 @@ def fixture_from(snapshot: dict[str, Any]) -> dict[str, Any]: cleaned = sanitize_snapshot(snapshot) if cleaned.get("ct"): # Diagnostics tell a known profile from an unknown one; the suite - # needs one the car profile select offers (post-check F31-01). + # needs one the car profile select offers. cleaned["ct"] = "default" return cleaned diff --git a/scripts/check.sh b/scripts/check.sh index 4ee0393..c07a936 100644 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -53,7 +53,7 @@ mutation_gate() { accepted=$(grep -v -e '^#' -e '^$' scripts/equivalent-mutants.txt) for mutant in $accepted; do # A name mutmut no longer has ran the original code and was booked - # "survived" below, so a renumbered entry was never caught (RF-D-06). + # "survived" below, so a renumbered entry was never caught. if ! grep -q "^ *${mutant}: " mutants/results.txt; then echo "accepted mutant ${mutant} does not exist; renumbered?" >&2 return 1 diff --git a/scripts/complexity_snapshot.py b/scripts/complexity_snapshot.py index 45eaeb3..81aca4c 100644 --- a/scripts/complexity_snapshot.py +++ b/scripts/complexity_snapshot.py @@ -13,7 +13,7 @@ def snapshot() -> dict[str, int]: """Every function over the threshold, keyed as the ratchet keys it.""" # rglob, as the ratchet does: glob missed api/, and a table regenerated - # from it lost all of that package's entries (audit A19-12). + # from it lost all of that package's entries. return { f"{path.name}::{block.fullname}": block.complexity for path in sorted(PACKAGE.rglob("*.py")) diff --git a/scripts/judge_mutants.py b/scripts/judge_mutants.py index acc4f52..85b4ff5 100644 --- a/scripts/judge_mutants.py +++ b/scripts/judge_mutants.py @@ -6,7 +6,7 @@ "no tests", "not checked", "suspicious" and "timeout" all mean the mutant was never judged. Filtering for survivors alone let a newly untested mutant vanish from the comparison entirely while the accepted survivors still -matched, and the gate passed (audit A11-10). +matched, and the gate passed. Every outcome therefore gets an explicit verdict here, and anything the policy does not know is a failure rather than a silence. diff --git a/tests/README.md b/tests/README.md index 41244da..b5f5a0b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -20,7 +20,7 @@ with the fork is checked in each platform's own test file): | Guard | Holds | |---|---| -| mutation run (check.sh --release) | tests are real: every mutant in hub/sensor/init must die, except the equivalent ones in scripts/equivalent-mutants.txt, which must survive exactly | +| mutation run (check.sh --release) | tests are real: every mutant in the modules `[tool.mutmut] source_paths` lists must die, except the equivalent ones in scripts/equivalent-mutants.txt, which must survive exactly | | next Home Assistant (.github/workflows/ha-next.yml) | weekly, against the newest release on PyPI, beta included: what it reports or removes shows up before the release; the tests run once a harness for it exists, the import check always. A warning, not a merge gate | ## Recorded frame sequences @@ -48,7 +48,7 @@ WSL — the suite only runs there, and the `gitleaks` on the Windows PATH is an older build without the `git` subcommand — and the blocklist itself lives outside the repository, because several of its entries are exactly the words it keeps out. **A fresh clone therefore has no hook.** The template it was -adapted from is in the erosionsschutz kit. +adapted from lives outside the repository as well. Why a blocklist next to `gitleaks`: on 2026-09-05 `gitleaks` reported zero findings over the full history while a test fixture carried the repository diff --git a/tests/conftest.py b/tests/conftest.py index 0f3bbcf..9b1f1f3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,4 @@ -"""Shared fixtures. FakeWattpilot mimics the wattpilot-api surface we use.""" +"""Shared fixtures. FakeWattpilot mimics the API client's surface we use.""" from __future__ import annotations @@ -7,11 +7,13 @@ import re import sys from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Self +from unittest.mock import patch import pytest from custom_components.wattpilot import _SerialMask +from custom_components.wattpilot.api import DeviceIdentityError from .deprecations import REPORTS, assert_listening, import_every_module @@ -32,22 +34,22 @@ def auto_enable_custom_integrations( Allow HA to load custom_components/wattpilot in tests. After the log fixtures, which then outlast hass: autouse fixtures are set - up by name, so the stop of Home Assistant went unchecked (PC5-T-03). + up by name, so the stop of Home Assistant went unchecked. """ # Serials and addresses the tests hand out. None may reach a log record # unmasked, tracebacks included: the static guard was widened three rounds -# running and still missed shapes, so every test now checks the real output -# (reaudit RF-P-03). Any IPv4 literal, as a list of the tests' addresses -# missed four (post-check PC5-T-04); 192.0.2.1 is the redaction's stand-in. +# running and still missed shapes, so every test now checks the real output. +# Any IPv4 literal, as a list of the tests' addresses +# missed four; 192.0.2.1 is the redaction's stand-in. IDENTIFIERS = re.compile( r"(? Iterator[No """Fail a test whose log names a charger; mark the few that must.""" handler = _LogLines() # Home Assistant's setup DEBUG lines too: it logs a setup retry's - # traceback there (post-check PC5-T-04). + # traceback there. loggers = [ logging.getLogger(name) for name in ( @@ -165,8 +167,15 @@ def device_properties_fixture() -> dict[str, Any]: return json.loads(path.read_text(encoding="utf-8")) +def patch_charger(charger: FakeWattpilot) -> Any: + """Patch the client's constructor to hand out this fake.""" + return patch( + "custom_components.wattpilot.hub.Wattpilot", side_effect=charger.built_with + ) + + class FakeWattpilot: - """Duck-typed stand-in for wattpilot_api.Wattpilot.""" + """Duck-typed stand-in for custom_components.wattpilot.api.client.Wattpilot.""" def __init__(self, properties: dict[str, Any] | None = None) -> None: self._properties: dict[str, Any] = dict(properties or {}) @@ -175,15 +184,19 @@ def __init__(self, properties: dict[str, Any] | None = None) -> None: self.properties_initialized = True # Why the real client stopped reconnecting for good, if it did. self.refusal: Exception | None = None - self.serial = "123456" + # The charger's own serial. `serial` is what the client knows of it: + # what it was built with, else what it heard, else nothing yet. + self.device_serial = "123456" + self._learned_serial = "" self.name = "Wattpilot" self.manufacturer = "fronius" self.firmware = "42.5" self.model = "Wattpilot Home 11 J 2.0" - # Deliberately an int: the real device reports "var" as a number, - # contrary to the vendor client's `-> str | None` annotation. + # Deliberately an int: the real device reports "var" as a number. self.variant = 11 self.connect_error: Exception | None = None + # The serial the integration built the client with, if any. + self.expected_serial: str | None = None self.set_error: Exception | None = None self.disconnect_error: Exception | None = None self.disconnect_attempts = 0 @@ -210,10 +223,38 @@ def all_properties(self) -> dict[str, Any]: """Mirror the real client: a fresh copy per access, not a live view.""" return dict(self._properties) + @property + def serial(self) -> str: + """Return what the client knows, like the client.""" + return self.expected_serial or self._learned_serial + + @serial.setter + def serial(self, value: str) -> None: + self.device_serial = value + + def built_with( + self, *_args: Any, serial: str | None = None, **_kwargs: Any + ) -> Self: + """ + Stand in for the client's constructor, keeping its contract. + + A serial it is given is checked against the charger before the + password is. A fake that ignored it let a flow pass that built its + client with the wrong serial. + """ + self.expected_serial = serial + self._learned_serial = "" + return self + async def connect(self) -> None: + if self.serial not in ("", self.device_serial): + msg = "Another charger answered" + self.refusal = DeviceIdentityError(msg, found=self.device_serial) + raise self.refusal if self.connect_error is not None: raise self.connect_error self.connected = True + self._learned_serial = self.device_serial # The real client replays the whole property snapshot to its # subscribers as soon as the connection is up (_on_full_status -> # _update_property -> callbacks). Without that here, a reconnect @@ -231,6 +272,9 @@ async def disconnect(self) -> None: self.connected = False self.disconnect_count += 1 + def get_property(self, name: str, default: Any = None) -> Any: + return self._properties.get(name, default) + def on_property_change( self, callback: Callable[[str, Any], None] ) -> Callable[[], None]: diff --git a/tests/parity.py b/tests/parity.py index e8e419f..84d0f47 100644 --- a/tests/parity.py +++ b/tests/parity.py @@ -73,7 +73,7 @@ # Same entity, same reason: the fork declares NumberDeviceClass.MONETARY, # which Home Assistant documents as requiring an ISO 4217 currency code. "ct" # is not one, and the currency is not fixed anyway (awc selects the market), -# so the class is dropped rather than declared falsely (audit VA-15). +# so the class is dropped rather than declared falsely. _DEVICE_CLASS_EXCEPTIONS: dict[str, str | None] = {"awp": None} diff --git a/tests/test_api_auth.py b/tests/test_api_auth.py index 37989b3..0180205 100644 --- a/tests/test_api_auth.py +++ b/tests/test_api_auth.py @@ -51,7 +51,7 @@ def test_the_password_hash_keeps_its_value( hash_type: AuthHashType, expected: bytes ) -> None: """Without a value, a change to rounds, salt or encoding still hashed - deterministically and passed (audit A18-17). There is no vendor test + deterministically and passed. There is no vendor test vector; these come from the implementation, which logs in on a Home 11 J (PBKDF2) and a Flex (bcrypt).""" assert hash_password(PASSWORD, SERIAL, hash_type) == expected @@ -115,7 +115,7 @@ async def test_authentication_hashes_off_the_event_loop( ) -> None: """PBKDF2 with 100,000 rounds measured 110-116 ms per handshake. Home Assistant runs this client in its event loop, so every connection stalled - it for that long (audit A11-08).""" + it for that long.""" client = Wattpilot("192.0.2.10", "secret") client._device.serial = "123456" @@ -155,7 +155,7 @@ async def fake_send(message: dict[str, Any], **_kwargs: object) -> None: async def test_a_reconnect_reuses_the_password_hash( monkeypatch: pytest.MonkeyPatch, ) -> None: - """PBKDF2 took 110-116 ms per handshake (A11-08) for a result that only + """PBKDF2 took 110-116 ms per handshake for a result that only password, serial and hash type decide, so every reconnect delayed its auth reply by that much for nothing. A changed hash type must still re-hash.""" client = Wattpilot("192.0.2.10", PASSWORD) diff --git a/tests/test_api_client.py b/tests/test_api_client.py index fd5cd5a..1be777b 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -72,7 +72,7 @@ def respond(client: Wattpilot, request_id: int | str, *, success: bool) -> None: async def test_a_rejected_write_raises_instead_of_reporting_success() -> None: - """Audit VA-03: set_property returned as soon as the frame was on the + """set_property returned as soon as the frame was on the wire. A later `success: false` was only logged, so a switch the charger refused still showed as flipped.""" socket = FakeSocket() @@ -89,7 +89,7 @@ async def test_a_rejected_write_raises_instead_of_reporting_success() -> None: @pytest.mark.parametrize("success", [True, "true", 1]) async def test_an_acknowledgement_that_reads_true_is_accepted(success: object) -> None: - """Deliberately not only a JSON true (L-039): the Home sends one, the Flex + """Deliberately not only a JSON true: the Home sends one, the Flex was never measured, and refusing a real acceptance would fail every write.""" socket = FakeSocket() client = make_client(socket) @@ -159,7 +159,7 @@ async def test_disconnect_releases_a_waiting_command() -> None: async def test_a_failing_close_still_marks_the_client_disconnected() -> None: - """Audit VA-04: `connected` stayed True when ws.close() raised, while the + """`connected` stayed True when ws.close() raised, while the message loop behind it was already cancelled.""" socket = FakeSocket() socket.close_error = ConnectionResetError("close failed") @@ -173,7 +173,7 @@ async def test_a_failing_close_still_marks_the_client_disconnected() -> None: async def test_no_frame_payload_reaches_the_log( caplog: pytest.LogCaptureFixture, ) -> None: - """Audit VA-01, fixed at the source now that the client is ours: frames + """Fixed at the source now that the client is ours: frames carry Wi-Fi passwords, cloud tokens and OCPP keys, and DEBUG logging is one UI toggle away.""" socket = FakeSocket() @@ -308,8 +308,7 @@ def test_a_socket_error_is_described_without_the_chargers_address() -> None: def test_a_name_resolution_error_keeps_its_hint() -> None: """A resolver error quotes no address, and `gaierror (-2)` dropped the - one thing that says what to fix: the name did not resolve (reaudit - R1-18).""" + one thing that says what to fix: the name did not resolve.""" error = socket.gaierror(socket.EAI_NONAME, "Name or service not known") assert describe_error(error) == "gaierror (Name or service not known)" @@ -372,7 +371,7 @@ async def test_a_restart_does_not_wait_for_an_answer_that_never_comes() -> None: async def test_connect_loads_the_api_definition_off_the_event_loop() -> None: - """The definition is an 88 kB YAML file, and it used to be read on the + """The definition is a large YAML file, and it used to be read on the first write -- blocking the event loop at a moment nobody chose. Loading it while connecting, in a thread, removes both the blocking read and the warm-up the integration had to do on the client's behalf. @@ -412,7 +411,7 @@ def record_thread(**kwargs: object) -> object: async def test_an_acknowledgement_completes_whichever_id_type_arrives( request_id: int | str, ) -> None: - """Audit A11-01: commands go out with an integer id, but the bundled + """Commands go out with an integer id, but the bundled protocol documents responses carrying it as a string. Looking the waiter up unnormalized meant a string answer applied its properties and left the caller waiting for its full timeout -- a write that worked, reported as a diff --git a/tests/test_api_commands.py b/tests/test_api_commands.py index 0a5e96b..a368887 100644 --- a/tests/test_api_commands.py +++ b/tests/test_api_commands.py @@ -82,7 +82,7 @@ async def test_the_departure_is_written_unshifted_in_every_zone_mode( scheme and the process clock was in summer time. Measured on the real charger on 2026-09-06, during summer time and with `tds` = 1: a departure set to 07:30 in the Wattpilot app reads back as 27000, not 30600. The - firmware applies no offset, so neither do we (audit A12-06). + firmware applies no offset, so neither do we. Parametrized over every `tds` value because that announcement is what the old branch keyed on. There is no clock to pin any more: the write is @@ -119,9 +119,9 @@ async def test_enable_cloud_api_gives_up_within_the_timeout_it_was_given( client: Wattpilot, ) -> None: """The wait counted whole one-second sleeps, so a 10 ms timeout still - slept a full second before giving up (audit A13-04).""" - # The first write loads the API definition from disk; that one-off cost - # is not the wait being measured here. + slept a full second before giving up.""" + # A first write in the process loads the API definition from disk; that + # one-off cost is not the wait being measured here. await client.set_property("cae", False) # noqa: FBT003 -- a written value started = time.monotonic() with pytest.raises(WattpilotConnectionError, match="Timeout"): diff --git a/tests/test_api_definition.py b/tests/test_api_definition.py index 0fcb528..f3f64bd 100644 --- a/tests/test_api_definition.py +++ b/tests/test_api_definition.py @@ -48,3 +48,9 @@ def test_the_shipped_definition_loads() -> None: definition = load_api_definition() assert definition.properties["nrg"]["jsonType"] == "array" assert definition.properties["amp"]["jsonType"] == "integer" + + +def test_the_definition_is_read_once_per_process() -> None: + """Every client parsed the large YAML file again, a setup probe included; + nothing changes it at runtime, and its readers only read.""" + assert load_api_definition() is load_api_definition() diff --git a/tests/test_api_lifecycle.py b/tests/test_api_lifecycle.py index 3828642..ffde1bc 100644 --- a/tests/test_api_lifecycle.py +++ b/tests/test_api_lifecycle.py @@ -11,16 +11,21 @@ import contextlib import errno import json +import logging +import re import time +from pathlib import Path from typing import TYPE_CHECKING, Any import pytest +from custom_components.wattpilot.api import client as client_module from custom_components.wattpilot.api.client import Wattpilot from custom_components.wattpilot.api.exceptions import ( DeviceIdentityError, PropertyError, WattpilotConnectionError, + WattpilotError, ) if TYPE_CHECKING: @@ -159,7 +164,7 @@ async def test_a_malformed_frame_does_not_kill_the_message_loop( """One bad frame must not end the loop -- the charger would still be connected, and nothing would ever update again. The predecessor of this test called the handler twice and never the loop, so it proved nothing - about the claim in its own name (audit A11-06).""" + about the claim in its own name.""" client._connection.socket = FakeSocket( # type: ignore[assignment] ["{not json", json.dumps({"type": "deltaStatus", "status": {"amp": 6}})] ) @@ -178,7 +183,7 @@ async def test_disconnect_closes_the_socket_after_the_loop_failed( client: Wattpilot, ) -> None: """Whatever ended the message loop, the socket still has to be closed -- - re-raising the loop's exception first left it open (audit A11-06).""" + re-raising the loop's exception first left it open.""" socket = FakeSocket([]) client._connection.socket = socket # type: ignore[assignment] @@ -200,7 +205,7 @@ async def test_a_cancelled_connect_leaves_nothing_running( ) -> None: """Home Assistant cancels a setup that takes too long. The cleanup used to catch Exception, which CancelledError is not, so socket and message loop - survived the cancelled attempt (audit A11-05).""" + survived the cancelled attempt.""" socket = FakeSocket([]) async def fake_connect(_url: str) -> FakeSocket: @@ -221,19 +226,6 @@ async def fake_connect(_url: str) -> FakeSocket: assert client._connection.message_loop_task is None -async def test_message_subscribers_see_every_frame(client: Wattpilot) -> None: - # Subscribers get the decoded frame as a plain dict, before the client - # turns it into a namespace for its own handlers. - seen: list[Any] = [] - unsubscribe = client.on_message(lambda message: seen.append(message["type"])) - - await client._handle_message(json.dumps({"type": "deltaStatus", "status": {}})) - unsubscribe() - await client._handle_message(json.dumps({"type": "deltaStatus", "status": {}})) - - assert seen == ["deltaStatus"] - - async def test_a_firmware_install_without_a_version_needs_an_offer( client: Wattpilot, ) -> None: @@ -248,7 +240,7 @@ async def test_a_reconnect_to_a_different_charger_is_refused( ) -> None: """Setup checks the serial once, but the address can be reused by DHCP or the hardware replaced. A reconnect kept the config entry, its entities and - their history pointed at whatever answered (audit A11-02).""" + their history pointed at whatever answered.""" client._device.serial = "111111" socket = FakeSocket( [ @@ -279,7 +271,7 @@ async def test_an_explicit_reconnect_waits_for_the_new_snapshot( ) -> None: """The explicit path carried _all_props_initialized over from the previous connection, so the first partial replay satisfied readiness and connect() - returned before the new snapshot arrived (audit A11-07).""" + returned before the new snapshot arrived.""" client._connection.mark_initialized() socket = FakeSocket( [ @@ -307,8 +299,7 @@ async def test_an_explicit_reconnect_to_a_different_charger_is_refused( ) -> None: """The service-driven path has to refuse the same way the automatic one does -- the check sits in the frame both of them send, so this is the - counter-question to the automatic case rather than a second mechanism - (audit A11-02).""" + counter-question to the automatic case rather than a second mechanism.""" client._device.serial = "111111" socket = FakeSocket([json.dumps({"type": "hello", "serial": "222222"})]) @@ -357,7 +348,7 @@ async def test_an_explicit_reconnect_during_backoff_leaves_one_reader( """A reconnect action while the client is waiting out its retry backoff must not leave the old reader running. It still owns a socket, and the two then race on the same connection object: measured three sockets and - two readers, two of them still open after disconnect (audit A12-01).""" + two readers, two of them still open after disconnect.""" factory = SocketFactory() monkeypatch.setattr( "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", @@ -389,7 +380,7 @@ async def test_two_explicit_opens_leave_no_socket_behind( """Two reconnect actions arriving together: each passed the "already connected" check, each opened a socket, and the second overwrote the references to the first. close() then reached only the last pair, and - the first socket and its reader lived on unowned (audit A13-01).""" + the first socket and its reader lived on unowned.""" factory = SocketFactory() monkeypatch.setattr( "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", @@ -413,7 +404,7 @@ async def test_a_close_during_a_pending_open_stays_closed( """close() ran while open() was still waiting for the connector, found neither socket nor reader, and returned. The connector then delivered, open() installed socket and reader, and a connection that had just been - torn down was live again behind the caller's back (audit A14-01).""" + torn down was live again behind the caller's back.""" factory = SocketFactory() release = asyncio.Event() @@ -453,7 +444,7 @@ async def test_an_open_queued_before_a_close_does_not_outlive_it( """Two opens, then a close, all requested before any connector returned. The first open noticed the close; the second one, queued behind it, only looked afterwards and opened a live connection after close() had - returned (audit A15-01).""" + returned.""" factory = SocketFactory() release = asyncio.Event() entered = asyncio.Event() @@ -521,7 +512,7 @@ async def test_an_open_during_the_cancel_step_of_a_close_stays_open( ) -> None: """close() cancelled the pending attempt and only then made itself visible. An open() in that gap started on its own, succeeded -- and was - torn down by the close it had come after (audit A16-01).""" + torn down by the close it had come after.""" connection = client._connection entered = asyncio.Event() calls: list[str] = [] @@ -615,7 +606,7 @@ async def test_an_open_during_a_slow_close_waits_for_a_new_connection( ) -> None: """While close() waited for the socket's close handshake, the connection still counted as connected -- so an open() in that window returned at - once, reporting a connection the close then took down (audit A15-01).""" + once, reporting a connection the close then took down.""" factory = SocketFactory() monkeypatch.setattr( "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", @@ -651,7 +642,7 @@ async def test_a_silent_socket_after_reconnect_is_given_up_on( ) -> None: """The explicit path bounds the handshake; the automatic retry did not. A peer whose socket stays open but never finishes the application - handshake held the integration unavailable for good (audit A12-04).""" + handshake held the integration unavailable for good.""" factory = SocketFactory() monkeypatch.setattr( "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", @@ -686,7 +677,7 @@ class BufferedSocket(FakeSocket): Real WebSockets do exactly that: closing does not discard frames that already arrived. FakeSocket ends its iteration on close, and that - difference hid a defect (audit A12-02). + difference hid a defect. """ async def __anext__(self) -> str: @@ -703,7 +694,7 @@ async def test_no_frame_is_applied_after_the_charger_was_refused( ) -> None: """Refusing the identity closes the socket, but frames already in the receive buffer still arrive. They were applied: amp=14 from the wrong - charger landed in the cache after the rejection (audit A12-02).""" + charger landed in the cache after the rejection.""" client._device.serial = "111111" socket = BufferedSocket( [ @@ -731,7 +722,7 @@ async def test_a_cancelled_definition_load_hands_the_connection_back( ) -> None: """connect() is handshake plus definition load. A cancellation between the two fell outside the transport's cleanup scope and outside the - integration's, leaving reader and socket alive (audit A12-03).""" + integration's, leaving reader and socket alive.""" socket = FakeSocket( [ json.dumps({"type": "authSuccess"}), @@ -772,7 +763,7 @@ async def test_the_firmware_wait_keeps_the_budget_it_was_given( ) -> None: """The reconnect loop counted only its own sleeps, so every attempt could burn a full authentication timeout for free -- with the default budget - roughly half an hour past the two minutes asked for (audit A12-08). + roughly half an hour past the two minutes asked for. """ client._all_props["onv"] = ["42.6"] @@ -783,7 +774,7 @@ async def slow_and_failing() -> None: monkeypatch.setattr(client, "set_property", lambda *_a, **_k: asyncio.sleep(0)) monkeypatch.setattr(client, "connect", slow_and_failing) - # The timeout hands recovery back to the reconnect loop (A18-06); park it. + # The timeout hands recovery back to the reconnect loop; park it. client._connection._reconnect_delay_min = 3600 started = time.monotonic() @@ -799,7 +790,7 @@ async def test_a_binary_frame_that_is_not_utf8_does_not_end_the_reader( client: Wattpilot, ) -> None: """The frame was decoded before the frame guard, so a byte sequence that - is not UTF-8 ended the reader instead of being skipped (audit A19-08).""" + is not UTF-8 ended the reader instead of being skipped.""" client._connection.socket = FakeSocket( # type: ignore[assignment] [bytes([0xFF, 0xFE]), json.dumps({"type": "deltaStatus", "status": {"amp": 6}})] # type: ignore[list-item] ) @@ -818,7 +809,7 @@ async def test_an_empty_array_does_not_end_the_reader(client: Wattpilot) -> None """`nrg` arrives as a list the client indexes into. An empty one raised IndexError, which the frame guard did not catch -- so the reader stopped and the valid frame behind it was never read, while the socket still - looked alive (audit 12.4, the remainder of A11-06).""" + looked alive.""" client._connection.socket = FakeSocket( # type: ignore[assignment] [ json.dumps({"type": "deltaStatus", "status": {"nrg": []}}), @@ -859,7 +850,7 @@ async def test_a_failed_reconnect_action_during_backoff_keeps_recovering( """The reconnect action stopped the loop that was waiting out its backoff before its own attempt had a socket. When that attempt failed, nothing was left to retry: the charger came back to an integration that never - reached it again (audit A18-05).""" + reached it again.""" connector = RefusingConnector() monkeypatch.setattr(CONNECT, connector) connection = client._connection @@ -886,8 +877,7 @@ async def test_a_firmware_update_outlasting_its_budget_keeps_recovering( ) -> None: """The update closes the connection to reconnect by itself. When the charger took longer than the budget, the action failed and left the - client closed for good -- the updated charger was never reached again - (audit A18-06).""" + client closed for good -- the updated charger was never reached again.""" monkeypatch.setattr(CONNECT, RefusingConnector()) client._all_props["onv"] = ["42.6"] monkeypatch.setattr(client, "set_property", lambda *_a, **_k: asyncio.sleep(0)) @@ -906,7 +896,7 @@ async def test_a_refusal_ends_the_firmware_wait_with_its_reason( client: Wattpilot, monkeypatch: pytest.MonkeyPatch ) -> None: """Retrying cannot help against a refusal. The wait went on until its - deadline and reported a timeout, never the reason (reaudit R1-13).""" + deadline and reported a timeout, never the reason.""" async def refused() -> None: client._connection.fail(WattpilotConnectionError("Unsupported password hash")) @@ -926,8 +916,7 @@ async def test_a_firmware_wait_with_no_time_for_an_attempt_keeps_recovering( ) -> None: """The charger dropped the socket only as the budget ran out, so no attempt ran. The update's own disconnect still counted as an unload, and - the hand-back at the deadline was refused: nothing retried (post-check - PC-01).""" + the hand-back at the deadline was refused: nothing retried.""" monkeypatch.setattr(CONNECT, RefusingConnector()) client._all_props["onv"] = ["42.6"] monkeypatch.setattr(client, "set_property", lambda *_a, **_k: asyncio.sleep(0)) @@ -951,7 +940,7 @@ async def test_a_close_before_the_attempt_runs_is_not_undone( ) -> None: """The attempt cleared the stop flag as its first line, so a close that landed between open() starting the attempt and the attempt running was - undone, and a later hand-back reconnected after an unload (PC-01).""" + undone, and a later hand-back reconnected after an unload.""" release = asyncio.Event() async def blocked_connect(_url: str) -> FakeSocket: @@ -988,7 +977,7 @@ async def test_a_close_during_the_firmware_wait_is_not_overridden( ) -> None: """The update reconnects by itself and, at its deadline, handed recovery back to a reconnect loop -- even when the entry had been unloaded in the - meantime, so a session outlived its integration (reaudit R1-11).""" + meantime, so a session outlived its integration.""" connector = RefusingConnector() monkeypatch.setattr(CONNECT, connector) client._all_props["onv"] = ["42.6"] @@ -1036,8 +1025,8 @@ async def blocked_connect(_url: str) -> FakeSocket: async def test_a_cancelled_reconnect_action_keeps_recovering( client: Wattpilot, monkeypatch: pytest.MonkeyPatch ) -> None: - """Sibling of A18-05: an action cancelled while connecting (an automation - restarted) stopped the loop that was retrying (reaudit R1-12).""" + """An action cancelled while connecting (an automation + restarted) stopped the loop that was retrying.""" connecting = asyncio.Event() @@ -1067,7 +1056,7 @@ async def test_an_open_into_a_stalled_handshake_keeps_recovering( client: Wattpilot, monkeypatch: pytest.MonkeyPatch ) -> None: """The reconnect had authenticated but the full status stalled; an open - then timed out and closed the only loop (reaudit R1-12).""" + then timed out and closed the only loop.""" monkeypatch.setattr(CONNECT, RefusingConnector()) connection = client._connection connection._reconnect_delay_min = 3600 @@ -1085,10 +1074,10 @@ async def test_an_open_into_a_stalled_handshake_keeps_recovering( await connection.close() -# ---- One rule over every way in and every settled state (ledger L-021) ---- +# ---- One rule over every way in and every settled state ---- # # The single interleaving tests above each came from a finding, and two of the -# fixes made the next finding (R1-11, PC-01). This matrix states the rule once: +# fixes made the next finding. This matrix states the rule once: # a deliberate close leaves nothing running; any other way in leaves recovery # as it found it -- a reconnect loop that existed still exists, none is made # up -- and never two readers or a socket without a reader. An open after a @@ -1228,7 +1217,7 @@ async def test_a_password_refused_after_sign_in_drops_the_connection( ) -> None: """An authError after authSuccess set the refusal but left the socket open and the client signed in until the next frame: the entities stayed - available while the reauthentication started (post-check PC2-02).""" + available while the reauthentication started.""" socket = FakeSocket( [ json.dumps({"type": "authSuccess"}), @@ -1253,7 +1242,7 @@ async def test_a_refused_connection_stops_counting_as_connected_at_once( ) -> None: """The refusal closed the socket, but the close waits for the charger's reply -- up to ten seconds -- and the client counted as signed in all - that time (post-check PC3-01).""" + that time.""" release = asyncio.Event() socket = SlowClosingSocket(release) connection = client._connection @@ -1274,8 +1263,7 @@ async def test_a_refusal_during_the_full_status_is_reported_as_itself( client: Wattpilot, monkeypatch: pytest.MonkeyPatch ) -> None: """A refusal after authSuccess did not end the wait for the full status: - setup reported a timeout and retried, and never asked for the password - (post-check PC3-02).""" + setup reported a timeout and retried, and never asked for the password.""" from custom_components.wattpilot.api.exceptions import ( # noqa: PLC0415 AuthenticationError, ) @@ -1304,7 +1292,7 @@ async def test_a_refusal_ends_the_wait_of_an_open_into_a_signed_in_reconnect( ) -> None: """The other branch of open(): the reconnect loop is signed in and the full status pending, as when the reconnect action or the firmware wait - opens (post-check PC4-01, sibling of PC3-02).""" + opens.""" from custom_components.wattpilot.api.exceptions import ( # noqa: PLC0415 AuthenticationError, ) @@ -1331,8 +1319,7 @@ async def test_a_close_while_an_open_stops_the_old_reader_is_not_swallowed( ) -> None: """The open first stops the reader in its backoff. A close landing there was taken for that reader's own cancellation: the attempt went on to - open a socket after the close, and the close waited for it (reaudit - RF-L-02).""" + open a socket after the close, and the close waited for it.""" connector = RefusingConnector() monkeypatch.setattr(CONNECT, connector) connection = client._connection @@ -1358,8 +1345,8 @@ async def test_an_unload_while_the_firmware_update_waits_for_the_reboot_wins( ) -> None: """The close itself ended the wait for the reboot, and the close count was taken only after it: the update reconnected and reported success - behind an unloaded entry (reaudit RF-L-01; R1-11 covered the reconnect - phase only).""" + behind an unloaded entry; an earlier fix covered the reconnect phase + only.""" sockets: list[FakeSocket] = [] async def signs_in(_url: str) -> FakeSocket: @@ -1446,3 +1433,282 @@ async def test_a_frame_that_fails_is_skipped_and_the_reader_goes_on( task.cancel() with contextlib.suppress(asyncio.CancelledError): await task + + +def _api_records(caplog: pytest.LogCaptureFixture, level: int) -> list[str]: + return [ + f"{record.levelname} {record.getMessage()}" + for record in caplog.records + if record.name.startswith("custom_components.wattpilot.api") + and record.levelno >= level + ] + + +def _serve(monkeypatch: pytest.MonkeyPatch, socket: FakeSocket) -> None: + async def fake_connect(_url: str) -> FakeSocket: + return socket + + monkeypatch.setattr( + "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", + fake_connect, + ) + + +async def test_a_connect_says_nothing_above_debug( + client: Wattpilot, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Every setup, reload and form probe connects. Availability is the hub's + to report, once per outage; an INFO here repeated it on each of them.""" + status = {"type": "fullStatus", "partial": False, "status": {"amp": 6}} + _serve( + monkeypatch, + FakeSocket([json.dumps({"type": "authSuccess"}), json.dumps(status)]), + ) + await client.connect() + await client.disconnect() + assert _api_records(caplog, logging.INFO) == [] + + +@pytest.mark.parametrize( + "frame", + [ + {"type": "authError", "message": "Wrong password"}, + {"type": "hello", "serial": "222222"}, + ], +) +async def test_a_refusal_is_not_logged_as_an_error( + client: Wattpilot, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + frame: dict[str, Any], +) -> None: + """The caller reports a refusal: the form shows it, setup raises it, and + at runtime the hub opens reauth or a repair notice. Two ERROR lines of + the client's own came on top, for every mistyped password in the form.""" + client._device.serial = "111111" + socket = FakeSocket([json.dumps(frame)]) + _serve(monkeypatch, socket) + client._connection.connect_timeout = 0.5 + with pytest.raises(WattpilotError): + await client.connect() + await _settle(lambda: client._connection.message_loop_task is None) + assert _api_records(caplog, logging.WARNING) == [] + + +def _nested(depth: int) -> object: + value: object = 6 + for _ in range(depth): + value = [value] + return value + + +def _failing_callback(error: type[Exception]) -> Callable[[str, Any], None]: + def callback(key: str, _value: Any) -> None: + if key == "amp": + raise error(key) + + return callback + + +# Each way a defect can break every frame: the frame itself, a property +# callback, and a value nested too deep to apply. +REPEATED_FAILURES = { + "frame": (["{not json"], None), + "callback TypeError": ([{"amp": 6}], TypeError), + "callback KeyError": ([{"amp": 6}], KeyError), + "nested value": ([{"amp": _nested(1200)}], None), +} + + +def _repeating(statuses: list[Any], times: int) -> list[str]: + return [ + status + if isinstance(status, str) + else json.dumps({"type": "deltaStatus", "status": status}) + for _ in range(times) + for status in statuses + ] + + +@pytest.mark.parametrize("failure", list(REPEATED_FAILURES)) +async def test_a_defect_in_every_frame_is_warned_about_once( + client: Wattpilot, caplog: pytest.LogCaptureFixture, failure: str +) -> None: + """A defect that breaks every frame broke one about every second, and + each one logged a WARNING without saying where it failed. A defect in + applying a value blamed the charger for it: "an unreadable value".""" + statuses, callback_error = REPEATED_FAILURES[failure] + if callback_error is not None: + client.on_property_change(_failing_callback(callback_error)) + done = json.dumps({"type": "deltaStatus", "status": {"nrg": [1]}}) + client._connection.socket = FakeSocket([*_repeating(statuses, 5), done]) # type: ignore[assignment] + task = asyncio.create_task(client._connection._message_loop()) + try: + await _settle(lambda: client.all_properties.get("nrg") == [1]) + finally: + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + warnings = _api_records(caplog, logging.WARNING) + assert len(warnings) == 1, warnings + # The innermost line of the integration's own, by file name only: a + # full path names the user's machine. + assert re.fullmatch(r"WARNING Skipping .+: \w+ at [a-z_]+\.py:\d+", warnings[0]), ( + warnings[0] + ) + + +@pytest.mark.parametrize( + "frame", + [ + {"type": "deltaStatus"}, + {"type": "fullStatus", "partial": False}, + {"type": "response", "requestId": 1, "success": True}, + ], + ids=["delta", "full", "response"], +) +@pytest.mark.parametrize( + "error", + [ValueError, TypeError, KeyError, IndexError, AttributeError, RecursionError], +) +async def test_a_failing_value_costs_only_itself( + client: Wattpilot, + caplog: pytest.LogCaptureFixture, + error: type[Exception], + frame: dict[str, Any], +) -> None: + """A failure in one value cost the rest of its frame, and in the last + chunk of the first full status every entity stayed unavailable. The + frame's own catch would hide that: it warns just the same.""" + client.on_property_change(_failing_callback(error)) + status = {"amp": 6, "lmo": 3} + socket = BufferedSocket([json.dumps({**frame, "status": status})]) + await socket.close() # hands over what it holds, then ends + client._connection.socket = socket # type: ignore[assignment] + await client._connection._read_frames() + assert client.all_properties.get("lmo") == 3, "the neighbour was lost" + (warning,) = _api_records(caplog, logging.WARNING) + assert "Skipping the value of amp:" in warning, warning + if frame["type"] == "fullStatus": + assert client._connection.initialized, "the status never completed" + + +async def test_a_repeated_defect_is_warned_about_again_on_a_new_connection( + client: Wattpilot, caplog: pytest.LogCaptureFixture +) -> None: + """Once per connection, not once for good: a defect still there after + a reconnect would otherwise never be seen again.""" + for _ in range(2): + client._connection.begin() + socket = BufferedSocket(["{not json"] * 3) + await socket.close() # hands over what it holds, then ends + client._connection.socket = socket # type: ignore[assignment] + await client._connection._read_frames() + assert len(_api_records(caplog, logging.WARNING)) == 2 + + +async def test_the_reported_line_is_the_innermost_one_of_the_integration( + client: Wattpilot, caplog: pytest.LogCaptureFixture +) -> None: + """Not the line that caught the error, which is the same for every + defect, but the one closest to it: here the call into the callback.""" + client.on_property_change(_failing_callback(TypeError)) + frame = json.dumps({"type": "deltaStatus", "status": {"amp": 6}}) + socket = BufferedSocket([frame]) + await socket.close() # hands over what it holds, then ends + client._connection.socket = socket # type: ignore[assignment] + await client._connection._read_frames() + (warning,) = _api_records(caplog, logging.WARNING) + found = re.search(r"(\w+\.py):(\d+)$", warning) + assert found, warning + source = Path(client_module.__file__).with_name(found[1]).read_text("utf-8") + line = source.splitlines()[int(found[2]) - 1] + assert "(name, value)" in line, f"{warning}: {line.strip()}" + + +@pytest.mark.parametrize("peer", ["silent", "closing"]) +async def test_a_charger_that_never_gets_ready_is_retried_ever_more_slowly( + client: Wattpilot, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + peer: str, +) -> None: + """A socket that connects but never brings the snapshot -- silent, or + closed straight away -- is no sign that the charger is back. Counting it + as one put the backoff back to its minimum every cycle, and each silent + one logged a WARNING: about one a minute while the charger stayed so. + Read from the state, not the clock: timing ran slow under mutation + testing.""" + attempts: list[FakeSocket] = [] + + async def connect(_url: str) -> FakeSocket: + await asyncio.sleep(0) + socket = FakeSocket([]) + attempts.append(socket) + if peer == "closing": + await socket.close() + return socket + + monkeypatch.setattr( + "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", + connect, + ) + connection = client._connection + connection._reconnect_delay_min = 0.01 + connection.connect_timeout = connection.init_timeout = 0.01 + connection.socket = FakeSocket([]) + connection.message_loop_task = asyncio.create_task(connection._message_loop()) + await connection.socket.close() + for _ in range(300): + if len(attempts) >= 6: + break + await asyncio.sleep(0.01) + delay = connection._reconnect_delay + await connection.close() + + assert len(attempts) >= 6, f"{len(attempts)} attempt(s)" + # Doubled before each of the six attempts, never put back. + assert delay >= 0.01 * 2**6, f"the backoff did not grow: {delay}" + assert _api_records(caplog, logging.WARNING) == [] + + +async def test_the_backoff_starts_over_once_the_charger_was_back( + client: Wattpilot, monkeypatch: pytest.MonkeyPatch +) -> None: + """The counter-question to the growing backoff: a connection that got + ready proves the charger is back, and its next drop is retried at the + shortest delay again, not after the one the outage had grown to. Read + from the state, not the clock: timing ran slow under mutation testing.""" + ready = [ + json.dumps({"type": "authSuccess"}), + json.dumps({"type": "fullStatus", "partial": False, "status": {"amp": 6}}), + ] + sockets: list[FakeSocket] = [] + + async def connect(_url: str) -> FakeSocket: + await asyncio.sleep(0) + sockets.append(FakeSocket(ready if len(sockets) == 4 else [])) + return sockets[-1] + + monkeypatch.setattr( + "custom_components.wattpilot.api.connection.websockets.asyncio.client.connect", + connect, + ) + connection = client._connection + connection._reconnect_delay_min = 0.01 + connection.connect_timeout = connection.init_timeout = 0.01 + connection.socket = FakeSocket([]) + connection.message_loop_task = asyncio.create_task(connection._message_loop()) + await connection.socket.close() + for _ in range(300): + if connection.initialized and len(sockets) == 5: + break + await asyncio.sleep(0.01) + grown = len(sockets) == 5 and connection.initialized + delay = connection._reconnect_delay + await connection.close() + + assert grown, f"never got ready: {len(sockets)} attempt(s)" + assert delay == 0.01, f"the grown delay outlived the recovery: {delay}" diff --git a/tests/test_api_messages.py b/tests/test_api_messages.py index 343e1bb..9cadbae 100644 --- a/tests/test_api_messages.py +++ b/tests/test_api_messages.py @@ -168,7 +168,7 @@ async def test_a_malformed_property_costs_neither_its_siblings_nor_the_status( ) -> None: """One value the client cannot read ended the whole frame: the keys after it were lost, and in the last chunk of the first full status the status - never completed, so connecting ran into its timeout (audit A18-16).""" + never completed, so connecting ran into its timeout.""" await send( client, { @@ -185,7 +185,7 @@ async def test_an_acceptance_with_a_null_status_answers_the_command( client: Wattpilot, ) -> None: """`status: null` raised before the waiting command was answered, so a - write the charger had accepted was reported as unanswered (A18-19).""" + write the charger had accepted was reported as unanswered.""" waiter = asyncio.get_running_loop().create_future() client._pending_commands["7"] = waiter await send( @@ -197,7 +197,7 @@ async def test_an_acceptance_with_a_null_status_answers_the_command( async def test_an_unknown_password_hash_is_a_named_refusal(client: Wattpilot) -> None: """An algorithm this client does not know made the frame unreadable: no - answer was sent and setup timed out without naming the cause (A18-18).""" + answer was sent and setup timed out without naming the cause.""" await send( client, {"type": "authRequired", "token1": "a", "token2": "b", "hash": "argon2"} ) @@ -207,7 +207,7 @@ async def test_an_unknown_password_hash_is_a_named_refusal(client: Wattpilot) -> async def test_an_unknown_password_hash_drops_the_socket(client: Wattpilot) -> None: """Left open, the refused socket counted as connected until the readiness - guard gave up on it, as a wrong charger's does not (reaudit R1-13).""" + guard gave up on it, as a wrong charger's does not.""" class Socket: closed = False @@ -228,7 +228,7 @@ async def test_an_unreadable_full_status_does_not_complete_the_snapshot( ) -> None: """A last chunk whose status is no object completed the snapshot: setup then built its entities without the properties, and later deltas never - added them (branch audit 2026-10-11, finding 6; new in 44fa329).""" + added them.""" await send(client, {"type": "fullStatus", "partial": True, "status": {"fwv": "1"}}) with contextlib.suppress(TypeError): await send(client, {"type": "fullStatus", "partial": False, "status": status}) diff --git a/tests/test_api_properties.py b/tests/test_api_properties.py index 6b917d4..97e0399 100644 --- a/tests/test_api_properties.py +++ b/tests/test_api_properties.py @@ -72,7 +72,6 @@ async def test_property_callbacks_see_every_change(client: Wattpilot) -> None: ("float", "1.5", 1.5), ("float", 2, 2.0), ("string", "x", "x"), - ("string", 5, "5"), ] @@ -97,6 +96,7 @@ def test_a_value_is_coerced_to_the_type_the_charger_declares( ("integer", 5.7), ("integer", "5.7"), ("integer", float("inf")), + ("string", 5), ], ) def test_a_value_that_cannot_be_coerced_is_refused( @@ -123,3 +123,10 @@ def test_an_unknown_json_type_passes_the_value_through(client: Wattpilot) -> Non """The definition file may name a type this client does not know; that is not a reason to refuse the write.""" assert client._coerce_to_json_type("x", "something-new", "amp") == "x" + + +def test_one_property_reads_like_all_properties_does(client: Wattpilot) -> None: + """all_properties hands out a copy; one value needs none.""" + client._update_property("amp", 10) + assert client.get_property("amp") == 10 + assert client.get_property("zzz", 6) == 6 diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 4634df8..731bb32 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -27,7 +27,7 @@ DOMAIN, ) -from .conftest import FakeWattpilot +from .conftest import FakeWattpilot, patch_charger if TYPE_CHECKING: from homeassistant.core import HomeAssistant @@ -38,10 +38,6 @@ ) -def patch_charger(charger: FakeWattpilot) -> Any: - return patch("custom_components.wattpilot.hub.Wattpilot", return_value=charger) - - async def start_user_flow(hass: HomeAssistant) -> Any: return await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -123,7 +119,7 @@ async def test_a_malformed_host_is_a_form_error( ) -> None: """An unbracketed IPv6 address, a mistyped port or an empty label made building the address raise outside the connection errors: the form said - "Unknown error" and the log held a traceback (audit A18-21).""" + "Unknown error" and the log held a traceback.""" result = await start_user_flow(hass) fake_charger.connect_error = error with patch_charger(fake_charger): @@ -176,7 +172,7 @@ async def test_reauth_rejects_a_different_charger( accepted by whatever now answers at the address were written into the entry and the flow reported success -- while the setup that followed refused the device, leaving the entry broken with its password already - replaced (audit A12-07). Sibling of the setup and reconnect checks. + replaced. Sibling of the setup and reconnect checks. """ entry = MockConfigEntry( domain=DOMAIN, @@ -225,6 +221,45 @@ async def test_reconfigure_rejects_wrong_device( assert entry.unique_id == original_unique_id +@pytest.mark.parametrize( + ("start", "user_input"), + [ + ("start_reauth_flow", {"password": "other"}), + ("start_reconfigure_flow", {**USER_INPUT, "password": "other"}), + ], +) +async def test_another_charger_with_another_password_is_the_wrong_device( + hass: HomeAssistant, + device_properties: dict[str, Any], + start: str, + user_input: dict[str, str], +) -> None: + """ + The charger answering is another one, and its password differs too. + + The client checks a serial it is given before it answers the password + challenge, the way setup builds it. The flows built theirs without one, + so this reached the password check first and told the user their + password was wrong. + """ + entry = MockConfigEntry( + domain=DOMAIN, + unique_id="999999", # a different charger than the fake (123456) + version=2, + data={CONF_CONNECTION_TYPE: CONNECTION_LOCAL, **USER_INPUT}, + ) + entry.add_to_hass(hass) + charger = FakeWattpilot(device_properties) + charger.connect_error = AuthenticationError("Wrong password") + + result = await getattr(entry, start)(hass) + with patch_charger(charger): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input + ) + assert result["errors"] == {"base": "wrong_device"} + + async def test_reconfigure_rescues_a_fork_entry_whose_address_changed( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: @@ -519,7 +554,7 @@ async def test_a_known_charger_announcing_a_new_address_updates_the_entry( hass: HomeAssistant, ) -> None: """DHCP moved the charger: the entry follows instead of trying the old - address -- where another device may answer by now (audit A15-04).""" + address -- where another device may answer by now.""" entry = MockConfigEntry( domain=DOMAIN, unique_id="123456", @@ -692,8 +727,8 @@ async def test_a_card_for_a_fork_entry_awaiting_its_serial_is_not_offered( ) -> None: """A fork entry is keyed on its address until it connects. The card for the charger at that address was offered anyway, and confirming it made a - second entry with the serial that locked the fork entry out (reaudit - R1-14). The charger just announced itself, so the entry tries again.""" + second entry with the serial that locked the fork entry out. The + charger just announced itself, so the entry tries again.""" entry = MockConfigEntry( domain=DOMAIN, unique_id="192.168.1.60", @@ -720,7 +755,7 @@ async def test_adding_by_hand_a_charger_whose_entry_awaits_its_serial_is_refused hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """The same as the card, through the form: a second entry with the serial - locked the carried-over entry out (post-check PC-04).""" + locked the carried-over entry out.""" entry = MockConfigEntry( domain=DOMAIN, unique_id=USER_INPUT["host"], diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index efac16f..fbcab3e 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -50,7 +50,7 @@ async def test_every_entry_field_has_a_redaction_decision( hass: HomeAssistant, ) -> None: """A fork entry gained a stored name, which went out in the clear while - the same name was replaced among the properties (README audit RM2-01). + the same name was replaced among the properties. The shapes come from the writers themselves: the config flow's and the migration of both fork entry kinds.""" shapes = [{**V2_LOCAL_DATA, CONF_NAME: "Wattpilot"}, V2_AWAITING_SERIAL] @@ -215,7 +215,7 @@ async def test_diagnostics_redact_sensitive_data( def test_time_server_host_is_not_exposed() -> None: - """Audit VA-07: `ts` is the configured NTP server. A public pool name is + """`ts` is the configured NTP server. A public pool name is harmless, an internal hostname or private IP is not, and the redaction cannot tell them apart -- so it replaces the string. @@ -231,7 +231,7 @@ def test_time_server_host_is_not_exposed() -> None: def test_unexpected_card_and_companion_shapes_are_safe() -> None: - """Audit VA-12: the redaction is written for the shapes firmware 42.5 + """The redaction is written for the shapes firmware 42.5 sends. A different one must fail safe, not either way it did: - a card that is not a dict passed through untouched, so a bare string @@ -248,7 +248,7 @@ def test_unexpected_card_and_companion_shapes_are_safe() -> None: # The same boundary one level up: the outer `cards` value was iterated # without a check, so None or a number raised and a string was walked - # character by character (audit A13-05). + # character by character. for shape in (None, "OWNER-CARD-1234", 42, {}): cleaned = sanitize_snapshot({"cards": shape}) assert cleaned["cards"] == UNEXPECTED_SHAPE @@ -310,7 +310,7 @@ def test_a_flex_on_ethernet_keeps_no_identifier_in_the_diagnostics() -> None: def test_the_serial_is_replaced_also_where_sse_is_not_in_the_map() -> None: """A delta or a later chunk of the full status carries no `sse`, so a - recording kept the serial a new key embedded (R1-09).""" + recording kept the serial a new key embedded.""" sanitized = sanitize_snapshot({"newKey": "Wattpilot-91234567"}, serial="91234567") assert "91234567" not in json.dumps(sanitized) @@ -353,7 +353,7 @@ def test_text_that_only_looks_like_an_address_is_kept(value: str) -> None: ) def test_an_address_at_the_edge_of_a_text_is_replaced(text: str, address: str) -> None: """A full stop after an IPv4 address, or a label glued to an IPv6 one, - kept the address out of the net (R1-07).""" + kept the address out of the net.""" assert address not in sanitize_snapshot({"unknownKey": text})["unknownKey"] @@ -362,7 +362,7 @@ def test_an_empty_value_is_not_replaced_by_a_real_looking_one( empty: str | None, ) -> None: """An empty car profile became "default", a value the charger had not - sent (audit A18-28). An empty value names nobody; it stays as it is.""" + sent. An empty value names nobody; it stays as it is.""" assert sanitize_snapshot({"ct": empty, "sse": empty}) == {"ct": empty, "sse": empty} @@ -372,7 +372,7 @@ def test_an_empty_time_server_companion_or_chip_name_stays_empty( ) -> None: """The same rule on the paths with their own stand-ins: an unset time server became a configured one, an unpaired inverter a named device and - an unnamed chip "Card 0" (R1-08).""" + an unnamed chip "Card 0".""" sanitized = sanitize_snapshot( { "ts": empty, @@ -425,5 +425,5 @@ def test_the_fixture_keeps_a_car_profile_the_select_can_show( ) -> None: """Diagnostics say or , neither of which the select offers; a regenerated fixture took that over and the suite went - red (post-check F31-01). The owner's car stays out either way.""" + red. The owner's car stays out either way.""" assert fixture_from({"ct": car_profile, "sse": "123456"})["ct"] == stand_in diff --git a/tests/test_diagnostics_boundary.py b/tests/test_diagnostics_boundary.py index 97c1430..664a7be 100644 --- a/tests/test_diagnostics_boundary.py +++ b/tests/test_diagnostics_boundary.py @@ -4,7 +4,7 @@ object as a SimpleNamespace. `scrub` walks dictionaries, lists and strings, so the namespaces went past it untouched and Home Assistant's exporter wrote their repr() into the download -- IPs, SSIDs and MAC addresses from `dns`, -`wifis` and `scan` (audit A11-03). Dictionary-only fixtures cannot see that +`wifis` and `scan`. Dictionary-only fixtures cannot see that boundary, so this module drives frames through the client instead. """ @@ -84,7 +84,7 @@ async def test_the_normal_client_shapes_are_not_treated_as_unexpected( ) -> None: """`cards` and `cci` carry their own handling. Reporting the client's ordinary values as an unknown shape drops the very data a bug report - needs (audit A11-03).""" + needs.""" sanitized = sanitize_snapshot(loaded_client.all_properties) assert sanitized["cci"]["model"] == "Symo" diff --git a/tests/test_e2e_smoke.py b/tests/test_e2e_smoke.py index 0a79725..ffdf7ac 100644 --- a/tests/test_e2e_smoke.py +++ b/tests/test_e2e_smoke.py @@ -1,4 +1,4 @@ -"""Full-setup smoke test against the real-device fixture (slow). +"""Full-setup smoke test against the real-device fixture. Every other test builds its entities by hand, so nothing else checks the inventory a real install actually ends up with. This one runs the whole @@ -30,7 +30,7 @@ from .conftest import FakeWattpilot -pytestmark = [pytest.mark.e2e, pytest.mark.timeout(120)] +pytestmark = pytest.mark.e2e SERIAL = "123456" diff --git a/tests/test_guards.py b/tests/test_guards.py index 1c9ef61..fbb13a1 100644 --- a/tests/test_guards.py +++ b/tests/test_guards.py @@ -4,7 +4,9 @@ import ast import json +import os import re +import subprocess import sys from importlib import metadata from pathlib import Path @@ -229,7 +231,7 @@ def _traceback_leaks(source: str) -> list[str]: Tracebacks of an error that can quote the address. Home Assistant logs a failed setup's traceback, causes included, and a - message mask never reaches it (R1-01). + message mask never reaches it. """ return [ f"{inner.lineno} traceback of {handler.name or 'the error'}" @@ -312,7 +314,7 @@ def test_no_device_identifier_reaches_the_log() -> None: def _strip_vendor_spellings(text: str) -> str: """Remove the vendor package spellings before the domain-literal test. - Naming the library we wrap in prose (e.g. a module docstring) has + Naming the library the client was adopted from (a module docstring) has nothing to do with make_dev_copy's replace and shouldn't force code to dodge the guard with a capitalization trick. """ @@ -526,7 +528,7 @@ def test_manifest_is_consistent() -> None: def test_the_two_declared_versions_agree() -> None: """One release, one number. The release gate compares the tag to the manifest and never looked at pyproject.toml, so the two drifted to 0.1.0 - and 0.0.0 with nothing to notice (audit VA-17).""" + and 0.0.0 with nothing to notice.""" import tomllib # noqa: PLC0415 -- scoped on purpose, only this test needs it manifest = json.loads((PACKAGE / "manifest.json").read_text(encoding="utf-8")) @@ -655,30 +657,30 @@ def test_device_fixture_is_anonymized() -> None: # dispatch, property table and the typed write paths. Adopted at 1220 # lines; splitting it is a separate decision, not a side effect of taking # it over. Three raises in the 2026-09-05 audit round triggered the - # ratchet's own clustering criterion, and the owner decided to split - # (ledger L-016): the connection lifecycle moved to connection.py and + # ratchet's own clustering criterion, and the owner decided to split: + # the connection lifecycle moved to connection.py and # this budget came back down with it (measured: 1192). Nudged to 1230 on # 2026-09-06 for the audit round's connect cleanup and firmware deadline - # (measured: 1221). Down on 2026-10-11 with the 45 accessors nothing read - # (audit A18-37), keeping room for the refusal in the firmware wait. Down + # (measured: 1221). Down on 2026-10-11 with the 45 accessors nothing read, + # keeping room for the refusal in the firmware wait. Down # again the same day with the seven getters only tests read, then with # `version` (measured: 793). Up to 803 the same day, owner-approved, for # status frames without a status object and the close count of the - # firmware wait (findings 6 and RF-L-01). + # firmware wait. "client.py": 803, - # Past the default on 2026-09-25 by the open/close coordination (audits - # A15-01, A16-01): shared attempts, a close that cancels instead of + # Past the default on 2026-09-25 by the open/close coordination: shared + # attempts, a close that cancels instead of # waiting, and one visible transition at a time (measured: 406). Raised # on 2026-10-10, owner-approved: to 424 for recovery after a failed - # reconnect action (A18-05/06), to 440 for a deliberate close winning - # over recovery (R1-11/12) (measured on 2026-10-11: 437). + # reconnect action, to 440 for a deliberate close winning + # over recovery (measured on 2026-10-11: 437). "connection.py": 440, # Past the default on 2026-09-26 by the repair notice for a different - # charger answering at runtime (audit A15-04): the hub is where the + # charger answering at runtime: the hub is where the # connection's state turns into what the user is told (measured: 424). # Owner-approved raises: 444 on 2026-10-10 for announcing a recovery with - # the current values (A18-20), 446 on 2026-10-11 for refusal notices - # that go once the charger is back (reaudit RF-L-03). + # the current values, 446 on 2026-10-11 for refusal notices + # that go once the charger is back. "hub.py": 446, } SHRINK_SLACK = 0.85 # an entry >=15% below budget must be ratcheted down @@ -688,17 +690,17 @@ def test_device_fixture_is_anonymized() -> None: # partials and reached ~15,500 lines past a per-file budget. So a unit that was # split stays measured as one cluster. CLUSTER_BUDGET = { - # The adopted charger client. Split on 2026-09-05 (ledger L-016) into the + # The adopted charger client. Split on 2026-09-05 into the # protocol and the connection lifecycle; the pair is the unit that must # not grow. Raised once for the whole 2026-09-06 audit round: connection - # ownership and readiness deadlines (A12-01/04), fail-stop on refusal - # (A12-02), the cleanup that spans the definition load (A12-03) and the - # firmware deadline with the disconnect event it waits on (A12-08), at a + # ownership and readiness deadlines, fail-stop on refusal, + # the cleanup that spans the definition load and the + # firmware deadline with the disconnect event it waits on, at a # measured 1567 lines. Raised a second time on 2026-09-25, after about # forty lines of comment had already been condensed to make room: the - # open/close coordination (A13-01, A14-01, A15-01) and the refused- - # password flag (A14-02), at a measured 1607 lines. Down with client.py - # on 2026-10-11 (A18-37), and again with the last unread getters (1233); up + # open/close coordination and the refused- + # password flag, at a measured 1607 lines. Down with client.py + # on 2026-10-11, and again with the last unread getters (1233); up # to 1240 with client.py (owner-approved). "api client": (("client.py", "connection.py"), 1240), } @@ -721,8 +723,7 @@ def _imports_of(path: Path) -> set[str]: return modules -# Password hashing is deliberately expensive; the number below is the -# measured cost of one PBKDF2 handshake on the development machine. +# Password hashing is deliberately expensive. EXPENSIVE_HASHES = frozenset({"hash_password", "_hash_pbkdf2", "_hash_bcrypt"}) @@ -731,7 +732,7 @@ def test_expensive_hashing_never_runs_on_the_event_loop() -> None: A key-derivation call must be handed to a thread, never called directly. It was called directly once, and the handshake stalled Home Assistant's - event loop for 110-116 ms on every connection (audit A11-08). Moving the + event loop for 110-116 ms on every connection. Moving the one call site fixes the day; this stops the next one being written. Fix: `await asyncio.to_thread(hash_password, ...)` -- pass the function, @@ -757,6 +758,218 @@ def test_expensive_hashing_never_runs_on_the_event_loop() -> None: } +# Review finding IDs and the names of private notes: they mean nothing to a +# reader of a public repository and point at records it does not contain. +# The reason a line exists belongs in the line, not behind an ID. A number of +# one digit is too common outside reviews (WPA-2, UTC-5) to count, except in +# the one shape that used it. MAC addresses leave the line before the search, +# as their octets have the same shape. +INTERNAL_REFERENCE = re.compile( + r"\b[A-Z]{1,5}\d{0,2}(?:-[A-Z]{1,3})?-\d{2,3}\b" + r"|\bH-\d\b|\b[A-Z]{2,4}-[A-Z]\d\b|\bK-[A-Z]\b|\b[Ff]indings? \d+\b" + # Split, so that this line does not name what it looks for. + r"|\b(?:[Ll]ed" + r"gers?|LED" + r"GER|[Ee]rosions" + r"schutz|EROSIONS" + r"SCHUTZ|RUN" + r"BOOK|[Rr]un" + r"books?|STATUS\.md|BEF" + r"UND|Bef" + r"unde?)\b" +) +# Standards and terminals that have the same shape as an ID. +STANDARD_NAME = re.compile(r"(?:UTF|SHA|AES|CRC|IP)-\d+|(?:AC|PE)-L\d") +ANY_MAC = re.compile( + r"(? list[tuple[Path, str]]: + """Every UTF-8 text file git tracks or would, release notes and fixtures too.""" + command = ["git", "ls-files", "-z", "--cached", "--others", "--exclude-standard"] + try: + listed = subprocess.run( # noqa: S603 -- git from PATH, fixed arguments + command, cwd=root, capture_output=True, check=True + ).stdout + except (OSError, subprocess.CalledProcessError) as error: + pytest.fail(f"This guard needs a git checkout: {error}") + texts = [] + for name in filter(None, listed.split(b"\0")): + path = root / os.fsdecode(name) + if not path.is_file(): + continue + try: + texts.append((path, path.read_text(encoding="utf-8"))) + except UnicodeDecodeError: + continue + return texts + + +COMMIT_IDENTITY = { + "GIT_AUTHOR_NAME": "test", + "GIT_AUTHOR_EMAIL": "test@example.invalid", + "GIT_COMMITTER_NAME": "test", + "GIT_COMMITTER_EMAIL": "test@example.invalid", +} + + +def _internal_references(line: str) -> list[str]: + return [ + match[0] + for match in INTERNAL_REFERENCE.finditer(ANY_MAC.sub("", line)) + if not STANDARD_NAME.fullmatch(match[0]) + ] + + +def test_no_internal_review_reference_in_the_repository() -> None: + root = PACKAGE.parent.parent + texts = _repository_texts(root) + # An empty listing would pass the check below without reading a line. + assert { + "README.md", + "CHANGELOG.md", + "custom_components/wattpilot/manifest.json", + "tests/test_guards.py", + } <= {path.relative_to(root).as_posix() for path, _text in texts} + found = [ + f"{path.relative_to(root)}:{number}: {reference}" + for path, text in texts + for number, line in enumerate(text.splitlines(), start=1) + for reference in _internal_references(line) + ] + assert found == [], found + + +# Each sample is joined from parts, so this file does not trip the guard. A +# rebuilt guard keeps the samples of the one it replaces: one it drops is +# something it no longer catches. +@pytest.mark.parametrize( + "line", + [ + "fixed in the last round (VA" + "-11)", + "see RA-BF" + "-01", + "see A18" + "-31", + "A11" + "-03 said so", + "PC4" + "-02", + "R1" + "-14", + "F31" + "-05", + "L" + "-040", + "R" + "-01", + "H" + "-2", + "RF-L" + "-03", + "PC5-R" + "-03", + "RECON" + "-01", + "VA" + "-11-Fehler", + "A18" + "-31-fallback", + "LED" + "GER", + "led" + "gers", + "EROSIONS" + "SCHUTZ", + "Finding" + " 6", + "Bef" + "und", + "run" + "books", + "FC" + "-01", + "an old led" + "ger entry", + "erosions" + "schutz", + "the K" + "-A matrix", + "the K" + "-B matrix", + "as finding" + " 6 said", + "from the RUN" + "BOOK", + "see the run" + "book", + "as STATUS" + ".md says", + "BEF" + "UND", + "LIFE" + "-M1", + "DIAG" + "-M2", + "(ref:RW" + "-05)", + ], +) +def test_the_review_reference_guard_knows_each_shape(line: str) -> None: + assert _internal_references(line), line + + +@pytest.mark.parametrize( + "line", + [ + "mac fa-16-3e-12-34-56", + "MAC FA-16-3E-12-34-56", + "MAC FA-B1-3E-12-34-56", + "MAC AA-BB-CC-DD-EE-F1", + "aa:bb:cc:dd:ee:ff", + "phase L1", + "phases L1-3", + "a l-1", + "UTF-8", + "UTF-16", + "SHA-1", + "SHA-256", + "SHA-512", + "AES-128", + "HTTP-2", + "TLS-1", + "IP-65", + "CRC-32", + "WPA-2", + "UTC-5", + "OCPP-1.6", + "PEP-8", + "TYPE-2", + "phases L1-2", + "AC-L1", + ], +) +def test_the_review_reference_guard_lets_addresses_and_names_pass(line: str) -> None: + assert _internal_references(line) == [] + + +def test_the_review_reference_guard_skips_what_git_ignores(tmp_path: Path) -> None: + """A stale development copy of the integration, which git ignores, made + the suite and with it every push fail on that one machine.""" + subprocess.run( # noqa: S603 -- git from PATH, fixed arguments + ["git", "init", "-q", str(tmp_path)], # noqa: S607 + check=True, + ) + (tmp_path / ".gitignore").write_text("ignored/\n", encoding="utf-8") + (tmp_path / "ignored").mkdir() + (tmp_path / "ignored" / "stale.py").write_text("x = 1\n", encoding="utf-8") + (tmp_path / "kept.py").write_text("x = 1\n", encoding="utf-8") + names = {path.name for path, _text in _repository_texts(tmp_path)} + assert names == {".gitignore", "kept.py"} + + +def test_the_review_reference_guard_reads_past_what_is_no_file(tmp_path: Path) -> None: + """git lists an embedded repository as a directory, and a file name need + not be UTF-8; either one ended the listing instead of being passed by.""" + subprocess.run( # noqa: S603 -- git from PATH, fixed arguments + ["git", "init", "-q", str(tmp_path)], # noqa: S607 + check=True, + ) + embedded = tmp_path / "embedded" + embedded.mkdir() + subprocess.run( # noqa: S603 -- git from PATH, fixed arguments + ["git", "init", "-q", str(embedded)], # noqa: S607 + check=True, + ) + (embedded / "inner.py").write_text("x = 1\n", encoding="utf-8") + subprocess.run( + ["git", "commit", "-q", "--allow-empty", "-m", "x"], # noqa: S607 + cwd=embedded, + check=True, + env={**os.environ, **COMMIT_IDENTITY}, + ) + (tmp_path / os.fsdecode(b"caf\xe9.txt")).write_text("x\n", encoding="utf-8") + (tmp_path / "kept.py").write_text("x = 1\n", encoding="utf-8") + names = {path.name for path, _text in _repository_texts(tmp_path)} + assert "kept.py" in names + assert "embedded" not in names + + +def test_the_review_reference_guard_says_it_needs_a_checkout(tmp_path: Path) -> None: + """Outside a checkout it has to fail, not pass unread; an exit status + alone left the reader guessing what was wrong with the code.""" + with pytest.raises(pytest.fail.Exception, match="needs a git checkout"): + _repository_texts(tmp_path) + + def test_no_deprecated_home_assistant_call() -> None: """ A deprecated call works until the release that removes it, then setup @@ -974,7 +1187,7 @@ def _complexities_over(threshold: int) -> dict[str, int]: def test_the_snapshot_helper_sees_what_the_ratchet_sees() -> None: """The helper read only the top-level package; regenerating the table - with it dropped every api/ entry (audit A19-12).""" + with it dropped every api/ entry.""" assert complexity_snapshot.snapshot() == _complexities_over( complexity_snapshot.THRESHOLD ) @@ -990,7 +1203,7 @@ def test_complexity_ratchet() -> None: complexity_table: dict[str, int] = { "descriptions.py::firmware_supported": 7, "redaction.py::scrub": 8, - # 7 since the `ts` rule (audit VA-07): one more branch in what is + # 7 since the `ts` rule: one more branch in what is # a dispatch table written as code, not added logic. "redaction.py::sanitize_property": 6, "sensor.py::WattpilotSensor._apply_value": 8, @@ -1003,11 +1216,10 @@ def test_complexity_ratchet() -> None: "auth.py::_bcryptjs_base64_encode": 6, "connection.py::Connection._read_frames": 6, "connection.py::Connection._message_loop": 6, - "client.py::Wattpilot._handle_message": 11, + "client.py::Wattpilot._handle_message": 9, "client.py::Wattpilot._on_response": 6, - "client.py::Wattpilot._update_property": 6, "definition.py::validate_api_definition": 12, - "definition.py::load_api_definition": 8, + "definition.py::load_api_definition": 7, } actual = _complexities_over(threshold) for name, complexity in actual.items(): @@ -1030,7 +1242,7 @@ def test_complexity_ratchet() -> None: # Word for word: a substring check stayed green with the arguments reversed, -# with `|| true` and with `echo` in front (reaudit RF-D-05). +# with `|| true` and with `echo` in front. ANCESTRY_CHECK = 'git merge-base --is-ancestor "$GITHUB_SHA" origin/main' @@ -1070,7 +1282,7 @@ def test_a_release_is_only_cut_from_main() -> None: Tags come from main only -- a project rule the workflow did not check. A tag pushed from a branch before its merge published code main does - not contain, and HACS offers that to everyone (audit A18-31). Fix: keep + not contain, and HACS offers that to everyone. Fix: keep the ancestry check in the release job, with the full history checked out. """ assert _release_problems(RELEASE_WORKFLOW.read_text(encoding="utf-8")) == [] @@ -1112,7 +1324,7 @@ def _jobs_pinning_and_testing(jobs: dict[str, Any], pin: str) -> list[str]: # have to sit in the SAME job's run steps. A commented-out pin cannot # satisfy this -- the YAML parser drops comments before we look. And # "pytest" alone was found in pytest-homeassistant-custom-component on - # the install line, so a job without its test step passed (A18-30). + # the install line, so a job without its test step passed. return [ name for name, job in jobs.items() @@ -1163,7 +1375,7 @@ def test_ci_actually_runs_the_declared_minimum_home_assistant() -> None: def _enum_signature(enum: object) -> object: """Compare enums by what they map, not by which object they are. - Audit VA-11: comparing `id(enum)` let a behaviourally identical copy of + Comparing `id(enum)` let a behaviourally identical copy of CAR_STATE_ENUM pass as a different sensor -- the guard would have missed the very duplicate it exists to catch. """ @@ -1231,7 +1443,7 @@ def test_the_duplicate_sensor_guard_can_actually_fail() -> None: regressed = replace(car_connected, enum=car_state.enum) assert _rendering_signature(regressed) == _rendering_signature(car_state) - # A copy of that enum, which audit VA-11 slipped past the first version: + # A copy of that enum, which slipped past the first version: # same mapping, different object. copied = replace(car_connected, enum=dict(car_state.enum)) assert _rendering_signature(copied) == _rendering_signature(car_state) @@ -1243,7 +1455,7 @@ def test_the_duplicate_sensor_guard_can_actually_fail() -> None: ) -# The guards, and what each one holds. Adapted from the erosionsschutz kit's +# The guards, and what each one holds. Adapted from a template's # meta-guard: deleting a guard is otherwise a green diff, and nothing answers # "what stops the old problems coming back" six months later. Indexed per # function rather than per file, because this project keeps its guards in two @@ -1256,6 +1468,24 @@ def test_the_duplicate_sensor_guard_can_actually_fail() -> None: "test_no_deprecated_home_assistant_call": ( "no Home Assistant call that reports as deprecated" ), + "test_no_internal_review_reference_in_the_repository": ( + "no review finding IDs or private note names in public files" + ), + "test_the_review_reference_guard_knows_each_shape": ( + "it sees the shapes used so far" + ), + "test_the_review_reference_guard_lets_addresses_and_names_pass": ( + "counter-check: MAC octets, phases, standard names" + ), + "test_the_review_reference_guard_skips_what_git_ignores": ( + "a local ignored copy does not fail the suite" + ), + "test_the_review_reference_guard_reads_past_what_is_no_file": ( + "directories and odd file names do not end the listing" + ), + "test_the_review_reference_guard_says_it_needs_a_checkout": ( + "outside a checkout it fails and says why" + ), "test_every_user_facing_error_is_translated": ( "every error a user reads is translated, with matching placeholders" ), @@ -1419,8 +1649,7 @@ def test_every_guard_is_listed_in_the_index() -> None: ) -# The log check in conftest.py is itself a guard; these keep it honest -# (post-check PC6-04). +# The log check in conftest.py is itself a guard; these keep it honest. @pytest.mark.parametrize( "text", [ @@ -1449,7 +1678,7 @@ def test_the_log_check_lets_ordinary_output_pass(text: str) -> None: def test_the_log_check_outlasts_home_assistant() -> None: """hass comes in through auto_enable_custom_integrations, and autouse fixtures start in name order: only requesting the log fixtures first sets - them up before hass and tears them down after its stop (PC6-04). The + them up before hass and tears them down after its stop. The deprecation check likewise: Home Assistant reports while it stops, too.""" tree = ast.parse( (Path(__file__).parent / "conftest.py").read_text(encoding="utf-8") @@ -1469,7 +1698,7 @@ def test_the_log_check_outlasts_home_assistant() -> None: # The modules whose logic the mutation run measures. Code moved out of one -# of them left the run unless the config followed by hand (post-check PC7-05). +# of them left the run unless the config followed by hand. MUTATED_MODULES = {"__init__.py", "hub.py", "sensor.py", "readings.py"} diff --git a/tests/test_hub.py b/tests/test_hub.py index 7efcfd2..8eb8fa2 100644 --- a/tests/test_hub.py +++ b/tests/test_hub.py @@ -274,18 +274,19 @@ async def test_shutdown_disconnects_and_stops_dispatch( assert fake_charger._callbacks == [] -async def test_shutdown_maps_disconnect_error_and_still_clears_callbacks( +async def test_disconnect_maps_disconnect_error_and_still_clears_callbacks( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """A failing disconnect must surface as HomeAssistantError, but the hub's handle cleanup (unsubscribe, cancel timer) must already have - happened -- a failed shutdown must not leave the hub half-alive.""" + happened -- a failed disconnect must not leave the hub half-alive. + Shutting down for good logs the error instead (test_init).""" hub = make_hub(hass, fake_charger) await hub.async_connect() hub.start_dispatch() fake_charger.disconnect_error = ConnectionError("socket closed") with pytest.raises(HomeAssistantError) as raised: - await hub.async_shutdown() + await hub.async_disconnect() assert_translated(raised, "disconnect_failed", error="ConnectionError") assert fake_charger._callbacks == [] assert hub._unsubscribe_properties is None @@ -388,7 +389,7 @@ async def test_a_reconnect_during_shutdown_does_not_revive_the_hub( """Shutdown unregisters first and then waits for the disconnect. A reconnect in that window registered callbacks and timers again and cleared the torn-down flag; shutdown then returned with a hub live - behind an entry that was being unloaded (audit A15-02).""" + behind an entry that was being unloaded.""" hub = make_hub(hass, fake_charger, timedelta(seconds=5)) await hub.async_connect() hub.start_dispatch() @@ -521,7 +522,7 @@ async def test_an_unannounced_drop_is_reported_at_info( async def test_a_failed_disconnect_still_reports_unavailable( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """Audit VA-04: `available` used to read the vendor client's `connected` + """`available` used to read the client's `connected` flag, which that client clears only after a successful ws.close(). A close that raises left the flag True while its message loop was already gone -- entities kept claiming a live charger that could never update @@ -535,8 +536,7 @@ async def test_a_failed_disconnect_still_reports_unavailable( assert hub.available fake_charger.disconnect_error = ConnectionError("close failed") - with pytest.raises(HomeAssistantError): - await hub.async_shutdown() + await hub.async_shutdown() assert fake_charger.connected, "precondition: the client's own flag stays set" assert not hub.available @@ -545,8 +545,8 @@ async def test_a_failed_disconnect_still_reports_unavailable( async def test_a_refused_write_reaches_the_user_as_an_error( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """Audit VA-03, from the outside: the charger refusing a value must fail - the action, not end quietly. The vendored client raises CommandError for + """From the outside: the charger refusing a value must fail + the action, not end quietly. The client raises CommandError for that now; the hub has to keep translating it rather than let it escape as a library exception.""" hub = make_hub(hass, fake_charger) @@ -579,8 +579,7 @@ async def test_a_recovery_is_announced_with_the_current_values( """The first replayed value announced the recovery, so every entity showed its reading from before the outage -- charging, 11 kW -- until the next flush, and `to: charging` automations fired on stale data. The - announcement now waits for the full status and comes after its values - (audit A18-20).""" + announcement now waits for the full status and comes after its values.""" hub = make_hub(hass, fake_charger, update_interval=timedelta(seconds=5)) await hub.async_connect() hub.start_dispatch() @@ -621,7 +620,7 @@ async def test_a_reconnect_between_two_checks_does_not_flap_the_entities( """The client reconnects within its 5 s backoff, before the 30 s check saw it gone. The replay of the full status took every entity through unavailable at the moment the charger was back, logged an outage and - fired `to:` triggers again (R1-04).""" + fired `to:` triggers again.""" caplog.set_level(logging.INFO, logger="custom_components.wattpilot.hub") hub = make_hub(hass, fake_charger) await hub.async_connect() diff --git a/tests/test_init.py b/tests/test_init.py index 6343581..992b3de 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -54,7 +54,7 @@ TIME_DESCRIPTIONS, ) -from .conftest import FakeWattpilot +from .conftest import FakeWattpilot, patch_charger from .test_config_flow import DISCOVERY, start_discovery_flow from .test_sensor import nrg_reading @@ -87,10 +87,6 @@ } -def patch_charger(charger: FakeWattpilot) -> Any: - return patch("custom_components.wattpilot.hub.Wattpilot", return_value=charger) - - async def setup_entry( hass: HomeAssistant, entry: MockConfigEntry, charger: FakeWattpilot ) -> bool: @@ -107,7 +103,7 @@ async def test_a_password_rejected_while_running_asks_for_a_new_one( """The client stops reconnecting once the charger refuses the password -- retrying would only repeat the refusal. But nothing told Home Assistant: the entry just stayed unavailable until someone reloaded it, - with no hint that the password was the problem (audit A14-02).""" + with no hint that the password was the problem.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -137,9 +133,9 @@ async def test_another_charger_answering_while_running_is_reported( ) -> None: """The client stops for good when a different charger answers at the address -- DHCP handed it on -- rather than show that charger's readings - as this one's. But nothing said so: the entities just stayed unavailable - (audit A15-04). A new password would not help, so this is a repair - notice, not reauthentication; it lasts as long as the loaded entry.""" + as this one's. But nothing said so: the entities just stayed + unavailable. A new password would not help, so this is a repair notice, + not reauthentication; it lasts as long as the loaded entry.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -167,7 +163,7 @@ async def test_a_charger_asking_for_an_unknown_sign_in_is_reported( ) -> None: """A third way the client stops for good: the charger asks for a password scheme it does not know. Nothing reported it; the entities just stayed - unavailable (reaudit R1-13).""" + unavailable.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -204,7 +200,7 @@ async def test_a_refusal_notice_goes_once_the_charger_is_back( hass: HomeAssistant, fake_charger: FakeWattpilot, refusal: Any, kind: str ) -> None: """After a successful reconnect the notice still said the integration - had stopped connecting, next to available entities (reaudit RF-L-03).""" + had stopped connecting, next to available entities.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -427,18 +423,22 @@ async def test_unique_id_upgraded_from_ip_to_serial( assert entry.unique_id == "123456" +@pytest.mark.parametrize("unique_id", ["192.168.1.50", "123456"]) async def test_the_serial_upgrade_permission_is_consumed_once( - hass: HomeAssistant, device_properties: dict[str, Any] + hass: HomeAssistant, device_properties: dict[str, Any], unique_id: str ) -> None: """ A fork entry may adopt a serial exactly once, then never again. If the permission outlived the upgrade, the entry would stay open to being relabelled by whatever charger answers at that address next -- the - guard would exist but protect nothing from the second time on. + guard would exist but protect nothing from the second time on. That + includes an entry already keyed on the serial it is confirmed with, as a + cloud entry made local by Reconfigure is: it used to keep the permission + forever, and with it a client that never checked the charger's identity. """ entry = MockConfigEntry( - domain=DOMAIN, data=V2_AWAITING_SERIAL, version=2, unique_id="192.168.1.50" + domain=DOMAIN, data=V2_AWAITING_SERIAL, version=2, unique_id=unique_id ) assert await setup_entry(hass, entry, FakeWattpilot(device_properties)) assert entry.unique_id == "123456" @@ -453,6 +453,28 @@ async def test_the_serial_upgrade_permission_is_consumed_once( assert raised.value.translation_key == "wrong_charger" +async def test_an_entry_whose_disconnect_fails_still_unloads( + hass: HomeAssistant, fake_charger: FakeWattpilot +) -> None: + """ + Unloading closes the connection for good, and nothing is left to retry. + + A failure there used to fail the unload: the entry stayed at + FAILED_UNLOAD and refused every reload until Home Assistant restarted. + """ + entry = MockConfigEntry( + domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" + ) + assert await setup_entry(hass, entry, fake_charger) + fake_charger.disconnect_error = WattpilotError("socket already gone") + + assert await hass.config_entries.async_unload(entry.entry_id) + assert entry.state is ConfigEntryState.NOT_LOADED + with patch_charger(fake_charger): + assert await hass.config_entries.async_setup(entry.entry_id) + assert entry.state is ConfigEntryState.LOADED + + async def test_a_refused_charger_is_reported_even_if_disconnecting_fails( hass: HomeAssistant, device_properties: dict[str, Any], @@ -461,8 +483,11 @@ async def test_a_refused_charger_is_reported_even_if_disconnecting_fails( """The identity check shut the hub down before raising wrong_charger, and the outer setup guard shut it down a second time. With a disconnect that also fails, that cleanup error replaced the refusal: - the user saw "Failed to disconnect" instead of why setup was refused - (audit A14-03).""" + the user saw "Failed to disconnect" instead of why setup was refused. + + The client refuses another charger itself, at hello, before it + authenticates; this is setup's own check behind it, so the client here + does not check.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -473,7 +498,7 @@ async def test_a_refused_charger_is_reported_even_if_disconnecting_fails( with ( caplog.at_level(logging.DEBUG), - patch_charger(swapped), + patch("custom_components.wattpilot.hub.Wattpilot", return_value=swapped), pytest.raises(ConfigEntryError) as raised, ): await async_setup_entry(hass, entry) @@ -484,7 +509,7 @@ async def test_a_refused_charger_is_reported_even_if_disconnecting_fails( cleanup = [ r for r in caplog.records - if r.getMessage() == "Ignoring error while closing after failed setup" + if r.getMessage() == "Ignoring error while closing for good" ] assert len(cleanup) == 1 # Not `is not None`: logging keeps exc_info=False as False. @@ -552,6 +577,10 @@ async def test_a_different_charger_at_the_same_address_is_refused( same recorder history, another charger's readings, and not one error anywhere. Silent, plausible, wrong data is the worst failure this integration can produce, so setup has to refuse. + + The client refuses another charger itself, at hello, before it + authenticates; this is setup's own check behind it, so the client here + does not check. """ entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" @@ -564,7 +593,10 @@ async def test_a_different_charger_at_the_same_address_is_refused( swapped = FakeWattpilot(device_properties) swapped.serial = "999999" - with patch_charger(swapped), pytest.raises(ConfigEntryError) as raised: + with ( + patch("custom_components.wattpilot.hub.Wattpilot", return_value=swapped), + pytest.raises(ConfigEntryError) as raised, + ): await async_setup_entry(hass, entry) assert raised.value.translation_domain == DOMAIN @@ -643,7 +675,7 @@ async def test_a_migrated_fork_entry_names_new_entities_without_the_serial( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """A fork entry kept no name, so its device took the host name and every - entity the fork lacked got the serial in its ID (reaudit RF-P-04). The + entity the fork lacked got the serial in its ID. The fork's own friendly name is what its entity IDs were made from.""" entry = MockConfigEntry( domain=DOMAIN, @@ -699,7 +731,7 @@ async def test_a_migrated_fork_cloud_entry_is_named_for_its_reconfigure( ) -> None: """Setup sends a cloud entry to Reconfigure, which keeps the stored name; without one the device took the host name and new entity IDs carried the - serial (post-check PC5-R-04).""" + serial.""" entry = MockConfigEntry( domain=DOMAIN, data={**V1_CLOUD_DATA, "friendly_name": "Garage"}, @@ -778,7 +810,7 @@ async def test_ha_stop_disconnects( async def test_a_failing_setup_hands_back_the_live_connection( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """Audit VA-02: everything after async_connect() owns a live connection, + """Everything after async_connect() owns a live connection, a push callback and timers, and HA calls async_unload_entry only for an entry that finished setup. A failure in between must give them back, or the next retry adds a second hub on the same dispatcher signals. @@ -811,7 +843,7 @@ async def test_a_failing_setup_hands_back_the_live_connection( async def test_a_firmware_change_reloads_the_entry( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """Audit VA-05: the platforms filter their tables once, at setup, and the + """The platforms filter their tables once, at setup, and the device's sw_version is frozen when the entities are built. After a firmware jump the newly supported entities stay missing and the device page keeps showing the old version until someone reloads by hand. @@ -854,7 +886,7 @@ async def test_the_known_serial_is_checked_before_the_password_is_sent( """The entry knows its charger's serial; the client checks hello against it before answering the authentication challenge. It used to learn the serial from whatever answered, so a wrong device got the password proof - first and was refused only afterwards (audit A17-01). An entry still + first and was refused only afterwards. An entry still awaiting its serial has none to give.""" entry = MockConfigEntry(domain=DOMAIN, data=data, version=2, unique_id=unique_id) entry.add_to_hass(hass) @@ -929,8 +961,7 @@ async def test_an_entry_titled_by_its_address_does_not_log_it( caplog: pytest.LogCaptureFixture, ) -> None: """A fork entry can be titled by its IP address, and Home Assistant logs - the title on every failed setup; the mask knew only serials (reaudit - RF-P-02).""" + the title on every failed setup; the mask knew only serials.""" caplog.set_level(logging.DEBUG) entry = MockConfigEntry( domain=DOMAIN, @@ -973,7 +1004,7 @@ async def test_a_setup_retry_does_not_log_the_chargers_address( caplog: pytest.LogCaptureFixture, ) -> None: """Home Assistant logs each retry's traceback at DEBUG, the chained cause - included -- and asyncio's refused connect quotes the address (R1-01).""" + included -- and asyncio's refused connect quotes the address.""" caplog.set_level(logging.DEBUG) entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" @@ -1038,7 +1069,7 @@ async def test_an_entry_awaiting_its_serial_masks_nothing( ) -> None: """An entry carried over from the earlier integration is keyed on its name or address until it connects. Masked as a serial, that name turned - every other title holding it into `*******ot` (R1-17).""" + every other title holding it into `*******ot`.""" MockConfigEntry(domain=DOMAIN, unique_id="Wattpilot").add_to_hass(hass) record = _record("Config entry '%s' not ready", "Wattpilot meter") @@ -1054,7 +1085,7 @@ async def test_a_setup_cancelled_by_the_stop_is_logged_masked( ) -> None: """Home Assistant fires the stop event first and lets the setup retries it cancelled log their title afterwards. The mask went with the event, - so that line carried the full serial (R1-05).""" + so that line carried the full serial.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -1105,8 +1136,7 @@ async def test_adopting_the_serial_closes_the_charger_s_open_card( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """The card would stay offered for a charger that is configured now -- - and only that card: closing every flow of the domain passed too - (reaudit RF-E-03).""" + and only that card: closing every flow of the domain passed too.""" await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_ZEROCONF}, @@ -1210,7 +1240,7 @@ async def test_total_energy_does_not_fall_across_a_reload( """After a restart the charger reported 21 Wh less (replay fixture). The sensor held its value -- in memory only, so a reload in that window, as the one after a firmware update, reported the lower total and the energy - dashboard got a negative hour (audit A18-25).""" + dashboard got a negative hour.""" _, total = await _total_energy_after_restore( hass, fake_charger, restored=2681214, reported=2681193 ) @@ -1236,7 +1266,7 @@ async def test_a_restored_total_from_another_charger_does_not_freeze_it( ) -> None: """A replacement charger that took over the old entity ID restored the old charger's total and held it until its own passed it -- years at - zero in the energy dashboard (R1-03). Only a step back as small as a + zero in the energy dashboard. Only a step back as small as a restart's is held; a drop this large is a different meter.""" entity_id, total = await _total_energy_after_restore( hass, fake_charger, restored=2681214, reported=1200 @@ -1265,7 +1295,7 @@ async def test_an_unknown_total_keeps_the_hold_against_a_step_back( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """The hold compared against what was shown, so one null wiped it and - the next small step back reached the energy dashboard (RF-D-02).""" + the next small step back reached the energy dashboard.""" entity_id, total = await _total_energy_after_restore( hass, fake_charger, restored=2681214, reported=2681214 ) @@ -1283,7 +1313,7 @@ async def test_an_infinite_total_does_not_freeze_it( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """The hold took infinity as the last number and held every later - reading back until a reload (post-check PC5-R-01).""" + reading back until a reload.""" entity_id, _ = await _total_energy_after_restore( hass, fake_charger, restored=2681214, reported=2681214 ) @@ -1302,7 +1332,7 @@ async def test_a_restored_total_takes_the_first_number_after_an_unknown( caplog: pytest.LogCaptureFixture, ) -> None: """A restored total followed by a non-number dropped the next real - reading, with a TypeError warning about the wrong key (post-check PC-03).""" + reading, with a TypeError warning about the wrong key.""" entity_id, total = await _total_energy_after_restore( hass, fake_charger, restored=2681214, reported=None ) @@ -1379,8 +1409,7 @@ async def test_a_reading_without_total_power_keeps_the_last_sessions_average_awa hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """A push without a total power between sessions forgot which average - was stale, and the next session's first watt showed it (post-check - PC5-R-03).""" + was stale, and the next session's first watt showed it.""" _, entity_id = await _average_power_while_charging(hass, fake_charger) await _live_power(hass, fake_charger, 0, after=5) await _live_power(hass, fake_charger, 0, after=60) @@ -1398,7 +1427,7 @@ async def test_a_null_average_keeps_the_stale_one_known( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """A null average in a new session forgot which average was the last - session's, and it came back after (post-check PC7-01).""" + session's, and it came back after.""" _, entity_id = await _average_power_while_charging(hass, fake_charger) await _live_power(hass, fake_charger, 0, after=5) await _live_power(hass, fake_charger, 0, after=60) @@ -1416,7 +1445,7 @@ async def test_a_null_average_while_charging_is_unknown_not_zero( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """None stood for "nothing stale" too, so a null average while power - flowed read 0 W (post-check PC6-03).""" + flowed read 0 W.""" _, entity_id = await _average_power_while_charging(hass, fake_charger) fake_charger.push("tpa", None) @@ -1430,7 +1459,7 @@ async def test_a_total_power_that_is_no_number_is_not_idle( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """A NaN total power counted as no current, and the average fell to - 0 W a minute later (post-check PC6-06).""" + 0 W a minute later.""" _, entity_id = await _average_power_while_charging(hass, fake_charger) await _live_power(hass, fake_charger, float("nan"), after=5) @@ -1459,7 +1488,7 @@ async def test_a_session_after_an_idle_start_waits_for_its_own_average( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """At the first watt the last session's average came back until the - charger sent this session's (reaudit RF-D-04); also after an idle start.""" + charger sent this session's; also after an idle start.""" fake_charger._properties.update({"tpa": 4121.5, "nrg": nrg_reading(0)}) entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" @@ -1485,11 +1514,11 @@ async def test_an_unloaded_entry_leaves_no_average_power_timer_behind( await hass.async_block_till_done() -# What a charger could send where a number belongs (reaudit K-A). Never seen +# What a charger could send where a number belongs. Never seen # on a device, but each one escaped the bounded catch: Home Assistant raised # when it wrote the state, and at setup the entity was never added. # NaN and infinity are what json.loads makes of NaN, Infinity and a number -# beyond the float range; Home Assistant refuses those too (post-check PC5-R-01). +# beyond the float range; Home Assistant refuses those too. NOT_A_NUMBER = { "text": "n/a", "object": {"bogus": "n/a"}, @@ -1499,16 +1528,16 @@ async def test_an_unloaded_entry_leaves_no_average_power_timer_behind( "minus infinity": float("-inf"), "infinities": [float("inf")] * 16, # An integer literal past the float range stays an int, and converting - # it raises OverflowError (post-check PC6-01). + # it raises OverflowError. "beyond_float": 10**400, - # A list of lists: a select's option list keyed on it (post-check PC7-02). + # A list of lists: a select's option list keyed on it. "nested": [["n/a"]] * 2, } HOLDER = "Holder Example" # Every platform with a state: time and number raised OverflowError past -# their catch (post-check PC6-02), a select an unhashable error (PC7-02). +# their catch, a select an unhashable error. # The update entity has its own test: its installed version gates firmware. PLATFORM_DESCRIPTIONS = { "sensor": list(SENSOR_DESCRIPTIONS), @@ -1530,7 +1559,7 @@ async def test_no_value_a_charger_sends_keeps_an_entity_away( """Every entity, every malformed shape, at the boundary that raised: the state write, or the entity's own setup. A card's holder name never reaches the log -- Home Assistant logged the raw card list when the - write failed (RF-P-01).""" + write failed.""" bad = NOT_A_NUMBER[kind] for descriptions in PLATFORM_DESCRIPTIONS.values(): for description in descriptions: @@ -1538,8 +1567,7 @@ async def test_no_value_a_charger_sends_keeps_an_entity_away( if key is not None and key in fake_charger._properties: fake_charger._properties[key] = bad fake_charger._properties["cards"] = [{"name": HOLDER, "energy": bad}] * 10 - # A Flex's chips and power per source, which the fixture does not have - # (post-checks PC5-T-01, F31-05). + # A Flex's chips and power per source, which the fixture does not have. fake_charger._properties.update({f"c{i}e": bad for i in range(10)}) fake_charger._properties.update(dict.fromkeys(("ws", "wb", "wg", "wo"), bad)) entry = MockConfigEntry( @@ -1579,7 +1607,7 @@ async def test_a_card_energy_that_is_no_number_logs_no_holder_name( kind: str, ) -> None: """Home Assistant logged the raw card list, holder names included, each - time the state write of an enabled ID-chip sensor failed (RF-P-01).""" + time the state write of an enabled ID-chip sensor failed.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -1616,7 +1644,7 @@ async def test_a_failed_entry_whose_charger_announces_a_new_address_recovers( ) -> None: """Two chargers swapped addresses: this entry met the other charger and failed. Discovery stored the right address, but Home Assistant reloads - only a loaded or retrying entry, so it stayed failed (reaudit RF-E-01).""" + only a loaded or retrying entry, so it stayed failed.""" entry = MockConfigEntry( domain=DOMAIN, data=V2_LOCAL_DATA, version=2, unique_id="123456" ) @@ -1656,3 +1684,39 @@ async def test_a_failed_entry_is_not_reloaded_by_every_announcement( await hass.async_block_till_done() reload.assert_not_called() + + +async def test_the_fake_client_refuses_another_charger_every_time( + device_properties: dict[str, Any], +) -> None: + """The contract the setup and flow tests lean on: the real client + refuses another charger on each connect and keeps reporting the serial + it was built with. The fake took that serial over for good, so a second + attempt against the same other charger passed.""" + charger = FakeWattpilot(device_properties) + charger.serial = "999999" + for _ in range(2): + charger.built_with(serial="123456") + with pytest.raises(DeviceIdentityError) as raised: + await charger.connect() + assert raised.value.found == "999999" + assert charger.serial == "123456" + + +async def test_the_fake_client_learns_the_serial_at_hello( + device_properties: dict[str, Any], +) -> None: + """Built without a serial, the client knows none until hello, and takes + the first one it hears as the charger's from then on. A fake that knew + it from the start let code pass that reads the serial too early.""" + charger = FakeWattpilot(device_properties) + charger.built_with(serial=None) + assert charger.serial == "" + await charger.connect() + assert charger.serial == "123456" + + charger.device_serial = "999999" + with pytest.raises(DeviceIdentityError): + await charger.connect() + assert charger.serial == "123456" + assert isinstance(charger.refusal, DeviceIdentityError) diff --git a/tests/test_mutation_gate.py b/tests/test_mutation_gate.py index ff6562f..d8e7c97 100644 --- a/tests/test_mutation_gate.py +++ b/tests/test_mutation_gate.py @@ -3,9 +3,8 @@ The policy used to live in a sed expression inside check.sh that pulled out the survivors and nothing else. A mutant reported as "no tests" therefore disappeared from the comparison, the accepted survivors still matched, and -the gate passed while a module inside it was measurably uncovered -(audit A11-10). A gate nobody can run against a known input is a gate nobody -has checked. +the gate passed while a module inside it was measurably uncovered. A +gate nobody can run against a known input is a gate nobody has checked. """ from __future__ import annotations diff --git a/tests/test_redaction_register.py b/tests/test_redaction_register.py index 2b21500..eae9685 100644 --- a/tests/test_redaction_register.py +++ b/tests/test_redaction_register.py @@ -1,18 +1,19 @@ """Every field the device reports as a string carries a redaction decision. -Adapted from the erosionsschutz kit's completeness register. The class it +Adapted from a completeness-register template. The class it exists for is documented in this project: the diagnostics blocklist matched by field name, so every new data shape leaked exactly once -- wifis, scan and dns each had to be discovered separately before redaction.py unified them. A register turns "nobody thought of that field" into a red test at the moment the surface grows, instead of a finding two audits later. -The surface is every top-level string field the charger *can* report: the -string properties of the bundled API definition, plus anything the reference -device reports as a string that the definition does not list. Reading it off +The surface is every top-level field the charger *can* report as text: the +properties the bundled API definition declares as string or with no type, +plus anything the reference device reports as a string that the definition +does not list. Reading it off the snapshot alone was the same gap one level up -- `host`, `hsta` and `hsts` are documented as "Wattpilot_" but are unpopulated on firmware 42.5, -so they never reached a decision until an audit read the definition (A11-03). +so they never reached a decision until someone read the definition. Deliberate boundary: nested containers (cards, cci, wifis, scan, dns) carry their own assertions in test_guards.py::test_device_fixture_is_anonymized and @@ -39,7 +40,7 @@ # String fields a Wattpilot Flex reports that neither the reference device nor # the definition knows: names only, from a Flex diagnostics download # (2026-10-09). Without them a new Flex text field reached no decision, as -# the chip holder names once did (audit A18-15). +# the chip holder names once did. FLEX_STRING_FIELDS = frozenset( { *(f"c{i}n" for i in range(10)), @@ -128,7 +129,7 @@ "ims": "readable (mDNS service type of the paired inverter)", "los": "readable (load balancing status word, no address in it)", "wsm": REPLACED, - # Was the one known gap (audit VA-07); replaced since e6aec24's follow-up. + # Was the one known gap; replaced since e6aec24's follow-up. "ts": REPLACED, # Wattpilot Flex only (FLEX_STRING_FIELDS). **{f"c{i}n": REPLACED for i in range(10)}, # ID chip holder names @@ -170,7 +171,7 @@ # `log` (load group id) and `wsm` (WiFi error text) are free text whose # contents nobody controls. Both are empty or absent on the reference device, # which proves nothing about the next one: unknown free text is replaced until -# shown harmless, not the other way round (audit A17-02). +# shown harmless, not the other way round. def _actual_fields() -> set[str]: diff --git a/tests/test_registry_migration.py b/tests/test_registry_migration.py index 2643620..7952ade 100644 --- a/tests/test_registry_migration.py +++ b/tests/test_registry_migration.py @@ -152,7 +152,7 @@ async def test_longest_suffix_wins_over_shorter_shadow( async def test_a_skipped_collision_raises_a_repair_issue( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """Audit VA-06: skipping the legacy entity leaves exactly the one that + """Skipping the legacy entity leaves exactly the one that carries the history, entity id and customisations orphaned, while the integration binds the entity that was already there. Nothing destroys data here, and nothing should -- which of the two is worth keeping is @@ -177,7 +177,7 @@ async def test_the_migration_log_names_no_entity_id( fake_charger: FakeWattpilot, caplog: pytest.LogCaptureFixture, ) -> None: - """A legacy entity ID can carry the serial, as the new ones do (R1-02).""" + """A legacy entity ID can carry the serial, as the new ones do.""" caplog.set_level(logging.INFO) entry = MockConfigEntry(domain=DOMAIN, data=V2_LOCAL_DATA, version=2) entry.add_to_hass(hass) @@ -211,7 +211,7 @@ async def test_the_collision_notice_follows_the_entities_deleted_from_it( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """The notice says to delete the entities it lists. Doing so left it up, - naming them, until the next reload (reaudit R1-19).""" + naming them, until the next reload.""" entry = MockConfigEntry(domain=DOMAIN, data=V2_LOCAL_DATA, version=2) entry.add_to_hass(hass) seed(hass, entry, "number", "123456-amp") @@ -237,8 +237,7 @@ async def test_the_collision_notice_follows_a_renamed_entity( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """The notice watched entity IDs: renamed, a listed entity was still - named by its old ID, and deleting it then left the notice up (reaudit - RF-E-02).""" + named by its old ID, and deleting it then left the notice up.""" entry = MockConfigEntry(domain=DOMAIN, data=V2_LOCAL_DATA, version=2) entry.add_to_hass(hass) seed(hass, entry, "number", "123456-amp") @@ -279,7 +278,7 @@ async def test_the_collision_notice_goes_with_its_entry( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """Deleting the entry to start over left the notice about its entities - standing until Home Assistant restarted (audit A18-23).""" + standing until Home Assistant restarted.""" entry = MockConfigEntry(domain=DOMAIN, data=V2_LOCAL_DATA, version=2) entry.add_to_hass(hass) seed(hass, entry, "number", "123456-amp") diff --git a/tests/test_release_notes.py b/tests/test_release_notes.py index e2db52c..d381acb 100644 --- a/tests/test_release_notes.py +++ b/tests/test_release_notes.py @@ -113,7 +113,7 @@ def test_the_next_release_notes_carry_no_quality_figures() -> None: def test_every_version_heading_has_its_comparison_link() -> None: """Keep a Changelog ends with one link per version, and "Unreleased" has to compare against the newest one. 0.1.2b1 got its section and no link, - and Unreleased still diffed against 0.1.1 (audit A13-03).""" + and Unreleased still diffed against 0.1.1.""" changelog = CHANGELOG.read_text(encoding="utf-8") versions = re.findall(r"^## \[([^\]]+)\]", changelog, flags=re.MULTILINE) links = dict(re.findall(r"^\[([^\]]+)\]: (\S+)$", changelog, flags=re.MULTILINE)) diff --git a/tests/test_select.py b/tests/test_select.py index 11818e0..a59e520 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -264,7 +264,7 @@ async def test_an_unknown_value_is_reported_once_until_a_known_one( caplog: pytest.LogCaptureFixture, pushes: list[object], ) -> None: - """Logged on every push before (post-check F31-03).""" + """Logged on every push before.""" fake_charger._properties["ct"] = "default" await make_select(hass, fake_charger, "ct") for value in pushes: diff --git a/tests/test_sensor.py b/tests/test_sensor.py index 92f4003..04299d3 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -32,7 +32,7 @@ ENTRY_ID = "entry1" ENERGY_SPLIT_UIDS = {"whs", "whb", "whg", "who"} SOURCE_POWER_UIDS = ("ws", "wb", "wg", "wo") -# Which source each one is named after, so a swap cannot pass (F31-05). +# Which source each one is named after, so a swap cannot pass. SOURCE_POWER_NAMES = dict( zip(SOURCE_POWER_UIDS, ("solar", "battery", "grid", "other_sources"), strict=True) ) @@ -382,7 +382,7 @@ async def test_inverter_namespace_unescapes_label( async def test_inverter_namespace_reads_object_payloads( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """The vendor client hands nested payloads over as objects, not dicts. + """The client hands nested payloads over as objects, not dicts. Fields the object does not carry must read as None instead of raising.""" fake_charger._properties["cci"] = SimpleNamespace(label="Symo", model="Gen24") sensor = await make_sensor(hass, fake_charger, "cci") @@ -455,7 +455,7 @@ async def test_a_missing_card_slot_leaks_no_names_into_the_log( """Ten card sensors exist, and a charger may report fewer cards. Indexing past the end raised, and the generic handler logged the whole payload with %r -- card holder names included, in a log the diagnostics redaction - never touches (audit A12-05). The absent slot is a normal state, not a + never touches. The absent slot is a normal state, not a fault worth a warning. """ fake_charger._properties["cards"] = [ @@ -482,7 +482,7 @@ async def test_the_first_card_slot_the_charger_lacks_is_already_empty( """The boundary itself: two cards means slot 2 is the first missing one. Off by one here and the sensor indexes past the end again, which is the - exact path that put holder names into the log (audit A12-05). + exact path that put holder names into the log. """ fake_charger._properties["cards"] = [ {"name": "Alex Example", "cardId": True, "energy": 1.0}, @@ -519,10 +519,10 @@ async def test_malformed_value_keeps_previous_state( assert sensor.native_value == 1 # The warning is the only trace a dropped push leaves; it has to name # which sensor, what shape arrived, which charger key and what went wrong. - # The sensor by its key: the entity ID carries the serial (R1-02). + # The sensor by its key: the entity ID carries the serial. # It deliberately does NOT carry the value, nor the exception's message, # which can quote the value too: nothing redacts this sink, and a card - # list logged that way put holder names into it (audit A12-05). + # list logged that way put holder names into it. # Compared whole rather than as a substring: a substring check passes just # as happily when the message has grown extra text around it. assert any( @@ -566,7 +566,7 @@ async def test_a_flex_temperature_with_empty_slots_shows_the_hottest( async def test_a_temperature_that_is_no_number_is_skipped( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """max() over a NaN is NaN (post-check PC6-06).""" + """max() over a NaN is NaN.""" fake_charger._properties["tma"] = [float("nan"), 40.0] sensor = await make_sensor(hass, fake_charger, "tma") assert sensor.native_value == 40.0 @@ -707,7 +707,7 @@ async def test_allowed_current_is_unknown_while_no_car_charges( async def test_a_null_start_value_is_applied_when_the_property_exists( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: - """Audit VA-13: the initial read skipped None, treating "no value" and + """The initial read skipped None, treating "no value" and "the value None" as the same thing. For `trx` they are not -- None is the enum's own "No Transaction" -- so the entity sat at unknown until the charger happened to push the field again. @@ -794,7 +794,7 @@ async def test_one_bad_price_costs_only_itself( hass: HomeAssistant, fake_charger: FakeWattpilot, bad: dict[str, Any] ) -> None: """A bad entry in the price list took the current price with it, and an - out-of-range start escaped the bounded catch (reaudit RF-D-03).""" + out-of-range start escaped the bounded catch.""" fake_charger._properties.update({"awcp": PRICE_HOUR, "awpl": [bad, PRICE_HOUR]}) sensor = await make_sensor(hass, fake_charger, "awcp") @@ -869,7 +869,7 @@ def test_per_phase_readings_stay_out_of_the_recorder() -> None: # json.loads makes NaN and infinity of what the charger could send. ({"type": 0, "value": 2217901}, float("inf"), None), ({"type": 1, "value": float("nan")}, 14857, None), - # Each finite, their difference not (post-check PC6-01). + # Each finite, their difference not. ({"type": 0, "value": -1.7e308}, 1.7e308, None), ], ) @@ -936,8 +936,8 @@ async def test_average_power_is_zero_while_nothing_flows( ) -> None: """tpa keeps the last session's average after charging -- recorded on firmware 42.5 with the car plugged in and every power slot at 0, and - reported on a Flex -- so threshold automations kept firing for hours - (audit A18-13). Nothing flowing when the entry starts is already idle.""" + reported on a Flex -- so threshold automations kept firing for hours. + Nothing flowing when the entry starts is already idle.""" fake_charger._properties.update({"tpa": 4121.5, "nrg": nrg_reading(0)}) sensor = await make_sensor(hass, fake_charger, "tpa") @@ -989,7 +989,7 @@ async def test_each_phase_reading_has_a_sensor_of_its_own( ) def test_the_numeric_rule_is_home_assistants(description: Any) -> None: """The conversion checks for a number exactly where Home Assistant - will insist on one when it writes the state (reaudit K-A).""" + will insist on one when it writes the state.""" entity = SensorEntity() entity.entity_description = description @@ -1038,8 +1038,7 @@ async def test_the_current_limit_is_the_hardware_ceiling_not_the_request( hass: HomeAssistant, fake_charger: FakeWattpilot ) -> None: """Measured: currentLimitMax stayed 16 A while PV surplus charging - requested 6; the fixture has all limits at 16 and cannot tell them apart - (post-check F31-05).""" + requested 6; the fixture has all limits at 16 and cannot tell them apart.""" fake_charger._properties["cll"] = { "adapterCurrentLimit": 20, "cableCurrentLimit": 32, diff --git a/tests/test_services.py b/tests/test_services.py index 48cb23c..c9c0c81 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -81,7 +81,7 @@ async def test_set_goe_cloud_enable_logs_no_part_of_the_key( fake_charger: FakeWattpilot, caplog: pytest.LogCaptureFixture, ) -> None: - """Audit VA-14: this used to log the key's first and last four characters + """This used to log the key's first and last four characters plus the vendor URL, which embeds the full serial -- and the old test pinned those eight characters as intended behaviour. Neither earns its place: "enabled" is what an operator needs, and the key and URL are diff --git a/tests/test_update.py b/tests/test_update.py index bb58ff9..ada420a 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -5,6 +5,7 @@ from typing import TYPE_CHECKING import pytest +from homeassistant.exceptions import ServiceValidationError from custom_components.wattpilot.descriptions import UPDATE_DESCRIPTIONS from custom_components.wattpilot.hub import WattpilotHub @@ -140,7 +141,7 @@ async def test_installed_version_reacts_to_its_own_push( # What a charger could send instead of a version string. Home Assistant # compares versions through a cache, and an unhashable one raised there, so -# the entity was never added (post-check PC7-02). +# the entity was never added. ODD_VERSIONS = {"object": {"v": "43.0"}, "list_of_lists": [["43.0"]], "number": 43} @@ -165,3 +166,28 @@ async def test_an_odd_installed_version_is_unknown( assert entity.installed_version is None fake_charger.push("fwv", ODD_VERSIONS[kind]) assert entity.installed_version is None + + +@pytest.mark.parametrize( + ("offered", "version"), + [(["43.0"], "43.1"), ([], "43.1"), ([], None), (None, None)], +) +async def test_a_version_the_charger_does_not_offer_is_refused( + hass: HomeAssistant, + fake_charger: FakeWattpilot, + offered: list[str] | None, + version: str | None, +) -> None: + """A typo in an action or automation sent whatever it named to the + charger as the version to install; with nothing on offer, a request for + the latest one sent the installed version.""" + fake_charger._properties["fwv"] = "42.5" + if offered is None: + del fake_charger._properties["onv"] + else: + fake_charger._properties["onv"] = offered + entity = await make_update(hass, fake_charger) + with pytest.raises(ServiceValidationError) as raised: + await entity.async_install(version=version, backup=False) + assert raised.value.translation_key == "firmware_not_offered" + assert fake_charger.install_calls == []