Fix listener callback contracts across domain APIs (#72)#80
Merged
Conversation
The runtime contract for every public on_* listener decorator is (old, new), enforced by Entity._schedule_value. Several domain docstrings still described the older zero-argument or single-value shape, and every public on_* method used func: Any) -> Any so type checking did not protect users from registering callbacks with the wrong arity. Changes: - Standardise every domain listener docstring on the (old, new) contract (light, switch, binary_sensor, sensor, cover, lock, climate, humidifier, media_player, timer, vacuum, valve, scene, air_quality, fan). - Replace func: Any) -> Any with func: ValueChangeHandler) -> ValueChangeHandler on every public on_* method so mypy users get arity protection at the boundary. - Drop now-unused typing.Any imports. - Add regression tests in tests/test_granular_events.py covering the attribute, state-transition, state-value, media-change, and timer-event listener shapes, plus a test pinning that a zero-argument callback raises a logged TypeError and does not silently no-op other handlers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue validity assessment
Closes #72.
Verified valid and reproducible by reading the source:
Entity._schedule_value(src/haclient/entity/base.py:175-189) dispatches every granular listener with two positional arguments(old, new).on_*listener decorators across domain modules documented a zero-argument or single-value callback. A user following those docs would register a callable that raisesTypeErrorwhen the event fires; the exception is caught and logged by_schedule_value, so from the caller's perspective the listener silently does nothing.on_*method also usedfunc: Any) -> Any, so mypy could not catch the mismatch at the boundary.The runtime behaviour is already what the (small majority of) docs and every existing test expect, so the issue's recommended path — keep
(old, new)and align docs + types with it — is correct.Fix
Smallest correct change:
(old, new)contract:light,switch,binary_sensor,sensor,cover,lock,climate,humidifier,media_player,timer,vacuum,valve,scene,air_quality,fan.func: Any) -> Anywithfunc: ValueChangeHandler) -> ValueChangeHandleron every publicon_*method.ValueChangeHandlerisCallable[[Any, Any], Any]defined inhaclient.entity.base, so existing internal storage (list[ValueChangeHandler]) needed no change.typing.Anyimports.Timer's event-driven
on_finished/on_cancelledlisteners also receive two positional arguments ((entity_id, event_data)), which is structurally identical toValueChangeHandler, so they get the same typed signature.The
eventdomain'son_event(self, func: Callable[[str], Any])is a different, already-typed contract and was intentionally not touched.Tests / checks
tests/test_granular_events.pyalready pins the(old, new)shape across every domain. Six new regression tests were added at the bottom:test_state_transition_callback_receives_old_and_new— pins the transition-listener contract.test_attr_change_callback_receives_old_and_new— pins the attribute-listener contract.test_state_value_callback_receives_old_and_new— pins the state-value-listener contract.test_zero_argument_callback_is_logged_and_skipped— verifies that a misused zero-argument callback raisesTypeError, gets logged at ERROR, and does not break other handlers registered for the same event.test_media_change_callback_receives_old_and_new— pins theMediaPlayer.on_media_change(NowPlaying, NowPlaying)contract.test_timer_event_callback_receives_entity_id_and_data— pins the timer event-listener(entity_id, data)contract.Validation run on this branch:
ruff check src tests— passruff format --check src tests— passmypy src(strict) —Success: no issues found in 37 source filespytest tests/ --cov=haclient --cov-fail-under=95—291 passed, total coverage97.07%