From 05a0afd7eb3be11d60aa764be28f80e5e8462496 Mon Sep 17 00:00:00 2001 From: Eric Fetty Date: Fri, 27 Mar 2026 00:43:18 -0500 Subject: [PATCH 01/14] feat: Implement initial HTD client architecture including base, MCA, and Lync clients, data models, constants, and tests. --- .lync_clone | 1 + htd_client/base_client.py | 58 +++++++-- htd_client/constants.py | 6 +- htd_client/lync_client.py | 172 ++++++++++++--------------- htd_client/mca_client.py | 82 ++++++------- htd_client/models.py | 10 +- tests/test_base_client_additional.py | 1 + tests/test_models_coverage.py | 2 +- tests/test_utils_coverage.py | 6 +- 9 files changed, 178 insertions(+), 160 deletions(-) create mode 160000 .lync_clone diff --git a/.lync_clone b/.lync_clone new file mode 160000 index 0000000..527a4bf --- /dev/null +++ b/.lync_clone @@ -0,0 +1 @@ +Subproject commit 527a4bfc85386f27424ca4b2869d35aecd6a6cfe diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 4f9d8ad..915d2ab 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -95,7 +95,7 @@ async def async_connect(self): self._buffer = bytearray() self._zone_data = {} self._zones_loaded = 0 - self._zone_data = {} + self._source_names = {} self._connection = None self._disconnected = False @@ -330,14 +330,16 @@ def _parse_command(self, zone, cmd, data): # remove the extra null bytes elif cmd == HtdCommonCommands.ZONE_NAME_RECEIVE_COMMAND: - name = str(data[0:11].decode().rstrip('\0')).lower() - self._zone_data[zone].name = name - - elif cmd == HtdCommonCommands.SOURCE_NAME_RECEIVE_COMMAND: - source = data[11] - name = str(data[0:10].decode().rstrip('\0')).lower() - # self.zone_info[zone]['source_list'][source] = name - # self.source_info[zone][name] = source + name = str(data[0:11].decode(errors="ignore").rstrip('\0')).lower() + if self.has_zone_data(zone): + self._zone_data[zone].name = name + + elif cmd == HtdCommonCommands.SOURCE_NAME_RECEIVE_COMMAND or cmd == HtdCommonCommands.ZONE_SOURCE_NAME_RECEIVE_COMMAND_LYNC: + source = data[11] + 1 + name = str(data[0:10].decode(errors="ignore").rstrip('\0')).lower() + self._source_names[source] = name + if self.has_zone_data(zone): + self._zone_data[zone].source_name = name # # elif cmd == HtdCommonCommands.MP3_ON_RECEIVE_COMMAND: # self.mp3_status['state'] = 'on' @@ -389,7 +391,7 @@ def _parse_zone(self, zone_number: int, zone_data: bytearray) -> ZoneDetail | No HtdConstants.POWER_STATE_TOGGLE_INDEX ) zone.mute = htd_client.utils.is_bit_on(state_toggles, HtdConstants.MUTE_STATE_TOGGLE_INDEX) - zone.mode = htd_client.utils.is_bit_on(state_toggles, HtdConstants.MODE_STATE_TOGGLE_INDEX) + zone.dnd = htd_client.utils.is_bit_on(state_toggles, HtdConstants.DND_STATE_TOGGLE_INDEX) zone.source = zone_data[HtdConstants.SOURCE_ZONE_DATA_INDEX] + HtdConstants.SOURCE_QUERY_OFFSET zone.volume = volume @@ -497,6 +499,10 @@ def get_source_count(self) -> int: """ return self._model_info['sources'] + def get_source_name(self, source: int) -> str: + """Get the name of a source if it has been fetched.""" + return self._source_names.get(source, f"Source {source}") + def get_zone(self, zone: int): """ Query a zone and return `ZoneDetail` @@ -593,3 +599,35 @@ async def async_balance_left(self, zone: int): @abstractmethod async def async_balance_right(self, zone: int): pass + + @abstractmethod + async def async_set_dnd(self, zone: int, dnd: bool): + pass + + @abstractmethod + async def async_set_echo(self, echo: bool): + pass + + @abstractmethod + async def async_query_id(self): + pass + + @abstractmethod + async def async_query_all_zone_status(self): + pass + + @abstractmethod + async def async_query_zone_name(self, zone: int): + pass + + @abstractmethod + async def async_query_source_name(self, source: int): + pass + + @abstractmethod + async def async_set_zone_name(self, zone: int, name: str): + pass + + @abstractmethod + async def async_set_source_name(self, source: int, name: str): + pass diff --git a/htd_client/constants.py b/htd_client/constants.py index b0b0e02..faaf868 100644 --- a/htd_client/constants.py +++ b/htd_client/constants.py @@ -105,7 +105,7 @@ class HtdConstants: # indexes of each state toggle POWER_STATE_TOGGLE_INDEX = 0 MUTE_STATE_TOGGLE_INDEX = 1 - MODE_STATE_TOGGLE_INDEX = 2 + DND_STATE_TOGGLE_INDEX = 2 # the byte index for where to locate the corresponding setting SOURCE_ZONE_DATA_INDEX = 4 @@ -134,6 +134,7 @@ class HtdCommonCommands: MP3_ARTIST_NAME_RECEIVE_COMMAND = 0x12 MP3_ON_RECEIVE_COMMAND = 0x13 MP3_OFF_RECEIVE_COMMAND = 0x14 + QUERY_ID_CODE_RECEIVE_COMMAND = 0x08 ERROR_RECEIVE_COMMAND = 0x1b EXPECTED_MESSAGE_LENGTH_MAP = { @@ -150,6 +151,7 @@ class HtdCommonCommands: MP3_ON_RECEIVE_COMMAND: 1, MP3_OFF_RECEIVE_COMMAND: 17, ERROR_RECEIVE_COMMAND: 9, + QUERY_ID_CODE_RECEIVE_COMMAND: 1, } class HtdMcaConstants: @@ -171,8 +173,10 @@ class HtdLyncCommands: BALANCE_SETTING_CONTROL_COMMAND_CODE = 0x16 TREBLE_SETTING_CONTROL_COMMAND_CODE = 0x17 BASS_SETTING_CONTROL_COMMAND_CODE = 0x18 + SET_ECHO_COMMAND_CODE = 0x19 SET_AUDIO_TO_DEFAULT_COMMAND_CODE = 0x1c SET_NAME_TO_DEFAULT_COMMAND_CODE = 0x1e + QUERY_ID_CODE = 0x08 MP3_FAST_FORWARD_COMMAND_CODE = 0x0a MP3_PLAY_PAUSE_COMMAND_CODE = 0x0b diff --git a/htd_client/lync_client.py b/htd_client/lync_client.py index f8f6069..159d6b5 100644 --- a/htd_client/lync_client.py +++ b/htd_client/lync_client.py @@ -386,99 +386,79 @@ async def async_set_balance(self, zone: int, balance: int): balance ) - # def query_zone_name(self, zone: int) -> str: - # """ - # Query a zone and return `ZoneDetail` - # - # Args: - # zone (int): the zone - # - # Returns: - # ZoneDetail: a ZoneDetail instance representing the zone requested - # - # Raises: - # Exception: zone X is invalid - # """ - # - # # htd_client.utils.validate_zone(zo+ne) - # - # self._send_and_validate( - # zone, - # HtdLyncCommands.QUERY_ZONE_NAME_COMMAND_CODE, - # 0 - # ) - - # def query_source_name(self, source: int, zone: int) -> str: - # source_offset = source - 1 - # - # self._send_and_validate( - # zone, HtdLyncCommands.QUERY_SOURCE_NAME_COMMAND_CODE, source_offset - # ) - # - # source_name_bytes = response[4:14].strip(b'\x00') - # source_name = htd_client.utils.decode_response(source_name_bytes) - # - # return source_name - - # def set_source_name(self, source: int, zone: int, name: str): - # """ - # Query a zone and return `ZoneDetail` - # - # Args: - # source (int): the source - # zone: (int): the zone - # name (str): the name of the source (max length of 7) - # - # Returns: - # bytes: a ZoneDetail instance representing the zone requested - # - # Raises: - # Exception: zone X is invalid - # """ - # - # # htd_client.utils.validate_zone(zone) - # - # extra_data = bytes( - # [ord(char) for char in name] + [0] * (11 - len(name)) - # ) - # - # self._send_and_validate( - # zone, - # HtdLyncCommands.SET_SOURCE_NAME_COMMAND_CODE, - # source) - # extra_data - # ) - # - # def get_zone_names(self): - # self._send_cmd( - # 1, - # HtdLyncCommands.QUERY_ZONE_NAME_COMMAND_CODE, - # 1 - # ) - # def set_zone_name(self, zone: int, name: str): - # """ - # Query a zone and return `ZoneDetail` - # - # Args: - # zone: (int): the zone - # name (str): the name of the source (max length of 7) - # - # Returns: - # bytes: a ZoneDetail instance representing the zone requested - # - # Raises: - # Exception: zone X is invalid - # """ - # - # # htd_client.utils.validate_zone(zone) - # - # extra_data = bytes( - # [ord(char) for char in name] + [0] * (11 - len(name)) - # ) - # - # self._send_and_validate( - # zone, - # HtdLyncCommands.SET_ZONE_NAME_COMMAND_CODE, - # 0) - # extra_data - # ) + async def async_query_all_zone_status(self): + """Query status of all zones""" + return await self._send_cmd( + 0, + HtdLyncCommands.QUERY_COMMAND_CODE, + 0 + ) + + async def async_set_dnd(self, zone: int, dnd: bool): + """Set Do Not Disturb state on/off.""" + return await self._async_send_and_validate( + lambda z: z.dnd == dnd, + zone, + HtdLyncCommands.COMMON_COMMAND_CODE, + HtdLyncCommands.DND_ON_COMMAND_CODE if dnd else HtdLyncCommands.DND_OFF_COMMAND_CODE + ) + + async def async_set_echo(self, echo: bool): + """Set whether the unit should echo commands back.""" + return await self._send_cmd( + 0, + HtdLyncCommands.SET_ECHO_COMMAND_CODE if hasattr(HtdLyncCommands, "SET_ECHO_COMMAND_CODE") else 0x19, + 0xFF if echo else 0x00 + ) + + async def async_query_id(self): + """Query device ID.""" + return await self._send_cmd( + 0, + HtdLyncCommands.QUERY_ID_CODE if hasattr(HtdLyncCommands, "QUERY_ID_CODE") else 0x08, + 0x00 + ) + + async def async_query_zone_name(self, zone: int): + """Query the name of a zone.""" + await self._send_cmd( + zone, + HtdLyncCommands.QUERY_ZONE_NAME_COMMAND_CODE, + 0 + ) + + async def async_query_source_name(self, source: int): + """Query the name of a source.""" + await self._send_cmd( + 1, + HtdLyncCommands.QUERY_SOURCE_NAME_COMMAND_CODE, + source + ) + + async def async_set_source_name(self, source: int, name: str): + """Set the name of a source (max 10 chars).""" + trimmed = name[:10] + encoded = list(trimmed.encode("ascii", errors="ignore")) + encoded.extend([0] * (10 - len(encoded))) + extra_data = bytearray(encoded + [0]) + + await self._send_cmd( + 0, + HtdLyncCommands.SET_SOURCE_NAME_COMMAND_CODE, + source, + extra_data + ) + + async def async_set_zone_name(self, zone: int, name: str): + """Set the name of a zone (max 10 chars).""" + trimmed = name[:10] + encoded = list(trimmed.encode("ascii", errors="ignore")) + encoded.extend([0] * (10 - len(encoded))) + extra_data = bytearray(encoded + [0]) + + await self._send_cmd( + zone, + HtdLyncCommands.SET_ZONE_NAME_COMMAND_CODE, + 0, + extra_data + ) diff --git a/htd_client/mca_client.py b/htd_client/mca_client.py index 3d320da..682dc53 100644 --- a/htd_client/mca_client.py +++ b/htd_client/mca_client.py @@ -427,48 +427,40 @@ async def async_balance_right(self, zone: int): HtdMcaCommands.BALANCE_RIGHT_COMMAND ) - # def get_source_names(self): - # """ - # Query a zone and return `ZoneDetail` - # - # Returns: - # Dict[int, str]: a dictionary where each zone has a string value - # of the source name - # """ - # - # self._send_cmd( - # 0, - # HtdMcaCommands.QUERY_SOURCE_NAME_COMMAND_CODE, - # 0 - # ) - # - # def set_source_name(self, source: int, name: str): - # """ - # Query a zone and return `ZoneDetail` - # - # Args: - # source (int): the source - # name (str): the name of the source (max length of 7) - # - # Returns: - # ZoneDetail: a ZoneDetail instance representing the zone requested - # - # Raises: - # Exception: zone X is invalid - # """ - # - # # htd_client.utils.validate_zone(zone) - # - # extra_data = bytearray( - # [ord(char) for char in name] + [0] * (7 - len(name)) + [0x00] - # ) - # - # self._send_cmd( - # 0, - # HtdMcaCommands.SET_SOURCE_NAME_COMMAND_CODE, - # source, - # extra_data - # ) - # - # def get_zone_names(self): - # pass + async def async_set_dnd(self, zone: int, dnd: bool): + raise NotImplementedError("MCA does not support DND.") + + async def async_set_echo(self, echo: bool): + raise NotImplementedError("MCA does not support echo setting.") + + async def async_query_id(self): + raise NotImplementedError("MCA does not support ID query.") + + async def async_query_all_zone_status(self): + raise NotImplementedError("MCA does not support global status query.") + + async def async_query_zone_name(self, zone: int): + raise NotImplementedError("MCA does not support querying zone names.") + + async def async_set_zone_name(self, zone: int, name: str): + raise NotImplementedError("MCA does not support setting zone names.") + + async def async_query_source_name(self, source: int): + """Query a source name.""" + await self._send_cmd( + 0, + HtdMcaCommands.QUERY_SOURCE_NAME_COMMAND_CODE, + 0 + ) + + async def async_set_source_name(self, source: int, name: str): + """Set the name of a source (max 7 chars).""" + extra_data = bytearray( + [ord(char) for char in name[:7]] + [0] * (7 - len(name[:7])) + [0x00] + ) + await self._send_cmd( + 0, + HtdMcaCommands.SET_SOURCE_NAME_COMMAND_CODE, + source, + extra_data + ) diff --git a/htd_client/models.py b/htd_client/models.py index 17ffdfe..2ce698c 100644 --- a/htd_client/models.py +++ b/htd_client/models.py @@ -6,26 +6,28 @@ class ZoneDetail: enabled: bool = True power: bool = None mute: bool = None - mode: bool = None + dnd: bool = None source: int = None volume: int = None treble: int = None bass: int = None balance: int = None name: str = None + source_name: str = None def __str__(self): return ( - "zone_number = %s, enabled = %s, name = %s, power = %s, " - "mute = %s, mode = %s, source = %s, volume = %s, " + "zone_number = %s, enabled = %s, name = %s, source_name = %s, power = %s, " + "mute = %s, dnd = %s, source = %s, volume = %s, " "treble = %s, bass = %s, balance = %s" % ( self.number, self.enabled, self.name, + self.source_name, self.power, self.mute, - self.mode, + self.dnd, self.source, self.volume, self.treble, diff --git a/tests/test_base_client_additional.py b/tests/test_base_client_additional.py index ab3a6c2..11f2917 100644 --- a/tests/test_base_client_additional.py +++ b/tests/test_base_client_additional.py @@ -23,6 +23,7 @@ def client(): c._connection = MagicMock() c._subscribers = set() c._zone_data = {} + c._source_names = {} c._socket_lock = asyncio.Lock() c._callback_lock = asyncio.Lock() c._connected = True diff --git a/tests/test_models_coverage.py b/tests/test_models_coverage.py index 29de7c7..7641662 100644 --- a/tests/test_models_coverage.py +++ b/tests/test_models_coverage.py @@ -2,7 +2,7 @@ def test_zone_detail_str(): zone = ZoneDetail(1, enabled=True, name="Kitchen", power=True, mute=False, - mode=True, source=1, volume=30, treble=0, bass=0, balance=0) + dnd=True, source=1, volume=30, treble=0, bass=0, balance=0) s = str(zone) assert "zone_number = 1" in s assert "name = Kitchen" in s diff --git a/tests/test_utils_coverage.py b/tests/test_utils_coverage.py index 937d649..c01d530 100644 --- a/tests/test_utils_coverage.py +++ b/tests/test_utils_coverage.py @@ -32,10 +32,10 @@ def test_stringify_bytes(): def test_convert_volume_to_raw(): # MAX_RAW_VOLUME = 256, MAX_VOLUME = 60 - assert convert_volume_to_raw(0) == 0 + assert convert_volume_to_raw(60) == 0 # MAX_RAW_VOLUME - (MAX_VOLUME - volume) - # 60 -> 256 - (60 - 60) = 256 - assert convert_volume_to_raw(60) == 256 + # 0 -> 256 - (60 - 0) = 196 + assert convert_volume_to_raw(0) == 196 # 30 -> 256 - (60 - 30) = 226 assert convert_volume_to_raw(30) == 226 From 59c5ce5b52a5f681b81fbc5c13300aa06e50bade Mon Sep 17 00:00:00 2001 From: Adam Kirschner Date: Mon, 16 Feb 2026 11:09:50 -0500 Subject: [PATCH 02/14] feat: update MCA audio controls to use native steps and range - Update MCA bass/treble/balance range to -12..12 - Enforce step size of 4 for bass/treble and 6 for balance - Remove scaling logic from client and HA integration - Add unit tests for MCA audio control logic and rounding - Bump version to 0.0.26 --- htd_client/__init__.py | 6 +- htd_client/base_client.py | 15 +- htd_client/constants.py | 31 ++- htd_client/lync_client.py | 56 ++++-- htd_client/mca_client.py | 266 +++++++++++++++++++++++-- pyproject.toml | 2 +- tests/test_base_client_gap_coverage.py | 58 ++++++ tests/test_bass_treble.py | 209 +++++++++++++++++++ tests/test_lync_client_coverage.py | 47 +++-- tests/test_lync_options_coverage.py | 135 +++++++++++++ tests/test_mca_bass_treble_set.py | 167 ++++++++++++++++ tests/test_mca_client_coverage.py | 42 ++-- tests/test_mca_options_coverage.py | 99 +++++++++ tests/test_utils_coverage_gap.py | 40 ++++ 14 files changed, 1084 insertions(+), 89 deletions(-) create mode 100644 tests/test_base_client_gap_coverage.py create mode 100644 tests/test_bass_treble.py create mode 100644 tests/test_lync_options_coverage.py create mode 100644 tests/test_mca_bass_treble_set.py create mode 100644 tests/test_mca_options_coverage.py create mode 100644 tests/test_utils_coverage_gap.py diff --git a/htd_client/__init__.py b/htd_client/__init__.py index ab2d74b..d4680e9 100644 --- a/htd_client/__init__.py +++ b/htd_client/__init__.py @@ -28,6 +28,7 @@ async def async_get_client( serial_address: str = None, network_address: Tuple[str, int] = None, loop: asyncio.AbstractEventLoop = None, + retry_attempts: int = HtdConstants.DEFAULT_RETRY_ATTEMPTS, ) -> BaseClient: """ Create a new client object. @@ -36,6 +37,7 @@ async def async_get_client( network_address (str): The address to communicate with over TCP. serial_address (str): The location of the serial port. loop (asyncio.AbstractEventLoop): The event loop to use. + retry_attempts (int): Number of times to retry a command before failing. Returns: HtdClient: The new client object. @@ -53,6 +55,7 @@ async def async_get_client( model_info, network_address=network_address, serial_address=serial_address, + retry_attempts=retry_attempts, ) elif model_info["kind"] == HtdDeviceKind.lync: @@ -61,10 +64,11 @@ async def async_get_client( model_info, network_address=network_address, serial_address=serial_address, + retry_attempts=retry_attempts, ) else: - raise ValueError(f"Unknown Device Kind: {model_info["kind"]}") + raise ValueError(f"Unknown Device Kind: {model_info['kind']}") await client.async_connect() diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 915d2ab..e779b73 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -186,7 +186,11 @@ async def _async_reconnect(self): async def async_wait_until_ready(self): - pass + start_time = time.time() + while not self._ready: + if time.time() - start_time > self._socket_timeout_sec: + raise Exception("Timed out waiting for device to be ready") + await asyncio.sleep(0.1) def has_zone_data(self, zone: int): return zone in self._zone_data @@ -284,7 +288,6 @@ def _process_next_command(self, data: bytes): def _parse_command(self, zone, cmd, data): if cmd == HtdCommonCommands.KEYPAD_EXISTS_RECEIVE_COMMAND: - print(f"DEBUG: inside _parse_command _zone_data id: {id(self._zone_data)}") # if len(self._zone_data) == 0: # this is zone 0 with all zone data # second byte is zone 1 - 8 @@ -575,6 +578,10 @@ async def async_power_on(self, zone: int): @abstractmethod async def async_power_off(self, zone: int): pass + + @abstractmethod + async def async_set_bass(self, zone: int, bass: int): + pass @abstractmethod async def async_bass_up(self, zone: int): @@ -584,6 +591,10 @@ async def async_bass_up(self, zone: int): async def async_bass_down(self, zone: int): pass + @abstractmethod + async def async_set_treble(self, zone: int, treble: int): + pass + @abstractmethod async def async_treble_up(self, zone: int): pass diff --git a/htd_client/constants.py b/htd_client/constants.py index faaf868..97304d2 100644 --- a/htd_client/constants.py +++ b/htd_client/constants.py @@ -82,14 +82,26 @@ class HtdConstants: VOLUME_OFFSET = MAX_RAW_VOLUME - MAX_VOLUME - MIN_BASS = -10 - MAX_BASS = 10 - - MIN_TREBLE = -10 - MAX_TREBLE = 10 - - MIN_BALANCE = -18 - MAX_BALANCE = 18 + # Lync Constants + LYNC_MIN_BASS = -10 + LYNC_MAX_BASS = 10 + LYNC_MIN_TREBLE = -10 + LYNC_MAX_TREBLE = 10 + LYNC_MIN_BALANCE = -18 + LYNC_MAX_BALANCE = 18 + + # MCA Constants + # Raw values exposed to the user (-12 to 12) + MCA_MIN_BASS = -12 + MCA_MAX_BASS = 12 + MCA_MIN_TREBLE = -12 + MCA_MAX_TREBLE = 12 + MCA_MIN_BALANCE = -12 + MCA_MAX_BALANCE = 12 + + # Step size exposed to user + MCA_BASS_TREBLE_STEP = 4 + MCA_BALANCE_STEP = 6 # each message we get is chunked at 14 bytes MESSAGE_CHUNK_SIZE = 14 @@ -218,6 +230,9 @@ class HtdLyncConstants: BASS_COMMAND_OFFSET = 0x80 TREBLE_COMMAND_OFFSET = 0x80 + + STATUS_REFRESH_CODE = 0x1F + class HtdMcaCommands: diff --git a/htd_client/lync_client.py b/htd_client/lync_client.py index 159d6b5..7af58ff 100644 --- a/htd_client/lync_client.py +++ b/htd_client/lync_client.py @@ -237,7 +237,7 @@ async def async_bass_up(self, zone: int): current_zone = self.get_zone(zone) new_bass = current_zone.bass + 1 - if new_bass >= HtdConstants.MAX_BASS: + if new_bass > HtdConstants.LYNC_MAX_BASS: return await self.async_set_bass(zone, new_bass) @@ -253,11 +253,12 @@ async def async_bass_down(self, zone: int): current_zone = self.get_zone(zone) new_bass = current_zone.bass - 1 - if new_bass < HtdConstants.MIN_BASS: + if new_bass < HtdConstants.LYNC_MIN_BASS: return await self.async_set_bass(zone, new_bass) + async def async_set_bass(self, zone: int, bass: int): """ Set the bass of a zone. @@ -266,17 +267,27 @@ async def async_set_bass(self, zone: int, bass: int): zone (int): the zone bass (int): the bass value to set """ + + logging.debug(f"Setting bass for zone {zone} to {bass}") + zone_info = self.get_zone(zone) if zone_info.bass == bass: return - return await self._async_send_and_validate( + encoded_bass = bass & 0xFF + + await self._send_cmd( + zone, + HtdLyncCommands.BASS_SETTING_CONTROL_COMMAND_CODE, + encoded_bass + ) + + await self._async_send_and_validate( lambda z: z.bass == bass, zone, HtdLyncCommands.COMMON_COMMAND_CODE, - HtdLyncCommands.BASS_SETTING_CONTROL_COMMAND_CODE, - bytearray([bass]) + HtdLyncConstants.STATUS_REFRESH_CODE ) async def async_treble_up(self, zone: int): @@ -290,7 +301,7 @@ async def async_treble_up(self, zone: int): current_zone = self.get_zone(zone) new_treble = current_zone.treble + 1 - if new_treble >= HtdConstants.MAX_TREBLE: + if new_treble > HtdConstants.LYNC_MAX_TREBLE: return await self.async_set_treble(zone, new_treble) @@ -306,7 +317,7 @@ async def async_treble_down(self, zone: int): current_zone = self.get_zone(zone) new_treble = current_zone.treble - 1 - if new_treble < HtdConstants.MIN_TREBLE: + if new_treble < HtdConstants.LYNC_MIN_TREBLE: return await self.async_set_treble(zone, new_treble) @@ -325,12 +336,19 @@ async def async_set_treble(self, zone: int, treble: int): if treble == zone_info.treble: return - return await self._async_send_and_validate( + encoded_treble = treble & 0xFF + + await self._send_cmd( + zone, + HtdLyncCommands.TREBLE_SETTING_CONTROL_COMMAND_CODE, + encoded_treble + ) + + await self._async_send_and_validate( lambda z: z.treble == treble, zone, HtdLyncCommands.COMMON_COMMAND_CODE, - HtdLyncCommands.TREBLE_SETTING_CONTROL_COMMAND_CODE, - bytearray([treble]) + HtdLyncConstants.STATUS_REFRESH_CODE ) async def async_balance_left(self, zone: int): @@ -344,7 +362,7 @@ async def async_balance_left(self, zone: int): current_zone = self.get_zone(zone) new_balance = current_zone.balance - 1 - if new_balance < HtdConstants.MIN_BALANCE: + if new_balance < HtdConstants.LYNC_MIN_BALANCE: return await self.async_set_balance(zone, new_balance) @@ -360,7 +378,7 @@ async def async_balance_right(self, zone: int): current_zone = self.get_zone(zone) new_balance = current_zone.balance + 1 - if new_balance > HtdConstants.MAX_BALANCE: + if new_balance > HtdConstants.LYNC_MAX_BALANCE: return await self.async_set_balance(zone, new_balance) @@ -379,11 +397,19 @@ async def async_set_balance(self, zone: int, balance: int): if balance == current_zone.balance: return - return await self._async_send_and_validate( - lambda z: z.balance == balance, + encoded_balance = balance & 0xFF + + await self._send_cmd( zone, HtdLyncCommands.BALANCE_SETTING_CONTROL_COMMAND_CODE, - balance + encoded_balance + ) + + await self._async_send_and_validate( + lambda z: z.balance == balance, + zone, + HtdLyncCommands.COMMON_COMMAND_CODE, + HtdLyncConstants.STATUS_REFRESH_CODE ) async def async_query_all_zone_status(self): diff --git a/htd_client/mca_client.py b/htd_client/mca_client.py index 682dc53..3670c4d 100644 --- a/htd_client/mca_client.py +++ b/htd_client/mca_client.py @@ -24,11 +24,14 @@ class HtdMcaClient(BaseClient): _target_volumes: Dict[int, int | None] = None + _target_bass: Dict[int, int | None] = None + _target_treble: Dict[int, int | None] = None + _target_balance: Dict[int, int | None] = None _subscribed: bool = None def __init__( self, - loop: asyncio.EventLoop, + loop: asyncio.AbstractEventLoop, model_info: HtdModelInfo, network_address: Tuple[str, int] = None, serial_address: str = None, @@ -65,6 +68,9 @@ def __init__( # we'll re-run _set_volume to get to the target self._subscribed = False self._target_volumes = {key: None for key in range(1, self._model_info["sources"] + 1)} + self._target_bass = {key: None for key in range(1, self._model_info["sources"] + 1)} + self._target_treble = {key: None for key in range(1, self._model_info["sources"] + 1)} + self._target_balance = {key: None for key in range(1, self._model_info["sources"] + 1)} async def async_connect(self): @@ -85,6 +91,24 @@ def _on_zone_update(self, zone: int = None): else: asyncio.run_coroutine_threadsafe(self._async_set_volume(zone), self._loop) + if self._target_bass[zone] is not None: + if self._zone_data[zone].bass == self._target_bass[zone]: + self._target_bass[zone] = None + else: + asyncio.run_coroutine_threadsafe(self._async_set_bass(zone), self._loop) + + if self._target_treble[zone] is not None: + if self._zone_data[zone].treble == self._target_treble[zone]: + self._target_treble[zone] = None + else: + asyncio.run_coroutine_threadsafe(self._async_set_treble(zone), self._loop) + + if self._target_balance[zone] is not None: + if self._zone_data[zone].balance == self._target_balance[zone]: + self._target_balance[zone] = None + else: + asyncio.run_coroutine_threadsafe(self._async_set_balance(zone), self._loop) + async def async_mute(self, zone: int): if self._zone_data[zone].mute: return @@ -100,6 +124,15 @@ async def async_unmute(self, zone: int): def has_volume_target(self, zone: int): return self._target_volumes[zone] is not None + def has_bass_target(self, zone: int): + return self._target_bass[zone] is not None + + def has_treble_target(self, zone: int): + return self._target_treble[zone] is not None + + def has_balance_target(self, zone: int): + return self._target_balance[zone] is not None + async def async_set_volume(self, zone: int, volume: int): existing = False @@ -311,17 +344,79 @@ async def async_bass_up(self, zone: int): zone_info = self._zone_data[zone] - new_bass = zone_info.bass + 1 - if new_bass > HtdConstants.MAX_BASS: + if not zone_info.power: + await self.async_power_on(zone) + + new_bass = zone_info.bass + HtdConstants.MCA_BASS_TREBLE_STEP + if new_bass > HtdConstants.MCA_MAX_BASS: return await self._async_send_and_validate( - lambda z: z.bass >= zone_info.bass + 1, + lambda z: z.bass >= zone_info.bass + HtdConstants.MCA_BASS_TREBLE_STEP, zone, HtdMcaCommands.COMMON_COMMAND_CODE, HtdMcaCommands.BASS_UP_COMMAND ) + async def async_set_bass(self, zone: int, bass: int): + # Clip to limits + if bass > HtdConstants.MCA_MAX_BASS: + bass = HtdConstants.MCA_MAX_BASS + elif bass < HtdConstants.MCA_MIN_BASS: + bass = HtdConstants.MCA_MIN_BASS + + # Round to nearest step + if bass % HtdConstants.MCA_BASS_TREBLE_STEP != 0 and bass != 0: + bass = HtdConstants.MCA_BASS_TREBLE_STEP * round(bass / HtdConstants.MCA_BASS_TREBLE_STEP) + + existing = False + + if self._target_bass[zone] is not None: + existing = True + + self._target_bass[zone] = bass + + if existing: + return + + zone_info = self._zone_data[zone] + + if not zone_info.power: + await self.async_power_on(zone) + + return await self._async_set_bass(zone) + + async def _async_set_bass(self, zone: int): + """ + Resume setting the bass of a zone. + + Args: + zone (int): the zone + """ + + zone_info = self._zone_data[zone] + + if not zone_info.power: + self._target_bass[zone] = None + return + + diff = self._target_bass[zone] - zone_info.bass + + if diff == 0: + return + + if diff < 0: + bass_command = HtdMcaCommands.BASS_DOWN_COMMAND + else: + bass_command = HtdMcaCommands.BASS_UP_COMMAND + + await self._async_send_and_validate( + lambda z: z.bass != zone_info.bass, + zone, + HtdMcaCommands.COMMON_COMMAND_CODE, + bass_command + ) + async def async_bass_down(self, zone: int): """ Decrease the bass of a zone. @@ -332,12 +427,15 @@ async def async_bass_down(self, zone: int): zone_info = self._zone_data[zone] - new_bass = zone_info.bass - 1 - if new_bass < HtdConstants.MIN_BASS: + if not zone_info.power: + await self.async_power_on(zone) + + new_bass = zone_info.bass - HtdConstants.MCA_BASS_TREBLE_STEP + if new_bass < HtdConstants.MCA_MIN_BASS: return await self._async_send_and_validate( - lambda z: z.bass <= zone_info.bass - 1, + lambda z: z.bass <= zone_info.bass - HtdConstants.MCA_BASS_TREBLE_STEP, zone, HtdMcaCommands.COMMON_COMMAND_CODE, HtdMcaCommands.BASS_DOWN_COMMAND @@ -353,12 +451,15 @@ async def async_treble_up(self, zone: int): zone_info = self._zone_data[zone] - new_treble = zone_info.treble + 1 - if new_treble > HtdConstants.MAX_TREBLE: + if not zone_info.power: + await self.async_power_on(zone) + + new_treble = zone_info.treble + HtdConstants.MCA_BASS_TREBLE_STEP + if new_treble > HtdConstants.MCA_MAX_TREBLE: return await self._async_send_and_validate( - lambda z: z.treble >= zone_info.treble + 1, + lambda z: z.treble >= zone_info.treble + HtdConstants.MCA_BASS_TREBLE_STEP, zone, HtdMcaCommands.COMMON_COMMAND_CODE, HtdMcaCommands.TREBLE_UP_COMMAND @@ -374,17 +475,79 @@ async def async_treble_down(self, zone: int): zone_info = self._zone_data[zone] - new_treble = zone_info.treble - 1 - if new_treble < HtdConstants.MIN_TREBLE: + if not zone_info.power: + await self.async_power_on(zone) + + new_treble = zone_info.treble - HtdConstants.MCA_BASS_TREBLE_STEP + if new_treble < HtdConstants.MCA_MIN_TREBLE: return await self._async_send_and_validate( - lambda z: z.treble <= zone_info.treble - 1, + lambda z: z.treble <= zone_info.treble - HtdConstants.MCA_BASS_TREBLE_STEP, zone, HtdMcaCommands.COMMON_COMMAND_CODE, HtdMcaCommands.TREBLE_DOWN_COMMAND ) + async def async_set_treble(self, zone: int, treble: int): + # Clip to limits + if treble > HtdConstants.MCA_MAX_TREBLE: + treble = HtdConstants.MCA_MAX_TREBLE + elif treble < HtdConstants.MCA_MIN_TREBLE: + treble = HtdConstants.MCA_MIN_TREBLE + + # Round to nearest step + if treble % HtdConstants.MCA_BASS_TREBLE_STEP != 0 and treble != 0: + treble = HtdConstants.MCA_BASS_TREBLE_STEP * round(treble / HtdConstants.MCA_BASS_TREBLE_STEP) + + existing = False + + if self._target_treble[zone] is not None: + existing = True + + self._target_treble[zone] = treble + + if existing: + return + + zone_info = self._zone_data[zone] + + if not zone_info.power: + await self.async_power_on(zone) + + return await self._async_set_treble(zone) + + async def _async_set_treble(self, zone: int): + """ + Resume setting the treble of a zone. + + Args: + zone (int): the zone + """ + + zone_info = self._zone_data[zone] + + if not zone_info.power: + self._target_treble[zone] = None + return + + diff = self._target_treble[zone] - zone_info.treble + + if diff == 0: + return + + if diff < 0: + treble_command = HtdMcaCommands.TREBLE_DOWN_COMMAND + else: + treble_command = HtdMcaCommands.TREBLE_UP_COMMAND + + await self._async_send_and_validate( + lambda z: z.treble != zone_info.treble, + zone, + HtdMcaCommands.COMMON_COMMAND_CODE, + treble_command + ) + async def async_balance_left(self, zone: int): """ Increase the balance toward the left for a zone. @@ -395,12 +558,15 @@ async def async_balance_left(self, zone: int): zone_info = self._zone_data[zone] - new_balance = zone_info.balance - 1 - if new_balance < HtdConstants.MIN_BALANCE: + if not zone_info.power: + await self.async_power_on(zone) + + new_balance = zone_info.balance - HtdConstants.MCA_BALANCE_STEP + if new_balance < HtdConstants.MCA_MIN_BALANCE: return await self._async_send_and_validate( - lambda z: z.balance <= zone_info.balance - 1, + lambda z: z.balance <= zone_info.balance - HtdConstants.MCA_BALANCE_STEP, zone, HtdMcaCommands.COMMON_COMMAND_CODE, HtdMcaCommands.BALANCE_LEFT_COMMAND @@ -416,17 +582,79 @@ async def async_balance_right(self, zone: int): zone_info = self._zone_data[zone] - new_balance = zone_info.balance + 1 - if new_balance > HtdConstants.MAX_BALANCE: + if not zone_info.power: + await self.async_power_on(zone) + + new_balance = zone_info.balance + HtdConstants.MCA_BALANCE_STEP + if new_balance > HtdConstants.MCA_MAX_BALANCE: return await self._async_send_and_validate( - lambda z: z.balance >= zone_info.balance + 1, + lambda z: z.balance >= zone_info.balance + HtdConstants.MCA_BALANCE_STEP, zone, HtdMcaCommands.COMMON_COMMAND_CODE, HtdMcaCommands.BALANCE_RIGHT_COMMAND ) + async def async_set_balance(self, zone: int, balance: int): + # Clip to limits + if balance > HtdConstants.MCA_MAX_BALANCE: + balance = HtdConstants.MCA_MAX_BALANCE + elif balance < HtdConstants.MCA_MIN_BALANCE: + balance = HtdConstants.MCA_MIN_BALANCE + + # Round to nearest step + if balance % HtdConstants.MCA_BALANCE_STEP != 0 and balance != 0: + balance = HtdConstants.MCA_BALANCE_STEP * round(balance / HtdConstants.MCA_BALANCE_STEP) + + existing = False + + if self._target_balance[zone] is not None: + existing = True + + self._target_balance[zone] = balance + + if existing: + return + + zone_info = self._zone_data[zone] + + if not zone_info.power: + await self.async_power_on(zone) + + return await self._async_set_balance(zone) + + async def _async_set_balance(self, zone: int): + """ + Resume setting the balance of a zone. + + Args: + zone (int): the zone + """ + + zone_info = self._zone_data[zone] + + if not zone_info.power: + self._target_balance[zone] = None + return + + diff = self._target_balance[zone] - zone_info.balance + + if diff == 0: + return + + if diff < 0: + balance_command = HtdMcaCommands.BALANCE_LEFT_COMMAND + else: + balance_command = HtdMcaCommands.BALANCE_RIGHT_COMMAND + + await self._async_send_and_validate( + lambda z: z.balance != zone_info.balance, + zone, + HtdMcaCommands.COMMON_COMMAND_CODE, + balance_command + ) + async def async_set_dnd(self, zone: int, dnd: bool): raise NotImplementedError("MCA does not support DND.") diff --git a/pyproject.toml b/pyproject.toml index f049566..c563dba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "htd-client" -version = "0.0.25" +version = "0.0.26" description = "A client supporting Home Theater Direct's gateway device." authors = [ "Adam Kirschner " diff --git a/tests/test_base_client_gap_coverage.py b/tests/test_base_client_gap_coverage.py new file mode 100644 index 0000000..7979ea3 --- /dev/null +++ b/tests/test_base_client_gap_coverage.py @@ -0,0 +1,58 @@ +import pytest +import asyncio +from unittest.mock import MagicMock, AsyncMock, patch +from htd_client.base_client import BaseClient +from htd_client.constants import HtdModelInfo + +class ConcreteClient(BaseClient): + pass + +@pytest.fixture +def base_client(): + mock_loop = MagicMock() + model_info = {"zones": 6, "sources": 6, "kind": "lync", "name": "Lync6"} + client = ConcreteClient(mock_loop, model_info) + return client + +@pytest.mark.asyncio +async def test_wait_until_ready_success(base_client): + base_client._ready = False + + # Simulate ready becoming true after delay + async def make_ready(): + await asyncio.sleep(0.05) + base_client._ready = True + + asyncio.create_task(make_ready()) + + await base_client.async_wait_until_ready() + assert base_client._ready is True + +@pytest.mark.asyncio +async def test_wait_until_ready_timeout(base_client): + base_client._ready = False + base_client._socket_timeout_sec = 0.1 + + with pytest.raises(Exception, match="Timed out waiting for device to be ready"): + await base_client.async_wait_until_ready() + +@pytest.mark.asyncio +async def test_connect_already_connected(base_client): + base_client._connected = True + result = await base_client.async_connect() + assert result is None + +@pytest.mark.asyncio +async def test_connect_no_address(base_client): + base_client._connected = False + base_client._serial_address = None + base_client._network_address = None + + with pytest.raises(ValueError, match="No address provided"): + await base_client.async_connect() + +def test_ready_property(base_client): + base_client._ready = True + assert base_client.ready is True + base_client._ready = False + assert base_client.ready is False diff --git a/tests/test_bass_treble.py b/tests/test_bass_treble.py new file mode 100644 index 0000000..b210078 --- /dev/null +++ b/tests/test_bass_treble.py @@ -0,0 +1,209 @@ +import pytest +from unittest.mock import MagicMock, AsyncMock +from htd_client.mca_client import HtdMcaClient +from htd_client.lync_client import HtdLyncClient +from htd_client.constants import HtdConstants, HtdDeviceKind, HtdMcaCommands, HtdLyncCommands, HtdLyncConstants +from htd_client.models import ZoneDetail +import asyncio + +# --- Fixtures --- + +@pytest.fixture +def mca_client(): + loop = MagicMock() + model_info = { + "zones": 6, + "sources": 6, + "friendly_name": "MCA66", + "name": "MCA66", + "kind": HtdDeviceKind.mca, + "identifier": b'Wangine_MCA66' + } + client = HtdMcaClient(loop, model_info) + client._connection = MagicMock() + client._socket_lock = asyncio.Lock() + client._zone_data = { + i: ZoneDetail(i, enabled=True, power=True, volume=30, bass=0, treble=0) + for i in range(1, 7) + } + return client + +@pytest.fixture +def lync_client(): + loop = MagicMock() + model_info = { + "zones": 6, + "sources": 6, + "friendly_name": "Lync6", + "name": "Lync6", + "kind": HtdDeviceKind.lync, + "identifier": b'Wangine_Lync6' + } + client = HtdLyncClient(loop, model_info) + client._connection = MagicMock() + client._socket_lock = asyncio.Lock() + client._zone_data = { + i: ZoneDetail(i, enabled=True, power=True, volume=30, bass=0, treble=0) + for i in range(1, 7) + } + return client + +# --- MCA Tests --- + +@pytest.mark.asyncio +async def test_mca_bass_up(mca_client): + mca_client._async_send_and_validate = AsyncMock() + mca_client._zone_data[1].bass = 0 + + await mca_client.async_bass_up(1) + + args = mca_client._async_send_and_validate.call_args[0] + # args: validate, zone, cmd, code + assert args[1] == 1 + assert args[3] == HtdMcaCommands.BASS_UP_COMMAND + +@pytest.mark.asyncio +async def test_mca_bass_down(mca_client): + mca_client._async_send_and_validate = AsyncMock() + mca_client._zone_data[1].bass = 0 + + await mca_client.async_bass_down(1) + + args = mca_client._async_send_and_validate.call_args[0] + assert args[1] == 1 + assert args[3] == HtdMcaCommands.BASS_DOWN_COMMAND + +@pytest.mark.asyncio +async def test_mca_treble_up(mca_client): + mca_client._async_send_and_validate = AsyncMock() + mca_client._zone_data[1].treble = 0 + + await mca_client.async_treble_up(1) + + args = mca_client._async_send_and_validate.call_args[0] + assert args[1] == 1 + assert args[3] == HtdMcaCommands.TREBLE_UP_COMMAND + +@pytest.mark.asyncio +async def test_mca_treble_down(mca_client): + mca_client._async_send_and_validate = AsyncMock() + mca_client._zone_data[1].treble = 0 + + await mca_client.async_treble_down(1) + + args = mca_client._async_send_and_validate.call_args[0] + assert args[1] == 1 + assert args[3] == HtdMcaCommands.TREBLE_DOWN_COMMAND + +@pytest.mark.asyncio +async def test_mca_limits(mca_client): + mca_client._async_send_and_validate = AsyncMock() + + # Bass Max + mca_client._zone_data[1].bass = HtdConstants.MCA_MAX_BASS + await mca_client.async_bass_up(1) + mca_client._async_send_and_validate.assert_not_called() + + # Bass Min + mca_client._zone_data[1].bass = HtdConstants.MCA_MIN_BASS + await mca_client.async_bass_down(1) + mca_client._async_send_and_validate.assert_not_called() + + # Treble Max + mca_client._zone_data[1].treble = HtdConstants.MCA_MAX_TREBLE + await mca_client.async_treble_up(1) + mca_client._async_send_and_validate.assert_not_called() + + # Treble Min + mca_client._zone_data[1].treble = HtdConstants.MCA_MIN_TREBLE + await mca_client.async_treble_down(1) + mca_client._async_send_and_validate.assert_not_called() + +# --- Lync Tests --- + +@pytest.mark.asyncio +async def test_lync_set_bass(lync_client): + lync_client._send_cmd = AsyncMock() # Ensure this is mocked + lync_client._async_send_and_validate = AsyncMock() + + target_bass = 5 + await lync_client.async_set_bass(1, target_bass) + + args_send = lync_client._send_cmd.call_args[0] + # args: zone, command, data_code + assert args_send[0] == 1 + assert args_send[1] == HtdLyncCommands.BASS_SETTING_CONTROL_COMMAND_CODE + assert args_send[2] == target_bass & 0xFF + + args_validate = lync_client._async_send_and_validate.call_args[0] + # args: validate, zone, command, data_code + assert args_validate[1] == 1 + # Commit Command + assert args_validate[2] == HtdLyncCommands.COMMON_COMMAND_CODE + assert args_validate[3] == HtdLyncConstants.STATUS_REFRESH_CODE + +@pytest.mark.asyncio +async def test_lync_set_treble(lync_client): + lync_client._send_cmd = AsyncMock() # Mock _send_cmd as well + lync_client._async_send_and_validate = AsyncMock() + + target_treble = -5 + await lync_client.async_set_treble(1, target_treble) + + args_send = lync_client._send_cmd.call_args[0] + assert args_send[0] == 1 + assert args_send[1] == HtdLyncCommands.TREBLE_SETTING_CONTROL_COMMAND_CODE + assert args_send[2] == target_treble & 0xFF + + args_validate = lync_client._async_send_and_validate.call_args[0] + assert args_validate[1] == 1 + assert args_validate[2] == HtdLyncCommands.COMMON_COMMAND_CODE + assert args_validate[3] == HtdLyncConstants.STATUS_REFRESH_CODE + + +@pytest.mark.asyncio +async def test_lync_bass_up(lync_client): + lync_client.async_set_bass = AsyncMock() + lync_client._zone_data[1].bass = 0 + + await lync_client.async_bass_up(1) + lync_client.async_set_bass.assert_awaited_with(1, 1) + +@pytest.mark.asyncio +async def test_lync_bass_down(lync_client): + lync_client.async_set_bass = AsyncMock() + lync_client._zone_data[1].bass = 0 + + await lync_client.async_bass_down(1) + lync_client.async_set_bass.assert_awaited_with(1, -1) + +@pytest.mark.asyncio +async def test_lync_treble_up(lync_client): + lync_client.async_set_treble = AsyncMock() + lync_client._zone_data[1].treble = 0 + + await lync_client.async_treble_up(1) + lync_client.async_set_treble.assert_awaited_with(1, 1) + +@pytest.mark.asyncio +async def test_lync_treble_down(lync_client): + lync_client.async_set_treble = AsyncMock() + lync_client._zone_data[1].treble = 0 + + await lync_client.async_treble_down(1) + lync_client.async_set_treble.assert_awaited_with(1, -1) + +@pytest.mark.asyncio +async def test_lync_limits(lync_client): + lync_client.async_set_bass = AsyncMock() + lync_client.async_set_treble = AsyncMock() + + # Bass Max + lync_client._zone_data[1].bass = HtdConstants.LYNC_MAX_BASS + await lync_client.async_bass_up(1) + lync_client.async_set_bass.assert_not_called() + + # Bass Min + lync_client._zone_data[1].bass = HtdConstants.LYNC_MIN_BASS + await lync_client.async_bass_down(1) + lync_client.async_set_bass.assert_not_called() diff --git a/tests/test_lync_client_coverage.py b/tests/test_lync_client_coverage.py index c28be19..dea3a01 100644 --- a/tests/test_lync_client_coverage.py +++ b/tests/test_lync_client_coverage.py @@ -88,17 +88,17 @@ async def test_bass_treble_balance_limits(lync_client): lync_client.async_set_balance = AsyncMock() # Bass limit - lync_client._zone_data[1].bass = HtdConstants.MAX_BASS + lync_client._zone_data[1].bass = HtdConstants.LYNC_MAX_BASS await lync_client.async_bass_up(1) lync_client.async_set_bass.assert_not_called() # Treble limit - lync_client._zone_data[1].treble = HtdConstants.MAX_TREBLE + lync_client._zone_data[1].treble = HtdConstants.LYNC_MAX_TREBLE await lync_client.async_treble_up(1) lync_client.async_set_treble.assert_not_called() # Balance limit - lync_client._zone_data[1].balance = HtdConstants.MAX_BALANCE + lync_client._zone_data[1].balance = HtdConstants.LYNC_MAX_BALANCE await lync_client.async_balance_right(1) lync_client.async_set_balance.assert_not_called() @@ -180,31 +180,42 @@ async def test_audio_controls_success(lync_client): @pytest.mark.asyncio async def test_set_audio_values(lync_client): + lync_client._send_cmd = AsyncMock() lync_client._async_send_and_validate = AsyncMock() - lync_client._zone_data[1].bass = 0 # != 5 - lync_client._zone_data[1].treble = 0 - lync_client._zone_data[1].balance = 0 # Set bass await lync_client.async_set_bass(1, 5) - args = lync_client._async_send_and_validate.call_args[0] - # args: validate, zone, cmd, code, extra - assert args[2] == HtdLyncCommands.COMMON_COMMAND_CODE - assert args[3] == HtdLyncCommands.BASS_SETTING_CONTROL_COMMAND_CODE - assert args[4] == bytearray([5]) + + # Check 0x18 sent + args_send = lync_client._send_cmd.call_args[0] + assert args_send[1] == HtdLyncCommands.BASS_SETTING_CONTROL_COMMAND_CODE + assert args_send[2] == 5 + + # Check Commit + args_val = lync_client._async_send_and_validate.call_args[0] + assert args_val[2] == HtdLyncCommands.COMMON_COMMAND_CODE + assert args_val[3] == HtdLyncConstants.STATUS_REFRESH_CODE # Set treble await lync_client.async_set_treble(1, 5) - args = lync_client._async_send_and_validate.call_args[0] - assert args[3] == HtdLyncCommands.TREBLE_SETTING_CONTROL_COMMAND_CODE - assert args[4] == bytearray([5]) + args_send = lync_client._send_cmd.call_args[0] + assert args_send[1] == HtdLyncCommands.TREBLE_SETTING_CONTROL_COMMAND_CODE + assert args_send[2] == 5 + + args_val = lync_client._async_send_and_validate.call_args[0] + assert args_val[2] == HtdLyncCommands.COMMON_COMMAND_CODE + assert args_val[3] == HtdLyncConstants.STATUS_REFRESH_CODE # Set balance await lync_client.async_set_balance(1, 5) - # Lync balance: send_and_validate(..., zone, BALANCE_SETTING_CONTROL, balance) - args = lync_client._async_send_and_validate.call_args[0] - assert args[2] == HtdLyncCommands.BALANCE_SETTING_CONTROL_COMMAND_CODE - assert args[3] == 5 + + args_send = lync_client._send_cmd.call_args[0] + assert args_send[1] == HtdLyncCommands.BALANCE_SETTING_CONTROL_COMMAND_CODE + assert args_send[2] == 5 + + args_val = lync_client._async_send_and_validate.call_args[0] + assert args_val[2] == HtdLyncCommands.COMMON_COMMAND_CODE + assert args_val[3] == HtdLyncConstants.STATUS_REFRESH_CODE @pytest.mark.asyncio async def test_set_source_high(lync_client): diff --git a/tests/test_lync_options_coverage.py b/tests/test_lync_options_coverage.py new file mode 100644 index 0000000..966c782 --- /dev/null +++ b/tests/test_lync_options_coverage.py @@ -0,0 +1,135 @@ +import pytest +from unittest.mock import MagicMock, AsyncMock +from htd_client.lync_client import HtdLyncClient +from htd_client.constants import HtdConstants + +@pytest.fixture +def lync_client(): + mock_loop = MagicMock() + model_info = {"zones": 6, "sources": 6, "kind": "lync", "name": "Lync6"} + client = HtdLyncClient(mock_loop, model_info) + client._connection = MagicMock() + client._socket_lock = AsyncMock() + client._zone_data = {} + return client + +@pytest.mark.asyncio +async def test_lync_volume_down_boundary(lync_client): + # Test volume down when already 0 + lync_client._zone_data[1] = MagicMock(volume=0) + lync_client.async_set_volume = AsyncMock() + + await lync_client.async_volume_down(1) + + # Should not call set_volume + lync_client.async_set_volume.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_set_bass_no_change(lync_client): + # Test setting bass to same value + lync_client._zone_data[1] = MagicMock(bass=0) + lync_client._async_send_and_validate = AsyncMock() + + await lync_client.async_set_bass(1, 0) + + # Should not send command + lync_client._async_send_and_validate.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_set_treble_no_change(lync_client): + # Test setting treble to same value + lync_client._zone_data[1] = MagicMock(treble=0) + lync_client._async_send_and_validate = AsyncMock() + + await lync_client.async_set_treble(1, 0) + + # Should not send command + lync_client._async_send_and_validate.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_set_balance_no_change(lync_client): + # Test setting balance to same value + lync_client._zone_data[1] = MagicMock(balance=0) + lync_client._async_send_and_validate = AsyncMock() + + await lync_client.async_set_balance(1, 0) + + # Should not send command + lync_client._async_send_and_validate.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_bass_up_boundary(lync_client): + # Test bass up when already max + lync_client._zone_data[1] = MagicMock(bass=HtdConstants.LYNC_MAX_BASS) + lync_client.async_set_bass = AsyncMock() + + await lync_client.async_bass_up(1) + + # Should not call set_bass + lync_client.async_set_bass.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_bass_down_boundary(lync_client): + # Test bass down when already min + lync_client._zone_data[1] = MagicMock(bass=HtdConstants.LYNC_MIN_BASS) + lync_client.async_set_bass = AsyncMock() + + await lync_client.async_bass_down(1) + + # Should not call set_bass + lync_client.async_set_bass.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_treble_up_boundary(lync_client): + # Test treble up when already max + lync_client._zone_data[1] = MagicMock(treble=HtdConstants.LYNC_MAX_TREBLE) + lync_client.async_set_treble = AsyncMock() + + await lync_client.async_treble_up(1) + + # Should not call set_treble + lync_client.async_set_treble.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_treble_down_boundary(lync_client): + # Test treble down when already min + lync_client._zone_data[1] = MagicMock(treble=HtdConstants.LYNC_MIN_TREBLE) + lync_client.async_set_treble = AsyncMock() + + await lync_client.async_treble_down(1) + + # Should not call set_treble + lync_client.async_set_treble.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_balance_left_boundary(lync_client): + # Test balance left when already min + lync_client._zone_data[1] = MagicMock(balance=HtdConstants.LYNC_MIN_BALANCE) + lync_client.async_set_balance = AsyncMock() + + await lync_client.async_balance_left(1) + + # Should not call set_balance + lync_client.async_set_balance.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_balance_right_boundary(lync_client): + # Test balance right when already max + lync_client._zone_data[1] = MagicMock(balance=HtdConstants.LYNC_MAX_BALANCE) + lync_client.async_set_balance = AsyncMock() + + await lync_client.async_balance_right(1) + + # Should not call set_balance + lync_client.async_set_balance.assert_not_called() + +@pytest.mark.asyncio +async def test_lync_volume_down_success(lync_client): + # Test volume down when > 0 + lync_client._zone_data[1] = MagicMock(volume=10) + lync_client.async_set_volume = AsyncMock() + + await lync_client.async_volume_down(1) + + # Should call set_volume with 9 + lync_client.async_set_volume.assert_called_once_with(1, 9) diff --git a/tests/test_mca_bass_treble_set.py b/tests/test_mca_bass_treble_set.py new file mode 100644 index 0000000..7dbeb54 --- /dev/null +++ b/tests/test_mca_bass_treble_set.py @@ -0,0 +1,167 @@ +import pytest +from unittest.mock import MagicMock, AsyncMock, call +from htd_client.mca_client import HtdMcaClient +from htd_client.constants import HtdConstants + +@pytest.fixture +def mca_client(): + mock_loop = MagicMock() + model_info = {"zones": 6, "sources": 6, "kind": "mca", "name": "MCA66"} + client = HtdMcaClient(mock_loop, model_info) + client._connection = MagicMock() + client._socket_lock = AsyncMock() + client._zone_data = {} + + # Mock methods to avoid actual network calls + client._async_send_and_validate = AsyncMock() + client.async_power_on = AsyncMock() + + return client + +@pytest.mark.asyncio +async def test_set_bass_target(mca_client): + zone = 1 + target_bass = 4 # Use a valid step (4 on wire) + current_bass = 0 + + # Setup initial state + mca_client._zone_data[zone] = MagicMock(bass=current_bass, power=True) + + # Call set_bass + await mca_client.async_set_bass(zone, target_bass) + + # Check target is set + assert mca_client._target_bass[zone] == target_bass + assert mca_client.has_bass_target(zone) + + # Check _async_set_bass logic trigger (should send UP command) + # The first call interacts with _async_set_bass which calls _async_send_and_validate + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_set_treble_logic(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(treble=0, power=True) + mca_client._target_treble[zone] = 4 # Step of 4 + + # Test _async_set_treble directly + await mca_client._async_set_treble(zone) + + # Should send UP command + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_target_cleared_when_reached(mca_client): + zone = 1 + target_bass = 4 + mca_client._target_bass[zone] = target_bass + mca_client._zone_data[zone] = MagicMock(bass=target_bass, power=True) + + # Trigger update with matching value + # We need to simulate _on_zone_update logic without the threadsafe call for simplicity + if mca_client._zone_data[zone].bass == mca_client._target_bass[zone]: + mca_client._target_bass[zone] = None + + assert mca_client._target_bass[zone] is None + assert not mca_client.has_bass_target(zone) + +@pytest.mark.asyncio +async def test_auto_power_on_bass(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(bass=0, power=False) + + # calling bass up should trigger power on + await mca_client.async_bass_up(zone) + + mca_client.async_power_on.assert_called_with(zone) + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_auto_power_on_treble(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(treble=0, power=False) + + await mca_client.async_treble_up(zone) + + mca_client.async_power_on.assert_called_with(zone) + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_auto_power_on_balance(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(balance=0, power=False) + + await mca_client.async_balance_right(zone) + + mca_client.async_power_on.assert_called_with(zone) + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_set_balance_target(mca_client): + zone = 1 + target_balance = 6 + mca_client._zone_data[zone] = MagicMock(balance=0, power=True) + + await mca_client.async_set_balance(zone, target_balance) + + assert mca_client._target_balance[zone] == target_balance + assert mca_client.has_balance_target(zone) + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_set_balance_logic_resume(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(balance=0, power=True) + mca_client._target_balance[zone] = 6 + + await mca_client._async_set_balance(zone) + + # Should be RIGHT command since target > current (1 > 0) + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_set_balance_logic_left(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(balance=6, power=True) + mca_client._target_balance[zone] = 0 + + await mca_client._async_set_balance(zone) + + # Should be LEFT command + mca_client._async_send_and_validate.assert_called() + +@pytest.mark.asyncio +async def test_mca_rounding(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(bass=0, power=True) + + # Test round up: 3 -> 4 + await mca_client.async_set_bass(zone, 3) + assert mca_client._target_bass[zone] == 4 + + # Test round down: 1 -> 0 + await mca_client.async_set_bass(zone, 1) + assert mca_client._target_bass[zone] == 0 + + # Test exact: 4 -> 4 + await mca_client.async_set_bass(zone, 4) + assert mca_client._target_bass[zone] == 4 + +@pytest.mark.asyncio +async def test_mca_balance_rounding(mca_client): + zone = 1 + mca_client._zone_data[zone] = MagicMock(balance=0, power=True) + + # Test round up: 4 -> 6 (step is 6) + # 4 / 6 = 0.66 -> round to 1 -> 1 * 6 = 6 + await mca_client.async_set_balance(zone, 4) + assert mca_client._target_balance[zone] == 6 + + # Test round down: 2 -> 0 + # 2 / 6 = 0.33 -> round to 0 -> 0 * 6 = 0 + await mca_client.async_set_balance(zone, 2) + assert mca_client._target_balance[zone] == 0 + + # Test exact: 6 -> 6 + await mca_client.async_set_balance(zone, 6) + assert mca_client._target_balance[zone] == 6 diff --git a/tests/test_mca_client_coverage.py b/tests/test_mca_client_coverage.py index a6f0242..a3e4e01 100644 --- a/tests/test_mca_client_coverage.py +++ b/tests/test_mca_client_coverage.py @@ -149,25 +149,25 @@ async def test_audio_limits_mca(mca_client): mca_client._async_send_and_validate = AsyncMock() # Bass Down limit - mca_client._zone_data[1].bass = HtdConstants.MIN_BASS + mca_client._zone_data[1].bass = HtdConstants.MCA_MIN_BASS await mca_client.async_bass_down(1) mca_client._async_send_and_validate.assert_not_called() # Treble limits - mca_client._zone_data[1].treble = HtdConstants.MAX_TREBLE + mca_client._zone_data[1].treble = HtdConstants.MCA_MAX_TREBLE await mca_client.async_treble_up(1) mca_client._async_send_and_validate.assert_not_called() - mca_client._zone_data[1].treble = HtdConstants.MIN_TREBLE + mca_client._zone_data[1].treble = HtdConstants.MCA_MIN_TREBLE await mca_client.async_treble_down(1) mca_client._async_send_and_validate.assert_not_called() # Balance limits - mca_client._zone_data[1].balance = HtdConstants.MAX_BALANCE + mca_client._zone_data[1].balance = HtdConstants.MCA_MAX_BALANCE await mca_client.async_balance_right(1) mca_client._async_send_and_validate.assert_not_called() - mca_client._zone_data[1].balance = HtdConstants.MIN_BALANCE + mca_client._zone_data[1].balance = HtdConstants.MCA_MIN_BALANCE await mca_client.async_balance_left(1) mca_client._async_send_and_validate.assert_not_called() @@ -183,29 +183,21 @@ async def test_set_source(mca_client): @pytest.mark.asyncio async def test_volume_limits_mca(mca_client): - mca_client._async_send_and_validate = AsyncMock() - - # Max volume - mca_client._zone_data[1].volume = HtdConstants.MAX_VOLUME - await mca_client.async_volume_up(1) - mca_client._async_send_and_validate.assert_not_called() - - # Min volume? mca_client has no check for min volume in async_volume_down? - # Let's check code. Code: - # await self._async_send_and_validate(lambda z: z.volume >= zone_info.volume - 1, ...) - # It assumes hardware limit or validate failure? - # But async_volume_up HAS check: code line 226: if zone_info.volume == HtdConstants.MAX_VOLUME: return. + mca_client._async_send_and_validate = AsyncMock() - # async_volume_down has NO check in code provided in Step 492. + # Max volume + mca_client._zone_data[1].volume = HtdConstants.MAX_VOLUME + await mca_client.async_volume_up(1) + mca_client._async_send_and_validate.assert_not_called() - # Treble/Bass limits? - mca_client._zone_data[1].treble = HtdConstants.MAX_TREBLE - await mca_client.async_treble_up(1) - mca_client._async_send_and_validate.assert_not_called() + # Treble/Bass limits? + mca_client._zone_data[1].treble = HtdConstants.MCA_MAX_TREBLE + await mca_client.async_treble_up(1) + mca_client._async_send_and_validate.assert_not_called() - mca_client._zone_data[1].treble = HtdConstants.MIN_TREBLE - await mca_client.async_treble_down(1) - mca_client._async_send_and_validate.assert_not_called() + mca_client._zone_data[1].treble = HtdConstants.MCA_MIN_TREBLE + await mca_client.async_treble_down(1) + mca_client._async_send_and_validate.assert_not_called() def test_on_zone_update(mca_client): diff --git a/tests/test_mca_options_coverage.py b/tests/test_mca_options_coverage.py new file mode 100644 index 0000000..4016998 --- /dev/null +++ b/tests/test_mca_options_coverage.py @@ -0,0 +1,99 @@ +import pytest +from unittest.mock import MagicMock, AsyncMock +from htd_client.mca_client import HtdMcaClient +from htd_client.constants import HtdConstants, HtdMcaCommands + +@pytest.fixture +def mca_client(): + mock_loop = MagicMock() + model_info = {"zones": 6, "sources": 6, "kind": "mca", "name": "MCA66"} + client = HtdMcaClient(mock_loop, model_info) + client._connection = MagicMock() + client._socket_lock = AsyncMock() + # Initialize zone data + client._zone_data = {1: MagicMock(volume=30, mute=False, power=True)} + client._target_volumes = {key: None for key in range(1, 7)} + return client + +@pytest.mark.asyncio +async def test_mca_on_zone_update_none(mca_client): + # Test _on_zone_update with None or 0 + mca_client._on_zone_update(None) + mca_client._on_zone_update(0) + # Should not crash + +@pytest.mark.asyncio +async def test_mca_unmute_already_unmuted(mca_client): + # Test unmute when already unmuted + mca_client._zone_data[1].mute = False + mca_client._async_toggle_mute = AsyncMock() + + await mca_client.async_unmute(1) + + mca_client._async_toggle_mute.assert_not_called() + +@pytest.mark.asyncio +async def test_mca_has_volume_target(mca_client): + mca_client._target_volumes[1] = 50 + assert mca_client.has_volume_target(1) is True + + mca_client._target_volumes[1] = None + assert mca_client.has_volume_target(1) is False + +@pytest.mark.asyncio +async def test_mca_set_volume_existing_target(mca_client): + # Test setting volume when a target already exists + mca_client._target_volumes[1] = 40 + mca_client._async_set_volume = AsyncMock() + + await mca_client.async_set_volume(1, 50) + + # Target should be updated, but _async_set_volume should not be called again immediately? + # Actually logic says: if existing: return. So _async_set_volume NOT called. + assert mca_client._target_volumes[1] == 50 + mca_client._async_set_volume.assert_not_called() + +@pytest.mark.asyncio +async def test_mca_async_set_volume_power_off(mca_client): + # Test _async_set_volume when power is off + mca_client._zone_data[1].power = False + mca_client._target_volumes[1] = 50 + + await mca_client._async_set_volume(1) + + assert mca_client._target_volumes[1] is None + +@pytest.mark.asyncio +async def test_mca_async_set_volume_no_diff(mca_client): + # Test _async_set_volume when current volume equals target + mca_client._target_volumes[1] = 30 + mca_client._zone_data[1].volume = 30 + mca_client._async_send_and_validate = AsyncMock() + + await mca_client._async_set_volume(1) + + mca_client._async_send_and_validate.assert_not_called() + +@pytest.mark.asyncio +async def test_mca_async_set_volume_down(mca_client): + # Test _async_set_volume when target is lower (diff < 0) + mca_client._target_volumes[1] = 20 + mca_client._zone_data[1].volume = 30 + mca_client._async_send_and_validate = AsyncMock() + + await mca_client._async_set_volume(1) + + mca_client._async_send_and_validate.assert_called_once() + # Check command arg + args, _ = mca_client._async_send_and_validate.call_args + assert args[3] == HtdMcaCommands.VOLUME_DOWN_COMMAND + +@pytest.mark.asyncio +async def test_mca_volume_down_boundary(mca_client): + # Test volume down when at 0 + mca_client._zone_data[1].volume = 0 + mca_client._async_send_and_validate = AsyncMock() + + await mca_client.async_volume_down(1) + + mca_client._async_send_and_validate.assert_not_called() diff --git a/tests/test_utils_coverage_gap.py b/tests/test_utils_coverage_gap.py new file mode 100644 index 0000000..9e5552f --- /dev/null +++ b/tests/test_utils_coverage_gap.py @@ -0,0 +1,40 @@ +import pytest +import htd_client.utils +from htd_client.constants import HtdConstants +from unittest.mock import AsyncMock, patch + +def test_build_command_with_extra_data(): + # Test build_command with extra_data (line 34) + zone = 1 + command = 2 + data = 3 + extra = bytearray([4, 5]) + + cmd = htd_client.utils.build_command(zone, command, data, extra) + + # Header (2) + Zone (1) + Command (1) + Data (1) + Extra (2) + Checksum (1) = 8 bytes + assert len(cmd) == 8 + assert cmd[0] == HtdConstants.HEADER_BYTE + assert cmd[4] == data + assert cmd[5] == 4 + assert cmd[6] == 5 + +@pytest.mark.asyncio +async def test_async_send_command_no_header(): + # Test async_send_command when response has no header (line 104) + loop = AsyncMock() + # Mock open_connection + reader = AsyncMock() + writer = AsyncMock() + + # Return data without header + reader.read.return_value = b'\x00\x00\x00\x00' + + with patch('asyncio.open_connection', return_value=(reader, writer)): + response = await htd_client.utils.async_send_command( + loop, + b'cmd', + network_address=('localhost', 1234) + ) + + assert response == b'\x00\x00\x00\x00' From 47373f3ca2d4ef6c782c441f499d621d1cd090c3 Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sun, 24 May 2026 19:38:59 -0600 Subject: [PATCH 03/14] refactor: remove redundant hasattr guards; add abstract async_set_balance declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SET_ECHO_COMMAND_CODE and QUERY_ID_CODE are already defined in constants.py — drop the defensive hasattr ternaries in lync_client.py. Also add the missing @abstractmethod declaration for async_set_balance in base_client.py alongside async_set_bass and async_set_treble. --- htd_client/base_client.py | 4 ++++ htd_client/lync_client.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/htd_client/base_client.py b/htd_client/base_client.py index e779b73..8589066 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -611,6 +611,10 @@ async def async_balance_left(self, zone: int): async def async_balance_right(self, zone: int): pass + @abstractmethod + async def async_set_balance(self, zone: int, balance: int): + pass + @abstractmethod async def async_set_dnd(self, zone: int, dnd: bool): pass diff --git a/htd_client/lync_client.py b/htd_client/lync_client.py index 7af58ff..adb72dd 100644 --- a/htd_client/lync_client.py +++ b/htd_client/lync_client.py @@ -433,7 +433,7 @@ async def async_set_echo(self, echo: bool): """Set whether the unit should echo commands back.""" return await self._send_cmd( 0, - HtdLyncCommands.SET_ECHO_COMMAND_CODE if hasattr(HtdLyncCommands, "SET_ECHO_COMMAND_CODE") else 0x19, + HtdLyncCommands.SET_ECHO_COMMAND_CODE, 0xFF if echo else 0x00 ) @@ -441,7 +441,7 @@ async def async_query_id(self): """Query device ID.""" return await self._send_cmd( 0, - HtdLyncCommands.QUERY_ID_CODE if hasattr(HtdLyncCommands, "QUERY_ID_CODE") else 0x08, + HtdLyncCommands.QUERY_ID_CODE, 0x00 ) From 6b18e77ad14b51ac5b179ac5b65cefbcbd11a60e Mon Sep 17 00:00:00 2001 From: theharshl Date: Sun, 24 May 2026 22:50:43 -0600 Subject: [PATCH 04/14] chore: remove dangling .lync_clone submodule reference Co-Authored-By: Claude Sonnet 4.6 --- .lync_clone | 1 - 1 file changed, 1 deletion(-) delete mode 160000 .lync_clone diff --git a/.lync_clone b/.lync_clone deleted file mode 160000 index 527a4bf..0000000 --- a/.lync_clone +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 527a4bfc85386f27424ca4b2869d35aecd6a6cfe From f8422557ab3eebaecc86e32cdd0bf3f8f0b8fd51 Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Mon, 25 May 2026 06:44:21 -0600 Subject: [PATCH 05/14] feat: add get_zone_name() getter with independent _zone_names cache Populate _zone_names on every ZONE_NAME_RECEIVE_COMMAND regardless of whether zone_data exists yet. Expose get_zone_name(zone) -> str | None mirroring get_source_name(). Also initialize _zone_names in async_connect. Co-Authored-By: Claude Sonnet 4.6 --- htd_client/base_client.py | 6 ++++ tests/test_base_client_edge_cases.py | 1 + tests/test_zone_name_cache.py | 54 ++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 tests/test_zone_name_cache.py diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 8589066..0188bfe 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -96,6 +96,7 @@ async def async_connect(self): self._zone_data = {} self._zones_loaded = 0 self._source_names = {} + self._zone_names = {} self._connection = None self._disconnected = False @@ -334,6 +335,7 @@ def _parse_command(self, zone, cmd, data): elif cmd == HtdCommonCommands.ZONE_NAME_RECEIVE_COMMAND: name = str(data[0:11].decode(errors="ignore").rstrip('\0')).lower() + self._zone_names[zone] = name if self.has_zone_data(zone): self._zone_data[zone].name = name @@ -506,6 +508,10 @@ def get_source_name(self, source: int) -> str: """Get the name of a source if it has been fetched.""" return self._source_names.get(source, f"Source {source}") + def get_zone_name(self, zone: int) -> str | None: + """Get the cached zone name, or None if not yet queried.""" + return self._zone_names.get(zone) + def get_zone(self, zone: int): """ Query a zone and return `ZoneDetail` diff --git a/tests/test_base_client_edge_cases.py b/tests/test_base_client_edge_cases.py index c8a345d..aef97a4 100644 --- a/tests/test_base_client_edge_cases.py +++ b/tests/test_base_client_edge_cases.py @@ -43,6 +43,7 @@ def client(): c._connection = MagicMock() c._subscribers = set() c._zone_data = {} + c._zone_names = {} return c def test_process_next_command_invalid_header(client): diff --git a/tests/test_zone_name_cache.py b/tests/test_zone_name_cache.py new file mode 100644 index 0000000..e649ed2 --- /dev/null +++ b/tests/test_zone_name_cache.py @@ -0,0 +1,54 @@ +import asyncio +import pytest +from unittest.mock import MagicMock +from htd_client.lync_client import HtdLyncClient +from htd_client.constants import HtdDeviceKind, HtdCommonCommands +from htd_client.models import ZoneDetail + + +@pytest.fixture +def lync_client(): + loop = MagicMock() + model_info = { + "zones": 6, "sources": 12, "friendly_name": "Lync6", + "name": "Lync6", "kind": HtdDeviceKind.lync, "identifier": b'Wangine_Lync6' + } + client = HtdLyncClient(loop, model_info) + client._connection = MagicMock() + client._socket_lock = asyncio.Lock() + client._zone_names = {} + client._zone_data = {} + client._source_names = {} + return client + + +def test_get_zone_name_returns_none_before_query(lync_client): + assert lync_client.get_zone_name(1) is None + + +def test_get_zone_name_returns_none_for_unknown_zone(lync_client): + lync_client._zone_names[1] = "Living Room" + assert lync_client.get_zone_name(99) is None + + +def test_get_zone_name_returns_cached_name(lync_client): + lync_client._zone_names[1] = "Living Room" + assert lync_client.get_zone_name(1) == "Living Room" + + +def test_zone_name_cached_independently_of_zone_data(lync_client): + # Zone data does NOT exist for zone 2 yet + # Simulate _handle_message processing a ZONE_NAME_RECEIVE_COMMAND for zone 2 + lync_client._zone_names[2] = "office" + # zone_data should NOT be required for get_zone_name to work + assert 2 not in lync_client._zone_data + assert lync_client.get_zone_name(2) == "office" + + +def test_zone_name_also_updates_zone_data_when_present(lync_client): + # When zone_data already exists, _zone_data[zone].name should also be updated + lync_client._zone_data[1] = ZoneDetail(1) + lync_client._zone_names[1] = "living room" + lync_client._zone_data[1].name = "living room" + assert lync_client._zone_data[1].name == "living room" + assert lync_client.get_zone_name(1) == "living room" From 72cb2976b02369976a01f0bbfed00c7f145c860c Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Mon, 25 May 2026 06:47:02 -0600 Subject: [PATCH 06/14] refactor: remove unused import, simplify misleading test in zone name cache tests Co-Authored-By: Claude Sonnet 4.6 --- tests/test_zone_name_cache.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/test_zone_name_cache.py b/tests/test_zone_name_cache.py index e649ed2..4dd0a51 100644 --- a/tests/test_zone_name_cache.py +++ b/tests/test_zone_name_cache.py @@ -2,8 +2,7 @@ import pytest from unittest.mock import MagicMock from htd_client.lync_client import HtdLyncClient -from htd_client.constants import HtdDeviceKind, HtdCommonCommands -from htd_client.models import ZoneDetail +from htd_client.constants import HtdDeviceKind @pytest.fixture @@ -45,10 +44,6 @@ def test_zone_name_cached_independently_of_zone_data(lync_client): assert lync_client.get_zone_name(2) == "office" -def test_zone_name_also_updates_zone_data_when_present(lync_client): - # When zone_data already exists, _zone_data[zone].name should also be updated - lync_client._zone_data[1] = ZoneDetail(1) +def test_get_zone_name_reads_from_zone_names_dict(lync_client): lync_client._zone_names[1] = "living room" - lync_client._zone_data[1].name = "living room" - assert lync_client._zone_data[1].name == "living room" assert lync_client.get_zone_name(1) == "living room" From 496ab6dc0ff90dc558f320243b5cb302b734159e Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sat, 4 Jul 2026 15:55:53 -0600 Subject: [PATCH 07/14] fix: retry model probe and add settle delay for flaky serial adapters async_get_model_info() previously probed once with no retry, so a bad read (partial data, or a gateway that resets on DTR toggle when a USB serial adapter opens the port) left model_info as None, which crashed async_get_client() with a bare TypeError on model_info["kind"]. Wires the existing retry_attempts into the model probe, adds a settle delay after opening the serial port before the first write, and raises a clear ValueError instead of crashing when detection still fails. Fixes theharshl/htd-home-assistant#6 --- htd_client/__init__.py | 41 ++++++++++++++++++++-------- htd_client/constants.py | 5 ++++ htd_client/utils.py | 6 ++++- tests/test_init.py | 52 +++++++++++++++++++++++++++++++----- tests/test_utils_coverage.py | 38 +++++++++++++++++++++++++- 5 files changed, 122 insertions(+), 20 deletions(-) diff --git a/htd_client/__init__.py b/htd_client/__init__.py index d4680e9..938cb71 100644 --- a/htd_client/__init__.py +++ b/htd_client/__init__.py @@ -46,9 +46,17 @@ async def async_get_client( model_info = await async_get_model_info( loop if loop is not None else asyncio.get_running_loop(), network_address=network_address, - serial_address=serial_address + serial_address=serial_address, + retry_attempts=retry_attempts, ) + if model_info is None: + address = f"serial: {serial_address}" if serial_address is not None else f"network: {network_address}" + raise ValueError( + f"Unable to detect HTD device model ({address}). " + f"Verify the device is powered on and the path/address is correct." + ) + if model_info["kind"] == HtdDeviceKind.mca: client = HtdMcaClient( loop if loop is not None else asyncio.get_running_loop(), @@ -79,6 +87,7 @@ async def async_get_model_info( loop: asyncio.AbstractEventLoop = None, network_address: Tuple[str, int] = None, serial_address:str=None, + retry_attempts: int = HtdConstants.DEFAULT_RETRY_ATTEMPTS, ) -> HtdModelInfo | None: """ Get the model information from the gateway. @@ -92,16 +101,26 @@ async def async_get_model_info( 1, HtdCommonCommands.MODEL_QUERY_COMMAND_CODE, 0 ) - model_id = await htd_client.utils.async_send_command( - loop if loop is not None else asyncio.get_running_loop(), - cmd, - network_address=network_address, - serial_address=serial_address - ) + for attempt in range(retry_attempts): + model_id = await htd_client.utils.async_send_command( + loop if loop is not None else asyncio.get_running_loop(), + cmd, + network_address=network_address, + serial_address=serial_address, + settle_delay=HtdConstants.MODEL_PROBE_SETTLE_DELAY if serial_address is not None else 0, + ) - for model_name in HtdConstants.SUPPORTED_MODELS: - model = HtdConstants.SUPPORTED_MODELS[model_name] - if model["identifier"] in model_id: - return model + for model_name in HtdConstants.SUPPORTED_MODELS: + model = HtdConstants.SUPPORTED_MODELS[model_name] + if model["identifier"] in model_id: + return model + + if attempt < retry_attempts - 1: + _LOGGER.warning( + "Model probe attempt %d/%d failed to match a known device, retrying", + attempt + 1, + retry_attempts, + ) + await asyncio.sleep(HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT) return None diff --git a/htd_client/constants.py b/htd_client/constants.py index 97304d2..054c4ae 100644 --- a/htd_client/constants.py +++ b/htd_client/constants.py @@ -69,6 +69,11 @@ class HtdConstants: # the device is flakey, let's retry a bunch of times DEFAULT_RETRY_ATTEMPTS = 3 + # some USB-serial adapters (e.g. Prolific) toggle DTR on port open, which can + # cause the gateway to reset. give it a moment to settle before the first + # write, only paid once at startup during model detection. + MODEL_PROBE_SETTLE_DELAY = 1.5 + # the port of the device, default is 10006 DEFAULT_PORT = 10006 diff --git a/htd_client/utils.py b/htd_client/utils.py index 2786ac2..aa94d13 100644 --- a/htd_client/utils.py +++ b/htd_client/utils.py @@ -76,7 +76,8 @@ async def async_send_command( loop: asyncio.AbstractEventLoop, cmd: bytes, network_address: Tuple[str, int] = None, - serial_address: str = None + serial_address: str = None, + settle_delay: float = 0 ) -> bytes | None: if serial_address is not None: reader, writer = await open_serial_connection( @@ -86,6 +87,9 @@ async def async_send_command( timeout=HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT ) + if settle_delay: + await asyncio.sleep(settle_delay) + elif network_address is not None: host, port = network_address reader, writer = await asyncio.open_connection(host, port) diff --git a/tests/test_init.py b/tests/test_init.py index a603762..329758a 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -24,13 +24,41 @@ async def test_async_get_model_info_success(): @pytest.mark.asyncio async def test_async_get_model_info_failure(): mock_loop = MagicMock() - - with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send: + + with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send, \ + patch("asyncio.sleep", new_callable=AsyncMock): mock_send.return_value = b"Unknown Device" - - model = await async_get_model_info(loop=mock_loop, network_address=("1.2.3.4", 10006)) - + + model = await async_get_model_info(loop=mock_loop, network_address=("1.2.3.4", 10006), retry_attempts=2) + assert model is None + assert mock_send.call_count == 2 + +@pytest.mark.asyncio +async def test_async_get_model_info_retries_then_succeeds(): + mock_loop = MagicMock() + + with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send, \ + patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + mock_send.side_effect = [b"Unknown Device", b"Wangine_MCA66"] + + model = await async_get_model_info(loop=mock_loop, network_address=("1.2.3.4", 10006), retry_attempts=3) + + assert model == HtdConstants.SUPPORTED_MODELS["mca66"] + assert mock_send.call_count == 2 + mock_sleep.assert_called_once_with(HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT) + +@pytest.mark.asyncio +async def test_async_get_model_info_passes_settle_delay_for_serial(): + mock_loop = MagicMock() + + with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send: + mock_send.return_value = b"Wangine_MCA66" + + await async_get_model_info(loop=mock_loop, serial_address="/dev/ttyUSB0") + + _, kwargs = mock_send.call_args + assert kwargs["settle_delay"] == HtdConstants.MODEL_PROBE_SETTLE_DELAY @pytest.mark.asyncio async def test_async_get_client_mca(): @@ -63,9 +91,19 @@ async def test_async_get_client_lync(): @pytest.mark.asyncio async def test_async_get_client_unknown(): mock_loop = MagicMock() - + with patch("htd_client.async_get_model_info", new_callable=AsyncMock) as mock_get_info: mock_get_info.return_value = {"kind": "unknown_kind"} - + with pytest.raises(ValueError, match="Unknown Device Kind"): await async_get_client(loop=mock_loop, network_address=("1.2.3.4", 10006)) + +@pytest.mark.asyncio +async def test_async_get_client_raises_clear_error_when_model_undetected(): + mock_loop = MagicMock() + + with patch("htd_client.async_get_model_info", new_callable=AsyncMock) as mock_get_info: + mock_get_info.return_value = None + + with pytest.raises(ValueError, match="Unable to detect HTD device model"): + await async_get_client(loop=mock_loop, serial_address="/dev/serial/by-id/usb-example") diff --git a/tests/test_utils_coverage.py b/tests/test_utils_coverage.py index c01d530..bec2284 100644 --- a/tests/test_utils_coverage.py +++ b/tests/test_utils_coverage.py @@ -98,6 +98,42 @@ async def test_async_send_command_serial(): @pytest.mark.asyncio async def test_async_send_command_no_address(): mock_loop = MagicMock() - + with pytest.raises(ValueError, match="unable to connect"): await async_send_command(mock_loop, b"cmd") + +@pytest.mark.asyncio +async def test_async_send_command_serial_settle_delay(): + mock_loop = MagicMock() + mock_reader = AsyncMock() + mock_writer = MagicMock() + mock_writer.drain = AsyncMock() + mock_writer.wait_closed = AsyncMock() + + mock_reader.read.return_value = b"response" + HtdConstants.MESSAGE_HEADER + + with patch("htd_client.utils.open_serial_connection", new_callable=AsyncMock) as mock_open, \ + patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + mock_open.return_value = (mock_reader, mock_writer) + + await async_send_command(mock_loop, b"cmd", serial_address="/dev/ttyUSB0", settle_delay=1.5) + + mock_sleep.assert_called_once_with(1.5) + +@pytest.mark.asyncio +async def test_async_send_command_no_settle_delay_by_default(): + mock_loop = MagicMock() + mock_reader = AsyncMock() + mock_writer = MagicMock() + mock_writer.drain = AsyncMock() + mock_writer.wait_closed = AsyncMock() + + mock_reader.read.return_value = b"response" + HtdConstants.MESSAGE_HEADER + + with patch("htd_client.utils.open_serial_connection", new_callable=AsyncMock) as mock_open, \ + patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + mock_open.return_value = (mock_reader, mock_writer) + + await async_send_command(mock_loop, b"cmd", serial_address="/dev/ttyUSB0") + + mock_sleep.assert_not_called() From 51a203247b976fe8f797d812a81deb8e2b3fcb5e Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Fri, 10 Jul 2026 18:26:02 -0600 Subject: [PATCH 08/14] fix: tolerate slow USB-serial adapters in model probe and connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model probe read the reply with a single StreamReader.read(), which returns as soon as any bytes are buffered — a cheap USB-serial adapter delivering a reply across several USB packets was misread as a failed probe. Reads now accumulate until the reply matches a known model, the line goes quiet, or an overall deadline expires; a silent device no longer hangs the probe forever. Probe retries also re-opened the serial port on every attempt, and each open can toggle DTR and reset the gateway — so retries kept resetting the device they were probing. The probe now opens the connection once, pays the settle delay once, and retries on the open connection. The persistent connection now waits out the same settle delay before its first refresh write, which was previously sent immediately after the port opened and could be lost to the same reset. Replaces utils.async_send_command with async_open_connection + async_read_response; renames MODEL_PROBE_SETTLE_DELAY to SERIAL_SETTLE_DELAY now that it also guards the persistent connection. Reported by @steve28 in theharshl/htd-home-assistant#19 (MCA-66 over USB-serial). Co-Authored-By: Claude Fable 5 --- htd_client/__init__.py | 54 +++++-- htd_client/base_client.py | 5 + htd_client/constants.py | 15 +- htd_client/utils.py | 77 ++++++++-- tests/test_init.py | 60 +++++--- tests/test_serial_robustness.py | 244 +++++++++++++++++++++++++++++++ tests/test_utils_coverage.py | 95 ++++++------ tests/test_utils_coverage_gap.py | 26 ++-- 8 files changed, 457 insertions(+), 119 deletions(-) create mode 100644 tests/test_serial_robustness.py diff --git a/htd_client/__init__.py b/htd_client/__init__.py index 938cb71..87c0712 100644 --- a/htd_client/__init__.py +++ b/htd_client/__init__.py @@ -101,26 +101,48 @@ async def async_get_model_info( 1, HtdCommonCommands.MODEL_QUERY_COMMAND_CODE, 0 ) - for attempt in range(retry_attempts): - model_id = await htd_client.utils.async_send_command( - loop if loop is not None else asyncio.get_running_loop(), - cmd, - network_address=network_address, - serial_address=serial_address, - settle_delay=HtdConstants.MODEL_PROBE_SETTLE_DELAY if serial_address is not None else 0, - ) - + def find_model(data: bytes) -> HtdModelInfo | None: for model_name in HtdConstants.SUPPORTED_MODELS: model = HtdConstants.SUPPORTED_MODELS[model_name] - if model["identifier"] in model_id: + if model["identifier"] in data: return model + return None + + # open the connection once and retry on it: every serial port open can + # toggle DTR and reset the gateway, so re-opening per attempt would keep + # resetting the device we are trying to probe + reader, writer = await htd_client.utils.async_open_connection( + loop if loop is not None else asyncio.get_running_loop(), + network_address=network_address, + serial_address=serial_address, + settle_delay=HtdConstants.SERIAL_SETTLE_DELAY if serial_address is not None else 0, + ) + + try: + for attempt in range(retry_attempts): + writer.write(cmd) + await writer.drain() - if attempt < retry_attempts - 1: - _LOGGER.warning( - "Model probe attempt %d/%d failed to match a known device, retrying", - attempt + 1, - retry_attempts, + data = await htd_client.utils.async_read_response( + reader, + response_complete=lambda d: find_model(d) is not None, + timeout=HtdConstants.RESPONSE_TIMEOUT, + quiet_window=HtdConstants.RESPONSE_QUIET_WINDOW, ) - await asyncio.sleep(HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT) + + model = find_model(data) + if model is not None: + return model + + if attempt < retry_attempts - 1: + _LOGGER.warning( + "Model probe attempt %d/%d failed to match a known device, retrying", + attempt + 1, + retry_attempts, + ) + await asyncio.sleep(HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT) + finally: + writer.close() + await writer.wait_closed() return None diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 0188bfe..2b4a513 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -128,6 +128,11 @@ def connection_made(self, transport: Transport): async def _heartbeat(self): + # opening a serial port can toggle DTR and reset the gateway; wait for + # it to come back or the first refresh command is lost + if self._serial_address is not None: + await asyncio.sleep(HtdConstants.SERIAL_SETTLE_DELAY) + while self._connected: await self.refresh() await asyncio.sleep(60) diff --git a/htd_client/constants.py b/htd_client/constants.py index 054c4ae..e265515 100644 --- a/htd_client/constants.py +++ b/htd_client/constants.py @@ -70,9 +70,18 @@ class HtdConstants: DEFAULT_RETRY_ATTEMPTS = 3 # some USB-serial adapters (e.g. Prolific) toggle DTR on port open, which can - # cause the gateway to reset. give it a moment to settle before the first - # write, only paid once at startup during model detection. - MODEL_PROBE_SETTLE_DELAY = 1.5 + # cause the gateway to reset. give it a moment to settle after every serial + # port open before the first write (model probe and persistent connection). + SERIAL_SETTLE_DELAY = 1.5 + + # a slow USB-serial adapter can split a reply across many small reads and + # a resetting gateway can delay it; keep reading until the response matches, + # the line goes quiet, or this overall deadline expires + RESPONSE_TIMEOUT = 3.0 + + # once some data has arrived, how long the line must stay quiet before we + # consider the response complete + RESPONSE_QUIET_WINDOW = 0.3 # the port of the device, default is 10006 DEFAULT_PORT = 10006 diff --git a/htd_client/utils.py b/htd_client/utils.py index aa94d13..d9ebac0 100644 --- a/htd_client/utils.py +++ b/htd_client/utils.py @@ -1,5 +1,6 @@ import asyncio import logging +import time from typing import Literal, Tuple from serial_asyncio import open_serial_connection @@ -72,21 +73,21 @@ def stringify_bytes(data: bytes) -> str: return ret_val -async def async_send_command( +async def async_open_connection( loop: asyncio.AbstractEventLoop, - cmd: bytes, network_address: Tuple[str, int] = None, serial_address: str = None, settle_delay: float = 0 -) -> bytes | None: +) -> Tuple[asyncio.StreamReader, asyncio.StreamWriter]: if serial_address is not None: reader, writer = await open_serial_connection( loop=loop, url=serial_address, baudrate=38400, - timeout=HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT ) + # opening the port can toggle DTR and reset the gateway; wait for it + # to come back before the first write if settle_delay: await asyncio.sleep(settle_delay) @@ -97,17 +98,67 @@ async def async_send_command( else: raise ValueError("unable to connect, no address") - writer.write(cmd) - await writer.drain() - data = await reader.read(MAX_BYTES_TO_RECEIVE) - writer.close() - await writer.wait_closed() + return reader, writer - header_index = data.find(HtdConstants.MESSAGE_HEADER) - if header_index == -1: - return data - return data[0:header_index] +async def async_read_response( + reader: asyncio.StreamReader, + response_complete: callable = None, + timeout: float = None, + quiet_window: float = None, +) -> bytes: + """ + Read a response, tolerating slow delivery. A cheap USB-serial adapter can + split a reply across many small reads, so a single read() may return only + a fragment. Accumulate reads until `response_complete` matches the buffer, + the line goes quiet, or the overall deadline expires. + + Args: + reader (asyncio.StreamReader): the stream to read from + response_complete (callable): optional predicate called with the + accumulated bytes; return True to stop reading early + timeout (float): overall deadline in seconds for the full response + quiet_window (float): once data has arrived, how long the line must + stay quiet before the response is considered complete + + Returns: + bytes: whatever data accumulated before the deadline, possibly empty + """ + if timeout is None: + timeout = HtdConstants.RESPONSE_TIMEOUT + + if quiet_window is None: + quiet_window = HtdConstants.RESPONSE_QUIET_WINDOW + + data = bytearray() + deadline = time.monotonic() + timeout + + while True: + if response_complete is not None and response_complete(bytes(data)): + break + + remaining = deadline - time.monotonic() + if remaining <= 0: + break + + wait = min(quiet_window, remaining) if data else remaining + + try: + chunk = await asyncio.wait_for( + reader.read(MAX_BYTES_TO_RECEIVE), wait + ) + except asyncio.TimeoutError: + if data: + # the line went quiet, the response is as complete as it gets + break + continue + + if not chunk: + break + + data += chunk + + return bytes(data) def convert_value(value: int): diff --git a/tests/test_init.py b/tests/test_init.py index 329758a..3d2f450 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -3,62 +3,78 @@ from htd_client import async_get_client, async_get_model_info, HtdMcaClient, HtdLyncClient from htd_client.constants import HtdConstants, HtdDeviceKind +def _mock_connection(): + mock_reader = AsyncMock() + mock_writer = MagicMock() + mock_writer.drain = AsyncMock() + mock_writer.wait_closed = AsyncMock() + return mock_reader, mock_writer + @pytest.mark.asyncio async def test_async_get_model_info_success(): mock_loop = MagicMock() - mock_response = b"MCA-66" - - # MCA-66 identifier is "MCA-66" in constants? Let's check constants.py content implicitly or mock it. - # Actually, let's look at what async_get_model_info does. - # It sends MODEL_QUERY_COMMAND_CODE. - # It iterates HtdConstants.SUPPORTED_MODELS and checks if model["identifier"] is in response. - - with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send: - mock_send.return_value = b"Wangine_MCA66" - + mock_reader, mock_writer = _mock_connection() + + with patch("htd_client.utils.async_open_connection", new_callable=AsyncMock) as mock_open, \ + patch("htd_client.utils.async_read_response", new_callable=AsyncMock) as mock_read: + mock_open.return_value = (mock_reader, mock_writer) + mock_read.return_value = b"Wangine_MCA66" + model = await async_get_model_info(loop=mock_loop, network_address=("1.2.3.4", 10006)) - + assert model == HtdConstants.SUPPORTED_MODELS["mca66"] - mock_send.assert_called_once() + mock_read.assert_called_once() + mock_writer.close.assert_called_once() @pytest.mark.asyncio async def test_async_get_model_info_failure(): mock_loop = MagicMock() + mock_reader, mock_writer = _mock_connection() - with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send, \ + with patch("htd_client.utils.async_open_connection", new_callable=AsyncMock) as mock_open, \ + patch("htd_client.utils.async_read_response", new_callable=AsyncMock) as mock_read, \ patch("asyncio.sleep", new_callable=AsyncMock): - mock_send.return_value = b"Unknown Device" + mock_open.return_value = (mock_reader, mock_writer) + mock_read.return_value = b"Unknown Device" model = await async_get_model_info(loop=mock_loop, network_address=("1.2.3.4", 10006), retry_attempts=2) assert model is None - assert mock_send.call_count == 2 + assert mock_read.call_count == 2 + assert mock_open.call_count == 1 + mock_writer.close.assert_called_once() @pytest.mark.asyncio async def test_async_get_model_info_retries_then_succeeds(): mock_loop = MagicMock() + mock_reader, mock_writer = _mock_connection() - with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send, \ + with patch("htd_client.utils.async_open_connection", new_callable=AsyncMock) as mock_open, \ + patch("htd_client.utils.async_read_response", new_callable=AsyncMock) as mock_read, \ patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - mock_send.side_effect = [b"Unknown Device", b"Wangine_MCA66"] + mock_open.return_value = (mock_reader, mock_writer) + mock_read.side_effect = [b"Unknown Device", b"Wangine_MCA66"] model = await async_get_model_info(loop=mock_loop, network_address=("1.2.3.4", 10006), retry_attempts=3) assert model == HtdConstants.SUPPORTED_MODELS["mca66"] - assert mock_send.call_count == 2 + assert mock_read.call_count == 2 mock_sleep.assert_called_once_with(HtdConstants.DEFAULT_COMMAND_RETRY_TIMEOUT) @pytest.mark.asyncio async def test_async_get_model_info_passes_settle_delay_for_serial(): mock_loop = MagicMock() + mock_reader, mock_writer = _mock_connection() - with patch("htd_client.utils.async_send_command", new_callable=AsyncMock) as mock_send: - mock_send.return_value = b"Wangine_MCA66" + with patch("htd_client.utils.async_open_connection", new_callable=AsyncMock) as mock_open, \ + patch("htd_client.utils.async_read_response", new_callable=AsyncMock) as mock_read: + mock_open.return_value = (mock_reader, mock_writer) + mock_read.return_value = b"Wangine_MCA66" await async_get_model_info(loop=mock_loop, serial_address="/dev/ttyUSB0") - _, kwargs = mock_send.call_args - assert kwargs["settle_delay"] == HtdConstants.MODEL_PROBE_SETTLE_DELAY + _, kwargs = mock_open.call_args + assert kwargs["settle_delay"] == HtdConstants.SERIAL_SETTLE_DELAY @pytest.mark.asyncio async def test_async_get_client_mca(): diff --git a/tests/test_serial_robustness.py b/tests/test_serial_robustness.py new file mode 100644 index 0000000..49f5a3a --- /dev/null +++ b/tests/test_serial_robustness.py @@ -0,0 +1,244 @@ +"""Tests for robustness against slow / low-quality USB-serial adapters. + +A cheap adapter can: +- deliver a reply split across multiple small reads (partial read) +- delay the reply while the gateway reboots after a DTR toggle on port open +- never deliver a reply at all (command lost during gateway reset) +- deliver line noise before the real reply + +The model probe must tolerate all of these, must not re-open the port +between retries (each open DTR-resets the gateway), and the persistent +connection must let the gateway settle before the first write. +""" +import asyncio +import pytest +from unittest.mock import MagicMock, AsyncMock, patch + +from htd_client import async_get_model_info +from htd_client.constants import HtdConstants + +from tests.test_base_client_additional import ConcreteClient + +MCA66_REPLY = b"Wangine_MCA66" + + +def make_writer(): + writer = MagicMock() + writer.drain = AsyncMock() + writer.wait_closed = AsyncMock() + return writer + + +def fast_timing(): + """Shrink real-time waits so tests stay fast.""" + return ( + patch.object(HtdConstants, "SERIAL_SETTLE_DELAY", 0), + patch.object(HtdConstants, "RESPONSE_TIMEOUT", 0.2), + patch.object(HtdConstants, "RESPONSE_QUIET_WINDOW", 0.05), + patch.object(HtdConstants, "DEFAULT_COMMAND_RETRY_TIMEOUT", 0.01), + ) + + +def apply_patches(patches): + for p in patches: + p.start() + + +def stop_patches(patches): + for p in patches: + p.stop() + + +@pytest.fixture +def timing(): + patches = fast_timing() + apply_patches(patches) + yield + stop_patches(patches) + + +@pytest.mark.asyncio +async def test_model_detected_when_reply_arrives_in_chunks(timing): + """A reply split across USB packets must still match the identifier.""" + mock_reader = AsyncMock() + mock_reader.read.side_effect = [b"Wangine_", b"MCA66", b""] + mock_writer = make_writer() + + with patch( + "htd_client.utils.open_serial_connection", new_callable=AsyncMock + ) as mock_open: + mock_open.return_value = (mock_reader, mock_writer) + + model = await async_get_model_info(serial_address="/dev/ttyUSB0") + + assert model == HtdConstants.SUPPORTED_MODELS["mca66"] + assert mock_open.call_count == 1 + + +@pytest.mark.asyncio +async def test_probe_returns_none_instead_of_hanging_when_no_reply(timing): + """If the gateway never answers (command lost during its DTR reset), + the probe must give up within its deadline, not await forever.""" + + async def hang(*args, **kwargs): + await asyncio.get_running_loop().create_future() + + mock_reader = MagicMock() + mock_reader.read = hang + mock_writer = make_writer() + + with patch( + "htd_client.utils.open_serial_connection", new_callable=AsyncMock + ) as mock_open: + mock_open.return_value = (mock_reader, mock_writer) + + model = await asyncio.wait_for( + async_get_model_info(serial_address="/dev/ttyUSB0", retry_attempts=2), + timeout=5, + ) + + assert model is None + + +@pytest.mark.asyncio +async def test_probe_opens_serial_port_once_across_retries(timing): + """Each port open DTR-resets the gateway, so probe retries must reuse + the already-open port instead of re-opening it per attempt.""" + mock_reader = AsyncMock() + # each attempt reads garbage then EOF; three attempts' worth + mock_reader.read.side_effect = [b"garbage", b"", b"junk", b"", b"noise", b""] + mock_writer = make_writer() + + with patch( + "htd_client.utils.open_serial_connection", new_callable=AsyncMock + ) as mock_open: + mock_open.return_value = (mock_reader, mock_writer) + + model = await async_get_model_info( + serial_address="/dev/ttyUSB0", retry_attempts=3 + ) + + assert model is None + assert mock_open.call_count == 1 + # the command was actually retried on the one open connection + assert mock_writer.write.call_count == 3 + + +@pytest.mark.asyncio +async def test_model_detected_with_noise_before_reply(timing): + """Line noise from the gateway reset (including bytes that look like a + message header) must not prevent the identifier match.""" + noisy = b"\x00\xff" + HtdConstants.MESSAGE_HEADER + b"\xfa" + MCA66_REPLY + mock_reader = AsyncMock() + mock_reader.read.side_effect = [noisy, b""] + mock_writer = make_writer() + + with patch( + "htd_client.utils.open_serial_connection", new_callable=AsyncMock + ) as mock_open: + mock_open.return_value = (mock_reader, mock_writer) + + model = await async_get_model_info(serial_address="/dev/ttyUSB0") + + assert model == HtdConstants.SUPPORTED_MODELS["mca66"] + + +@pytest.mark.asyncio +async def test_probe_stops_reading_once_model_identified(timing): + """Once the identifier is in the buffer the probe should return without + waiting out the quiet window or issuing further reads.""" + mock_reader = AsyncMock() + mock_reader.read.side_effect = [ + MCA66_REPLY, + RuntimeError("probe kept reading after the reply already matched"), + ] + mock_writer = make_writer() + + with patch( + "htd_client.utils.open_serial_connection", new_callable=AsyncMock + ) as mock_open: + mock_open.return_value = (mock_reader, mock_writer) + + model = await async_get_model_info(serial_address="/dev/ttyUSB0") + + assert model == HtdConstants.SUPPORTED_MODELS["mca66"] + + +@pytest.mark.asyncio +async def test_serial_settle_delay_applied_once_before_first_write(timing): + """The settle delay is paid once, after the single port open, before the + first probe write — not once per retry.""" + stop_patches_needed = patch.object(HtdConstants, "SERIAL_SETTLE_DELAY", 1.5) + stop_patches_needed.start() + try: + mock_reader = AsyncMock() + mock_reader.read.side_effect = [b"garbage", b"", b"junk", b"", b"noise", b""] + mock_writer = make_writer() + + with patch( + "htd_client.utils.open_serial_connection", new_callable=AsyncMock + ) as mock_open, patch( + "asyncio.sleep", new_callable=AsyncMock + ) as mock_sleep: + mock_open.return_value = (mock_reader, mock_writer) + + await async_get_model_info(serial_address="/dev/ttyUSB0", retry_attempts=3) + + settle_calls = [ + c for c in mock_sleep.call_args_list if c.args and c.args[0] == 1.5 + ] + assert len(settle_calls) == 1 + finally: + stop_patches_needed.stop() + + +@pytest.mark.asyncio +async def test_heartbeat_settles_before_first_refresh_on_serial(): + """The persistent connection's port open also DTR-resets the gateway; + the first refresh write must wait for the settle delay or it is lost.""" + loop = MagicMock() + model_info = HtdConstants.SUPPORTED_MODELS["mca66"] + client = ConcreteClient(loop, model_info, serial_address="/dev/ttyUSB0") + client._connected = True + + events = [] + + async def fake_refresh(zone=None): + events.append("refresh") + client._connected = False + + async def fake_sleep(delay): + events.append(("sleep", delay)) + + client.refresh = fake_refresh + + with patch("asyncio.sleep", new_callable=AsyncMock, side_effect=fake_sleep): + await client._heartbeat() + + assert events[0] == ("sleep", HtdConstants.SERIAL_SETTLE_DELAY) + assert "refresh" in events + + +@pytest.mark.asyncio +async def test_heartbeat_does_not_delay_first_refresh_on_network(): + """Network connections have no DTR reset; refresh should fire immediately.""" + loop = MagicMock() + model_info = HtdConstants.SUPPORTED_MODELS["mca66"] + client = ConcreteClient(loop, model_info, network_address=("1.2.3.4", 10006)) + client._connected = True + + events = [] + + async def fake_refresh(zone=None): + events.append("refresh") + client._connected = False + + async def fake_sleep(delay): + events.append(("sleep", delay)) + + client.refresh = fake_refresh + + with patch("asyncio.sleep", new_callable=AsyncMock, side_effect=fake_sleep): + await client._heartbeat() + + assert events[0] == "refresh" diff --git a/tests/test_utils_coverage.py b/tests/test_utils_coverage.py index bec2284..58a9c24 100644 --- a/tests/test_utils_coverage.py +++ b/tests/test_utils_coverage.py @@ -2,13 +2,14 @@ from unittest.mock import MagicMock, AsyncMock, patch from htd_client.constants import HtdConstants, HtdDeviceKind from htd_client.utils import ( - convert_value, - stringify_bytes_raw, - stringify_bytes, - convert_volume_to_raw, - decode_response, - parse_zone_name, - async_send_command + convert_value, + stringify_bytes_raw, + stringify_bytes, + convert_volume_to_raw, + decode_response, + parse_zone_name, + async_open_connection, + async_read_response ) def test_convert_value(): @@ -53,87 +54,85 @@ def test_parse_zone_name(): assert parse_zone_name(data) == "Zone1" @pytest.mark.asyncio -async def test_async_send_command_network(): +async def test_async_open_connection_network(): mock_loop = MagicMock() mock_reader = AsyncMock() mock_writer = MagicMock() - mock_writer.drain = AsyncMock() - mock_writer.wait_closed = AsyncMock() - - # Setup mock reader to return data with header. Header is 0x02 0x00. - # We should return header first then data? Or is it finding header in response? - # data.find(HEADER). If found, returns data[0:header_index]. - # So if we return b"RESPONSE" + HEADER, it returns "RESPONSE". - mock_reader.read.return_value = b"response" + HtdConstants.MESSAGE_HEADER - + with patch("asyncio.open_connection", new_callable=AsyncMock) as mock_open: mock_open.return_value = (mock_reader, mock_writer) - - response = await async_send_command(mock_loop, b"cmd", network_address=("1.2.3.4", 1234)) - + + reader, writer = await async_open_connection(mock_loop, network_address=("1.2.3.4", 1234)) + mock_open.assert_called_with("1.2.3.4", 1234) - mock_writer.write.assert_called_with(b"cmd") - mock_writer.drain.assert_called() - mock_writer.close.assert_called() - assert response == b"response" + assert reader is mock_reader + assert writer is mock_writer @pytest.mark.asyncio -async def test_async_send_command_serial(): +async def test_async_open_connection_serial(): mock_loop = MagicMock() mock_reader = AsyncMock() mock_writer = MagicMock() - mock_writer.drain = AsyncMock() - mock_writer.wait_closed = AsyncMock() - - mock_reader.read.return_value = b"response" + HtdConstants.MESSAGE_HEADER - + with patch("htd_client.utils.open_serial_connection", new_callable=AsyncMock) as mock_open: mock_open.return_value = (mock_reader, mock_writer) - - response = await async_send_command(mock_loop, b"cmd", serial_address="/dev/ttyUSB0") - + + reader, writer = await async_open_connection(mock_loop, serial_address="/dev/ttyUSB0") + mock_open.assert_called() - assert response == b"response" + assert reader is mock_reader + assert writer is mock_writer @pytest.mark.asyncio -async def test_async_send_command_no_address(): +async def test_async_open_connection_no_address(): mock_loop = MagicMock() with pytest.raises(ValueError, match="unable to connect"): - await async_send_command(mock_loop, b"cmd") + await async_open_connection(mock_loop) @pytest.mark.asyncio -async def test_async_send_command_serial_settle_delay(): +async def test_async_open_connection_serial_settle_delay(): mock_loop = MagicMock() mock_reader = AsyncMock() mock_writer = MagicMock() - mock_writer.drain = AsyncMock() - mock_writer.wait_closed = AsyncMock() - - mock_reader.read.return_value = b"response" + HtdConstants.MESSAGE_HEADER with patch("htd_client.utils.open_serial_connection", new_callable=AsyncMock) as mock_open, \ patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: mock_open.return_value = (mock_reader, mock_writer) - await async_send_command(mock_loop, b"cmd", serial_address="/dev/ttyUSB0", settle_delay=1.5) + await async_open_connection(mock_loop, serial_address="/dev/ttyUSB0", settle_delay=1.5) mock_sleep.assert_called_once_with(1.5) @pytest.mark.asyncio -async def test_async_send_command_no_settle_delay_by_default(): +async def test_async_open_connection_no_settle_delay_by_default(): mock_loop = MagicMock() mock_reader = AsyncMock() mock_writer = MagicMock() - mock_writer.drain = AsyncMock() - mock_writer.wait_closed = AsyncMock() - - mock_reader.read.return_value = b"response" + HtdConstants.MESSAGE_HEADER with patch("htd_client.utils.open_serial_connection", new_callable=AsyncMock) as mock_open, \ patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: mock_open.return_value = (mock_reader, mock_writer) - await async_send_command(mock_loop, b"cmd", serial_address="/dev/ttyUSB0") + await async_open_connection(mock_loop, serial_address="/dev/ttyUSB0") mock_sleep.assert_not_called() + +@pytest.mark.asyncio +async def test_async_read_response_returns_after_quiet_window(): + """Without a predicate, reading stops once the line goes quiet.""" + import asyncio as _asyncio + + mock_reader = MagicMock() + chunks = [b"response"] + + async def read(n): + if chunks: + return chunks.pop(0) + await _asyncio.get_running_loop().create_future() + + mock_reader.read = read + + response = await async_read_response(mock_reader, timeout=0.5, quiet_window=0.02) + + assert response == b"response" diff --git a/tests/test_utils_coverage_gap.py b/tests/test_utils_coverage_gap.py index 9e5552f..bef544c 100644 --- a/tests/test_utils_coverage_gap.py +++ b/tests/test_utils_coverage_gap.py @@ -20,21 +20,13 @@ def test_build_command_with_extra_data(): assert cmd[6] == 5 @pytest.mark.asyncio -async def test_async_send_command_no_header(): - # Test async_send_command when response has no header (line 104) - loop = AsyncMock() - # Mock open_connection +async def test_async_read_response_returns_empty_on_eof(): + # An immediately-closed stream yields empty bytes, not a hang reader = AsyncMock() - writer = AsyncMock() - - # Return data without header - reader.read.return_value = b'\x00\x00\x00\x00' - - with patch('asyncio.open_connection', return_value=(reader, writer)): - response = await htd_client.utils.async_send_command( - loop, - b'cmd', - network_address=('localhost', 1234) - ) - - assert response == b'\x00\x00\x00\x00' + reader.read.return_value = b"" + + response = await htd_client.utils.async_read_response( + reader, timeout=0.5, quiet_window=0.02 + ) + + assert response == b"" From aa3d267ccb2f24d90910ab1affeb41a2332e97c3 Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Fri, 10 Jul 2026 21:18:56 -0600 Subject: [PATCH 09/14] fix: resync minimally on checksum failure instead of trusting the frame A checksum mismatch means the parsed zone/command/length can't be trusted -- it may be a coincidental match on misaligned or corrupted bytes -- so skipping the full presumed frame length left the parser permanently desynced after a single dropped/corrupted byte from a flaky USB-serial adapter. Resync by header length only, matching the existing unknown-command recovery path, so one bad frame no longer cascades into a stream of "Bad sync buffer" / "Invalid command value" errors during normal zone control. Co-Authored-By: Claude Sonnet 5 --- htd_client/base_client.py | 7 ++++ tests/test_base_client_edge_cases.py | 56 +++++++++++++++++++--------- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 2b4a513..07fddf7 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -290,6 +290,13 @@ def _process_next_command(self, data: bytes): else: _LOGGER.info("Bad checksum %02x != %02x", frame_sum_checksum, checksum) + # the checksum mismatch means this frame's declared length can't + # be trusted (zone/command may be a coincidental match on + # misaligned or corrupted bytes), so don't skip the full + # presumed frame or a genuinely desynced buffer never recovers. + # Resync minimally, same as the unknown-command path above. + return None, start_message_index + HtdConstants.MESSAGE_HEADER_LENGTH + return zone, chunk_length def _parse_command(self, zone, cmd, data): diff --git a/tests/test_base_client_edge_cases.py b/tests/test_base_client_edge_cases.py index aef97a4..33d15c5 100644 --- a/tests/test_base_client_edge_cases.py +++ b/tests/test_base_client_edge_cases.py @@ -86,30 +86,52 @@ def test_process_next_command_checksum_fail(client): # Expected len 9. data = bytes([0]*9) checksum = 0xFF # Invalid - + full = header + bytes([zone, cmd]) + data + bytes([checksum]) - - # We need to mock calculate_checksum to fail? Or just pass wrong checksum. - # Correct checksum is calculate_checksum... - # We pass 0xFF. - + + # A checksum failure means the declared frame length can't be trusted + # (it may be a coincidental match on garbage/misaligned bytes), so we + # must NOT skip the whole presumed frame. Resync minimally by only the + # header length, same as the unknown-command path, so the next byte + # onward is re-scanned for a real header instead of staying corrupted. zone_ret, consumed = client._process_next_command(full) - # Returns chunk len but zone is returned? - # expected_len = 9. - # end_index = 0 + 2 + 2 + 9 = 13? - # Wait: end = start + HEADER + 2 + expected_len? - # No: end = start + HEADER + 2 + length? - # Code: end_message_index = start + HEADER_LEN + 2 + expected_length - # Wait, command map gives data length. - # frame includes zone + cmd + data? - # Checksum is on zone+cmd+data. - - assert consumed == len(full) + assert zone_ret is None + assert consumed == HtdConstants.MESSAGE_HEADER_LENGTH + # Check that _parse_command NOT called with patch.object(client, '_parse_command') as mock_parse: client._process_next_command(full) mock_parse.assert_not_called() +def test_process_next_command_checksum_fail_recovers_next_frame(client): + """A single corrupted frame must not desync parsing of the frame after it.""" + header = bytes([HtdConstants.HEADER_BYTE, HtdConstants.RESERVED_BYTE]) + + def build_zone_status(zone, checksum_override=None): + cmd = HtdCommonCommands.ZONE_STATUS_RECEIVE_COMMAND + data = bytes([0, 0, 0, 0, 1, 0xe2, 0, 0, 0]) + frame = header + bytes([zone, cmd]) + data + checksum = checksum_override + if checksum is None: + checksum = sum(frame) & 0xFF + return frame + bytes([checksum]) + + bad_frame = build_zone_status(1, checksum_override=0xFF) + good_frame = build_zone_status(2) + buffer = bad_frame + good_frame + + with patch.object(client, '_parse_command') as mock_parse: + zone_ret, consumed = client._process_next_command(buffer) + assert zone_ret is None + assert consumed == HtdConstants.MESSAGE_HEADER_LENGTH + mock_parse.assert_not_called() + + # advance past the corrupted frame's header the same way data_received does + remaining = buffer[consumed:] + zone_ret, consumed = client._process_next_command(remaining) + assert zone_ret == 2 + mock_parse.assert_called_once() + def test_parse_command_Zonename(client): zone = 1 cmd = HtdCommonCommands.ZONE_NAME_RECEIVE_COMMAND From 1d58ebfb4c90f8b81f13f7e32282591283948d60 Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sun, 12 Jul 2026 18:19:13 -0600 Subject: [PATCH 10/14] feat: raise HtdConnectionError instead of ValueError/OSError when the device is unreachable at setup --- htd_client/__init__.py | 26 +++++++++++++++++--------- htd_client/exceptions.py | 2 ++ tests/test_init.py | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 htd_client/exceptions.py diff --git a/htd_client/__init__.py b/htd_client/__init__.py index 87c0712..bef49ab 100644 --- a/htd_client/__init__.py +++ b/htd_client/__init__.py @@ -18,6 +18,7 @@ import htd_client.utils from .base_client import BaseClient from .constants import HtdCommonCommands, HtdModelInfo, HtdDeviceKind, HtdConstants +from .exceptions import HtdConnectionError from .lync_client import HtdLyncClient from .mca_client import HtdMcaClient @@ -43,16 +44,20 @@ async def async_get_client( HtdClient: The new client object. """ - model_info = await async_get_model_info( - loop if loop is not None else asyncio.get_running_loop(), - network_address=network_address, - serial_address=serial_address, - retry_attempts=retry_attempts, - ) + address = f"serial: {serial_address}" if serial_address is not None else f"network: {network_address}" + + try: + model_info = await async_get_model_info( + loop if loop is not None else asyncio.get_running_loop(), + network_address=network_address, + serial_address=serial_address, + retry_attempts=retry_attempts, + ) + except OSError as e: + raise HtdConnectionError(f"Unable to connect to HTD device ({address}): {e}") from e if model_info is None: - address = f"serial: {serial_address}" if serial_address is not None else f"network: {network_address}" - raise ValueError( + raise HtdConnectionError( f"Unable to detect HTD device model ({address}). " f"Verify the device is powered on and the path/address is correct." ) @@ -78,7 +83,10 @@ async def async_get_client( else: raise ValueError(f"Unknown Device Kind: {model_info['kind']}") - await client.async_connect() + try: + await client.async_connect() + except OSError as e: + raise HtdConnectionError(f"Unable to connect to HTD device ({address}): {e}") from e return client diff --git a/htd_client/exceptions.py b/htd_client/exceptions.py new file mode 100644 index 0000000..93fc32f --- /dev/null +++ b/htd_client/exceptions.py @@ -0,0 +1,2 @@ +class HtdConnectionError(Exception): + """Raised when the HTD device could not be reached during initial setup.""" diff --git a/tests/test_init.py b/tests/test_init.py index 3d2f450..d8c4068 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, AsyncMock, patch from htd_client import async_get_client, async_get_model_info, HtdMcaClient, HtdLyncClient from htd_client.constants import HtdConstants, HtdDeviceKind +from htd_client.exceptions import HtdConnectionError def _mock_connection(): mock_reader = AsyncMock() @@ -121,5 +122,35 @@ async def test_async_get_client_raises_clear_error_when_model_undetected(): with patch("htd_client.async_get_model_info", new_callable=AsyncMock) as mock_get_info: mock_get_info.return_value = None - with pytest.raises(ValueError, match="Unable to detect HTD device model"): + with pytest.raises(HtdConnectionError, match="Unable to detect HTD device model"): await async_get_client(loop=mock_loop, serial_address="/dev/serial/by-id/usb-example") + +@pytest.mark.asyncio +async def test_async_get_client_wraps_os_error_from_probe(): + mock_loop = MagicMock() + + with patch("htd_client.async_get_model_info", new_callable=AsyncMock) as mock_get_info: + mock_get_info.side_effect = OSError("Connection refused") + + with pytest.raises(HtdConnectionError) as exc_info: + await async_get_client(loop=mock_loop, network_address=("1.2.3.4", 10006)) + + assert isinstance(exc_info.value.__cause__, OSError) + +@pytest.mark.asyncio +async def test_async_get_client_wraps_os_error_from_connect(): + mock_loop = MagicMock() + + with patch("htd_client.async_get_model_info", new_callable=AsyncMock) as mock_get_info: + model_info = HtdConstants.SUPPORTED_MODELS["mca66"] + mock_get_info.return_value = model_info + + with patch( + "htd_client.mca_client.HtdMcaClient.async_connect", new_callable=AsyncMock + ) as mock_connect: + mock_connect.side_effect = OSError("Connection refused") + + with pytest.raises(HtdConnectionError) as exc_info: + await async_get_client(loop=mock_loop, network_address=("1.2.3.4", 10006)) + + assert isinstance(exc_info.value.__cause__, OSError) From d716af23a53100cf7123d815b4c425e1ad414941 Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sun, 23 Aug 2026 15:00:09 -0600 Subject: [PATCH 11/14] feat: add model key and cached name accessors for offline restore Give each SUPPORTED_MODELS entry a persistable "key" and add get_model_info(key) so a caller can rebuild the correct client class from storage alone, without probing the device. Add get_source_names() and get_zone_names() so only controller-confirmed names (never placeholders) are safe to persist across restarts. --- htd_client/__init__.py | 20 ++++++++++++ htd_client/base_client.py | 20 ++++++++++++ htd_client/constants.py | 4 +++ tests/test_model_key.py | 64 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 tests/test_model_key.py diff --git a/htd_client/__init__.py b/htd_client/__init__.py index bef49ab..f0844a3 100644 --- a/htd_client/__init__.py +++ b/htd_client/__init__.py @@ -25,6 +25,26 @@ _LOGGER = logging.getLogger(__name__) +def get_model_info(key: str | None) -> HtdModelInfo | None: + """ + Look up a model definition by its persistable key, without touching the device. + + This is the counterpart to `async_get_model_info`: a consumer records + `model_info["key"]` once, and can rebuild the same client on later runs while the + device is powered off. + + Args: + key (str | None): a key from `HtdConstants.SUPPORTED_MODELS`, e.g. "mca66". + + Returns: + HtdModelInfo | None: the model definition, or None if the key is unknown or missing. + """ + if key is None: + return None + + return HtdConstants.SUPPORTED_MODELS.get(key) + + async def async_get_client( serial_address: str = None, network_address: Tuple[str, int] = None, diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 07fddf7..591e8eb 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -49,6 +49,8 @@ class BaseClient(asyncio.Protocol): _heartbeat_task: asyncio.Task = None _buffer: bytearray | None = None _zone_data: Dict[int, ZoneDetail] = None + _source_names: Dict[int, str] = None + _zone_names: Dict[int, str] = None _zones_loaded: int = 0 _connected: bool = False _ready: bool = False @@ -524,6 +526,24 @@ def get_zone_name(self, zone: int) -> str | None: """Get the cached zone name, or None if not yet queried.""" return self._zone_names.get(zone) + def get_source_names(self) -> Dict[int, str]: + """ + Get every source name the controller has actually reported. + + Unlike `get_source_name`, this never invents a placeholder, so the result is safe + to persist across restarts. + """ + return dict(self._source_names or {}) + + def get_zone_names(self) -> Dict[int, str]: + """ + Get every zone name the controller has actually reported. + + Unlike `get_zone_name`, this never invents a placeholder, so the result is safe + to persist across restarts. + """ + return dict(self._zone_names or {}) + def get_zone(self, zone: int): """ Query a zone and return `ZoneDetail` diff --git a/htd_client/constants.py b/htd_client/constants.py index e265515..fc4cd6c 100644 --- a/htd_client/constants.py +++ b/htd_client/constants.py @@ -11,6 +11,7 @@ class HtdDeviceKind(Enum): class HtdModelInfo(TypedDict): + key: str zones: int sources: int friendly_name: str @@ -29,6 +30,7 @@ class HtdConstants: SUPPORTED_MODELS: Dict[str, HtdModelInfo] = { "mca66": { + "key": "mca66", "identifier": b'Wangine_MCA66', "zones": 6, "sources": 6, @@ -37,6 +39,7 @@ class HtdConstants: "kind": HtdDeviceKind.mca, }, "lync6": { + "key": "lync6", "identifier": b"Lync 6", "zones": 6, "sources": 12, @@ -45,6 +48,7 @@ class HtdConstants: "kind": HtdDeviceKind.lync, }, "lync12": { + "key": "lync12", "identifier": b"Lync12", "zones": 12, "sources": 19, diff --git a/tests/test_model_key.py b/tests/test_model_key.py new file mode 100644 index 0000000..eb9e341 --- /dev/null +++ b/tests/test_model_key.py @@ -0,0 +1,64 @@ +"""The model key is the single thing that makes an offline restart possible: it is what a +consumer persists so it can rebuild the right client without asking the device who it is.""" +import asyncio + +import pytest + +from htd_client import build_client, get_model_info +from htd_client.constants import HtdConstants + + +def test_every_supported_model_carries_its_own_lookup_key(): + """The key is what gets written to storage. If it does not match the dict key it was + stored under, a round-trip through storage resolves to the wrong hardware — a Lync 12 + entry rebuilt as an MCA66 would silently create 6 entities instead of 12.""" + for name, model in HtdConstants.SUPPORTED_MODELS.items(): + assert model["key"] == name + + +def test_get_model_info_round_trips_a_persisted_key(): + """A consumer that stored "mca66" last run must get back the same zone and source counts + with the amplifier powered off. These two numbers are what gate entity creation.""" + model = get_model_info("mca66") + assert model is HtdConstants.SUPPORTED_MODELS["mca66"] + assert model["zones"] == 6 + assert model["sources"] == 6 + + +def test_get_model_info_returns_none_for_an_unknown_key(): + """An entry written by a newer version and then rolled back must degrade to a live probe, + not raise. The caller branches on None to choose the probe path.""" + assert get_model_info("lync24") is None + + +def test_get_model_info_returns_none_for_a_missing_key(): + """Config entries created before this feature have no key at all. The caller passes the + missing value straight through rather than special-casing it.""" + assert get_model_info(None) is None + + +@pytest.mark.asyncio +async def test_get_source_names_reports_only_what_the_controller_said(): + """Callers persist this dict across restarts. get_source_name() invents a "Source N" + placeholder for anything unknown, so caching its output would freeze placeholders into + storage forever and permanently destroy real controller names.""" + client = build_client( + HtdConstants.SUPPORTED_MODELS["lync12"], + network_address=("1.2.3.4", 10006), + ) + client._source_names = {2: "turntable"} + + assert client.get_source_names() == {2: "turntable"} + assert client.get_source_name(1) == "Source 1" + + +@pytest.mark.asyncio +async def test_get_zone_names_reports_only_what_the_controller_said(): + """Same contract as sources, and the one the name-cache regression guard depends on.""" + client = build_client( + HtdConstants.SUPPORTED_MODELS["lync12"], + network_address=("1.2.3.4", 10006), + ) + client._zone_names = {3: "kitchen"} + + assert client.get_zone_names() == {3: "kitchen"} From 156d1a6ce0649beb5a0e7e28933cfbb0bb8bc73f Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sun, 23 Aug 2026 15:00:18 -0600 Subject: [PATCH 12/14] feat: add build_client factory and rewrite async_get_client on top of it --- htd_client/__init__.py | 75 ++++++++++++++++++++++++---------- tests/test_build_client.py | 84 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 tests/test_build_client.py diff --git a/htd_client/__init__.py b/htd_client/__init__.py index f0844a3..bee9df1 100644 --- a/htd_client/__init__.py +++ b/htd_client/__init__.py @@ -45,6 +45,52 @@ def get_model_info(key: str | None) -> HtdModelInfo | None: return HtdConstants.SUPPORTED_MODELS.get(key) +def build_client( + model_info: HtdModelInfo, + *, + serial_address: str = None, + network_address: Tuple[str, int] = None, + loop: asyncio.AbstractEventLoop = None, + retry_attempts: int = HtdConstants.DEFAULT_RETRY_ATTEMPTS, +) -> BaseClient: + """ + Create a client from a known model definition, without touching the device. + + No I/O happens here — nothing is opened, probed or connected. Call `async_connect()` + or `async_start()` afterwards. + + Args: + model_info (HtdModelInfo): the model definition, from `get_model_info` or + `async_get_model_info`. + network_address (Tuple[str, int]): the address to communicate with over TCP. + serial_address (str): the location of the serial port. + loop (asyncio.AbstractEventLoop): the event loop to use. + retry_attempts (int): number of times to retry a command before failing. + + Returns: + BaseClient: an unconnected client of the class matching the model's kind. + + Raises: + ValueError: the model's kind is not recognized. + """ + resolved_loop = loop if loop is not None else asyncio.get_running_loop() + + if model_info["kind"] == HtdDeviceKind.mca: + client_class = HtdMcaClient + elif model_info["kind"] == HtdDeviceKind.lync: + client_class = HtdLyncClient + else: + raise ValueError(f"Unknown Device Kind: {model_info['kind']}") + + return client_class( + resolved_loop, + model_info, + network_address=network_address, + serial_address=serial_address, + retry_attempts=retry_attempts, + ) + + async def async_get_client( serial_address: str = None, network_address: Tuple[str, int] = None, @@ -52,7 +98,7 @@ async def async_get_client( retry_attempts: int = HtdConstants.DEFAULT_RETRY_ATTEMPTS, ) -> BaseClient: """ - Create a new client object. + Probe the device for its model, then create and connect a client. Args: network_address (str): The address to communicate with over TCP. @@ -82,26 +128,13 @@ async def async_get_client( f"Verify the device is powered on and the path/address is correct." ) - if model_info["kind"] == HtdDeviceKind.mca: - client = HtdMcaClient( - loop if loop is not None else asyncio.get_running_loop(), - model_info, - network_address=network_address, - serial_address=serial_address, - retry_attempts=retry_attempts, - ) - - elif model_info["kind"] == HtdDeviceKind.lync: - client = HtdLyncClient( - loop if loop is not None else asyncio.get_running_loop(), - model_info, - network_address=network_address, - serial_address=serial_address, - retry_attempts=retry_attempts, - ) - - else: - raise ValueError(f"Unknown Device Kind: {model_info['kind']}") + client = build_client( + model_info, + serial_address=serial_address, + network_address=network_address, + loop=loop, + retry_attempts=retry_attempts, + ) try: await client.async_connect() diff --git a/tests/test_build_client.py b/tests/test_build_client.py new file mode 100644 index 0000000..ede3520 --- /dev/null +++ b/tests/test_build_client.py @@ -0,0 +1,84 @@ +"""build_client is what makes offline setup possible: it must decide zone and source counts +from a stored model definition alone, without opening a connection.""" +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from htd_client import build_client, HtdLyncClient, HtdMcaClient +from htd_client.constants import HtdConstants + + +@pytest.mark.asyncio +async def test_build_client_selects_the_mca_class(): + """The class carries the protocol dialect. Building the wrong one sends Lync frames to + an MCA66 and every command is silently ignored.""" + client = build_client( + HtdConstants.SUPPORTED_MODELS["mca66"], + network_address=("1.2.3.4", 10006), + ) + assert isinstance(client, HtdMcaClient) + assert client.get_zone_count() == 6 + + +@pytest.mark.asyncio +async def test_build_client_selects_the_lync_class(): + """Lync 12 must yield 12 zones and 19 sources with the amplifier unplugged, or the + offline restart creates the wrong number of entities.""" + client = build_client( + HtdConstants.SUPPORTED_MODELS["lync12"], + network_address=("1.2.3.4", 10006), + ) + assert isinstance(client, HtdLyncClient) + assert client.get_zone_count() == 12 + assert client.get_source_count() == 19 + + +@pytest.mark.asyncio +async def test_build_client_opens_no_connection(): + """This is the whole contract. If build_client touched the transport, the offline setup + path could not exist — it would fail for exactly the users it is meant to serve.""" + with patch( + "htd_client.utils.async_open_connection", new_callable=AsyncMock + ) as mock_open: + client = build_client( + HtdConstants.SUPPORTED_MODELS["mca66"], + serial_address="/dev/ttyUSB0", + ) + + mock_open.assert_not_called() + assert client.connected is False + assert client.ready is False + + +@pytest.mark.asyncio +async def test_build_client_rejects_an_unknown_kind(): + """A malformed model definition must fail at the factory rather than producing a client + that looks usable and does nothing.""" + bad_model = {**HtdConstants.SUPPORTED_MODELS["mca66"], "kind": "not-a-kind"} + + with pytest.raises(ValueError): + build_client(bad_model, network_address=("1.2.3.4", 10006)) + + +@pytest.mark.asyncio +async def test_async_get_client_still_probes_then_connects(): + """Guards the refactor. First-time setup and every legacy config entry still go through + this path, and its behavior must not shift.""" + mock_client = MagicMock() + mock_client.async_connect = AsyncMock() + + with patch( + "htd_client.async_get_model_info", new_callable=AsyncMock + ) as mock_probe, patch( + "htd_client.build_client", return_value=mock_client + ) as mock_build: + mock_probe.return_value = HtdConstants.SUPPORTED_MODELS["mca66"] + + from htd_client import async_get_client + + result = await async_get_client(network_address=("1.2.3.4", 10006)) + + assert result is mock_client + mock_probe.assert_awaited_once() + mock_build.assert_called_once() + mock_client.async_connect.assert_awaited_once() From 12f3cf556e016afbe0f39b1ed26e5b05c3b4e91e Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sun, 23 Aug 2026 15:07:42 -0600 Subject: [PATCH 13/14] feat: add non-raising async_start, single-handle reconnect loop, and safe disconnect async_start() connects once and, on failure, hands off to a while-loop reconnect task guarded by a single handle (_ensure_reconnect_task) instead of the old self-rescheduling _async_reconnect, which left a window where connection_lost could see a stale, not-done task handle and fail to start a new attempt. disconnect() is now safe on a client that never connected and cancels any in-flight retry task. Updates test_reconnect_logic in test_base_client_coverage.py, which asserted the old self-rescheduling behavior (asyncio.create_task called from within _async_reconnect); it now asserts the loop's observable contract (two connect attempts, then return) instead. --- htd_client/base_client.py | 65 +++++++++--- tests/test_async_start.py | 155 +++++++++++++++++++++++++++++ tests/test_base_client_coverage.py | 19 ++-- 3 files changed, 212 insertions(+), 27 deletions(-) create mode 100644 tests/test_async_start.py diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 591e8eb..237e9de 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -121,6 +121,24 @@ async def async_connect(self): else: raise ValueError("No address provided") + async def async_start(self) -> None: + """ + Connect if the device is reachable, and otherwise keep trying in the background. + + Unlike `async_connect`, this never raises. It is meant for a long-lived consumer + that wants to come up whether or not the device is currently powered on. + """ + self._disconnected = False + + try: + await self.async_connect() + except Exception as e: + _LOGGER.warning( + "Initial connection to the HTD device failed (%s); retrying in the background", + e, + ) + self._ensure_reconnect_task() + def connection_made(self, transport: Transport): _LOGGER.debug("connected") self._connected = True @@ -176,22 +194,35 @@ def connection_lost(self, exc): self._heartbeat_task.cancel() if not self._disconnected: - if self._reconnect_task is None or self._reconnect_task.done(): - self._reconnect_task = asyncio.create_task(self._async_reconnect()) + self._ensure_reconnect_task() - async def _async_reconnect(self): - """Reconnect with exponential backoff.""" - _LOGGER.info(f"Attempting to reconnect in {self._reconnect_delay} seconds...") - await asyncio.sleep(self._reconnect_delay) - - try: - await self.async_connect() - # Delay is reset in connection_made upon success - except Exception as e: - _LOGGER.error(f"Reconnection attempt failed: {e}") - self._reconnect_delay = min(self._reconnect_delay * 2, self._max_reconnect_delay) + def _ensure_reconnect_task(self): + """Start the backoff loop unless one is already running. + + A single task handle owns the whole loop. The previous implementation re-scheduled + itself by reassigning this handle from inside its own running task, which left a + window where `connection_lost` saw a stale not-done handle and started a second loop. + """ + if self._reconnect_task is None or self._reconnect_task.done(): self._reconnect_task = asyncio.create_task(self._async_reconnect()) + async def _async_reconnect(self): + """Reconnect with exponential backoff until connected or torn down.""" + while not self._disconnected and not self._connected: + _LOGGER.info(f"Attempting to reconnect in {self._reconnect_delay} seconds...") + await asyncio.sleep(self._reconnect_delay) + + if self._disconnected: + return + + try: + await self.async_connect() + # Delay is reset in connection_made upon success + return + except Exception as e: + _LOGGER.error(f"Reconnection attempt failed: {e}") + self._reconnect_delay = min(self._reconnect_delay * 2, self._max_reconnect_delay) + async def async_wait_until_ready(self): start_time = time.time() @@ -206,7 +237,13 @@ def has_zone_data(self, zone: int): def disconnect(self): self._disconnected = True - self._connection.close() + + if self._reconnect_task is not None: + self._reconnect_task.cancel() + self._reconnect_task = None + + if self._connection is not None: + self._connection.close() def _process_next_command(self, data: bytes): diff --git a/tests/test_async_start.py b/tests/test_async_start.py new file mode 100644 index 0000000..4988ff0 --- /dev/null +++ b/tests/test_async_start.py @@ -0,0 +1,155 @@ +"""Once deferred setup succeeds, Home Assistant stops retrying the config entry. The +library's backoff loop becomes the only thing that can bring the device back, so it must +start on a first failure, survive repeated failures, and stop cleanly on teardown.""" +import asyncio +from unittest.mock import AsyncMock, patch + +import pytest + +from htd_client import build_client +from htd_client.constants import HtdConstants + + +def _client(): + return build_client( + HtdConstants.SUPPORTED_MODELS["mca66"], + network_address=("1.2.3.4", 10006), + ) + + +@pytest.mark.asyncio +async def test_async_start_does_not_raise_when_the_device_is_unreachable(): + """This is the entire fix for issue #30. If start raises, the integration falls back to + ConfigEntryNotReady, no entities are created, and every HTD card stays Unavailable.""" + client = _client() + + with patch.object(client, "async_connect", new_callable=AsyncMock) as connect: + connect.side_effect = OSError("no route to host") + await client.async_start() + + assert client.connected is False + client.disconnect() + + +@pytest.mark.asyncio +async def test_async_start_schedules_a_retry_after_a_failed_first_connect(): + """A client that has never connected previously had no retry loop at all — reconnect was + only reachable from connection_lost. Without this the amplifier never comes back.""" + client = _client() + + with patch.object(client, "async_connect", new_callable=AsyncMock) as connect: + connect.side_effect = OSError("no route to host") + await client.async_start() + + assert client._reconnect_task is not None + assert not client._reconnect_task.done() + client.disconnect() + + +@pytest.mark.asyncio +async def test_async_start_does_not_schedule_a_retry_when_the_first_connect_works(): + """A healthy start must not leave a redundant loop dialing a device that is already up.""" + client = _client() + + with patch.object(client, "async_connect", new_callable=AsyncMock): + await client.async_start() + + assert client._reconnect_task is None + + +@pytest.mark.asyncio +async def test_reconnect_backoff_doubles_to_a_sixty_second_ceiling(): + """An amplifier can be off for days. Retrying every second for days hammers the network + and the log; an unbounded backoff would eventually never retry at all.""" + client = _client() + # _disconnected defaults to True until async_connect/async_start runs; calling + # _async_reconnect() directly (bypassing async_start) needs the same reset that + # test_reconnect_logic in test_base_client_coverage.py already relies on. + client._disconnected = False + delays = [] + + async def fake_sleep(delay): + delays.append(delay) + if len(delays) >= 8: + client._disconnected = True + + with patch("htd_client.base_client.asyncio.sleep", new=fake_sleep), patch.object( + client, "async_connect", new_callable=AsyncMock + ) as connect: + connect.side_effect = OSError("still off") + await client._async_reconnect() + + assert delays[:4] == [1.0, 2.0, 4.0, 8.0] + assert delays[-1] == 60.0 + + +@pytest.mark.asyncio +async def test_reconnect_loop_exits_on_a_successful_connect(): + """The loop must stop dialing once it is back, or a reconnected client keeps opening + connections underneath itself. On serial, every open can DTR-reset the gateway.""" + client = _client() + # See note above: _async_reconnect() is being called directly, so _disconnected + # needs the same manual reset async_start would otherwise perform. + client._disconnected = False + attempts = [] + + async def fake_sleep(delay): + attempts.append(delay) + + async def connect_third_time(): + if len(attempts) < 3: + raise OSError("still off") + client._connected = True + + with patch("htd_client.base_client.asyncio.sleep", new=fake_sleep), patch.object( + client, "async_connect", side_effect=connect_third_time + ): + await client._async_reconnect() + + assert len(attempts) == 3 + + +@pytest.mark.asyncio +async def test_reconnect_delay_resets_after_a_successful_connect(): + """Without a reset, a device that flaps once inherits a 60s recovery time for the rest of + the process lifetime.""" + client = _client() + client._reconnect_delay = 32.0 + + client.connection_made(AsyncMock()) + await asyncio.sleep(0) + + assert client._reconnect_delay == 1.0 + if client._heartbeat_task is not None: + client._heartbeat_task.cancel() + + +def test_disconnect_is_safe_on_a_client_that_never_connected(): + """Unload runs even when the device was never reachable. An AttributeError here leaves + the config entry half-torn-down and the retry task orphaned.""" + client = build_client( + HtdConstants.SUPPORTED_MODELS["mca66"], + network_address=("1.2.3.4", 10006), + loop=asyncio.new_event_loop(), + ) + + client.disconnect() + + assert client._disconnected is True + + +@pytest.mark.asyncio +async def test_disconnect_cancels_an_inflight_retry_task(): + """A retry loop that outlives the config entry keeps dialing hardware Home Assistant no + longer manages, forever.""" + client = _client() + + with patch.object(client, "async_connect", new_callable=AsyncMock) as connect: + connect.side_effect = OSError("no route to host") + await client.async_start() + + task = client._reconnect_task + client.disconnect() + await asyncio.sleep(0) + + assert task.cancelled() or task.done() diff --git a/tests/test_base_client_coverage.py b/tests/test_base_client_coverage.py index 2c8d657..832bb59 100644 --- a/tests/test_base_client_coverage.py +++ b/tests/test_base_client_coverage.py @@ -94,22 +94,15 @@ async def test_reconnect_logic(client): # Actually checking internal logic of _async_reconnect handles exceptions and retries # Rerunning logic: - # await client._async_reconnect() -> calls connect() -> raises Exception. - # catches exception -> updates delay -> creates task. - # func returns. - - # Check that new task was created - # We need to check if asyncio.create_task was called. - # But we didn't patch it inside the test function scope valid for the 'await client._async_reconnect()'. - # We need to wrap the call. - + # await client._async_reconnect() -> loops internally: calls connect() -> raises + # Exception, doubles the delay, and keeps looping until connect() succeeds, then + # returns. + with patch.object(client, 'async_connect', new_callable=AsyncMock) as mock_connect: mock_connect.side_effect = [Exception("Fail"), None] with patch("asyncio.sleep", new_callable=AsyncMock): - with patch("asyncio.create_task") as mock_create_task: - await client._async_reconnect() - assert mock_connect.call_count == 1 - mock_create_task.assert_called() + await client._async_reconnect() + assert mock_connect.call_count == 2 def test_data_received(client): # Mock _process_next_command to consume data From fdfdb0a41f3c6a93b0e1fa6d6605b39b7d51df30 Mon Sep 17 00:00:00 2001 From: Landon Harsh Date: Sun, 23 Aug 2026 15:16:02 -0600 Subject: [PATCH 14/14] fix: raise HtdConnectionError instead of AttributeError when disconnected, and broadcast connection state - _send_cmd, HtdMcaClient.refresh, and HtdLyncClient.refresh now raise HtdConnectionError when the connection is None or not connected, instead of the AttributeError that came from writing to a None transport. - The heartbeat loop exits quietly on HtdConnectionError instead of dying with an unretrieved task exception. - connection_made and connection_lost each broadcast(None) so subscribed entities (should_poll = False) repaint on connect/disconnect transitions. - Fixed test fixtures/tests that set _connection but never _connected, which the new guard exposed as tests implicitly assuming a connected state: test_send_and_validate_success/timeout, htd_instance (test_client.py), lync_client (test_lync_client_coverage.py), mca_client (test_mca_client_coverage.py). --- htd_client/base_client.py | 13 +++- htd_client/lync_client.py | 4 ++ htd_client/mca_client.py | 4 ++ tests/test_base_client_coverage.py | 2 + tests/test_client.py | 2 + tests/test_lync_client_coverage.py | 1 + tests/test_mca_client_coverage.py | 1 + tests/test_offline_commands.py | 109 +++++++++++++++++++++++++++++ 8 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/test_offline_commands.py diff --git a/htd_client/base_client.py b/htd_client/base_client.py index 237e9de..500dc76 100644 --- a/htd_client/base_client.py +++ b/htd_client/base_client.py @@ -23,6 +23,7 @@ import htd_client from .constants import HtdConstants, HtdDeviceKind, ONE_SECOND, HtdModelInfo, HtdCommonCommands +from .exceptions import HtdConnectionError from .models import ZoneDetail _LOGGER = logging.getLogger(__name__) @@ -145,6 +146,7 @@ def connection_made(self, transport: Transport): self._connection = transport self._reconnect_delay = 1.0 self._heartbeat_task = asyncio.create_task(self._heartbeat()) + self._loop.create_task(self._broadcast(None)) async def _heartbeat(self): @@ -154,7 +156,12 @@ async def _heartbeat(self): await asyncio.sleep(HtdConstants.SERIAL_SETTLE_DELAY) while self._connected: - await self.refresh() + try: + await self.refresh() + except HtdConnectionError: + # the link dropped between the loop check and the write; connection_lost + # already owns the reconnect, so exit rather than raising out of a bare task + return await asyncio.sleep(60) @@ -196,6 +203,8 @@ def connection_lost(self, exc): if not self._disconnected: self._ensure_reconnect_task() + self._loop.create_task(self._broadcast(None)) + def _ensure_reconnect_task(self): """Start the backoff loop unless one is already running. @@ -529,6 +538,8 @@ async def _send_cmd( data_code: int, extra_data: bytearray = None ): + if self._connection is None or not self._connected: + raise HtdConnectionError("not connected") cmd = htd_client.utils.build_command(zone, command, data_code, extra_data) diff --git a/htd_client/lync_client.py b/htd_client/lync_client.py index adb72dd..a8fc2d0 100644 --- a/htd_client/lync_client.py +++ b/htd_client/lync_client.py @@ -18,6 +18,7 @@ import htd_client.utils from .base_client import BaseClient from .constants import HtdConstants, HtdLyncCommands, HtdLyncConstants, HtdModelInfo +from .exceptions import HtdConnectionError _LOGGER = logging.getLogger(__name__) @@ -83,6 +84,9 @@ async def refresh(self, zone: int = None): Args: zone (int): the zone to refresh, or None to refresh all zones """ + if self._connection is None or not self._connected: + raise HtdConnectionError("not connected") + await self._send_cmd( zone if zone is not None else 0, HtdLyncCommands.QUERY_COMMAND_CODE, diff --git a/htd_client/mca_client.py b/htd_client/mca_client.py index 3670c4d..6b94621 100644 --- a/htd_client/mca_client.py +++ b/htd_client/mca_client.py @@ -17,6 +17,7 @@ from .base_client import BaseClient from .constants import HtdConstants, HtdMcaCommands, HtdMcaConstants, HtdModelInfo +from .exceptions import HtdConnectionError from .models import ZoneDetail _LOGGER = logging.getLogger(__name__) @@ -191,6 +192,9 @@ async def refresh(self, zone: int = None): number, and the value are the details of the zone """ + if self._connection is None or not self._connected: + raise HtdConnectionError("not connected") + refresh_zone = zone if zone is not None else 0 await self.refresh_zone(refresh_zone) diff --git a/tests/test_base_client_coverage.py b/tests/test_base_client_coverage.py index 832bb59..6c083d2 100644 --- a/tests/test_base_client_coverage.py +++ b/tests/test_base_client_coverage.py @@ -173,6 +173,7 @@ def test_process_next_command_parsing(client): async def test_send_and_validate_success(client): client._connection = MagicMock() client._connection.write = MagicMock() + client._connected = True client._socket_lock = asyncio.Lock() validate_func = MagicMock(side_effect=[False, True]) # Fail first check, succeed second @@ -188,6 +189,7 @@ async def test_send_and_validate_success(client): @pytest.mark.asyncio async def test_send_and_validate_timeout(client): client._connection = MagicMock() + client._connected = True client._socket_lock = asyncio.Lock() client._retry_attempts = 1 client._command_retry_timeout = 0 # Immediate retry diff --git a/tests/test_client.py b/tests/test_client.py index 6c92730..d4368a8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -27,6 +27,8 @@ def htd_instance(): socket_timeout=MOCK_SOCKET_TIMEOUT, ) client._zone_data = {} + client._connection = MagicMock() + client._connected = True return client diff --git a/tests/test_lync_client_coverage.py b/tests/test_lync_client_coverage.py index dea3a01..ea3a849 100644 --- a/tests/test_lync_client_coverage.py +++ b/tests/test_lync_client_coverage.py @@ -18,6 +18,7 @@ def lync_client(): } client = HtdLyncClient(loop, model_info) client._connection = MagicMock() + client._connected = True client._socket_lock = asyncio.Lock() # Mock _zone_data client._zone_data = { diff --git a/tests/test_mca_client_coverage.py b/tests/test_mca_client_coverage.py index a3e4e01..dbeaab9 100644 --- a/tests/test_mca_client_coverage.py +++ b/tests/test_mca_client_coverage.py @@ -19,6 +19,7 @@ def mca_client(): } client = HtdMcaClient(loop, model_info) client._connection = MagicMock() + client._connected = True client._socket_lock = asyncio.Lock() # Mock _zone_data for all zones client._zone_data = { diff --git a/tests/test_offline_commands.py b/tests/test_offline_commands.py new file mode 100644 index 0000000..7c450d3 --- /dev/null +++ b/tests/test_offline_commands.py @@ -0,0 +1,109 @@ +"""Commands issued while the amplifier is off must fail with the library's own exception type. +The integration translates exactly one type into a visible Home Assistant error; an +AttributeError from a None transport surfaces as an opaque failure with no useful trace.""" +import asyncio +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from htd_client import build_client +from htd_client.constants import HtdConstants +from htd_client.exceptions import HtdConnectionError + + +def _mca(): + return build_client( + HtdConstants.SUPPORTED_MODELS["mca66"], + network_address=("1.2.3.4", 10006), + ) + + +def _lync(): + return build_client( + HtdConstants.SUPPORTED_MODELS["lync12"], + network_address=("1.2.3.4", 10006), + ) + + +@pytest.mark.asyncio +async def test_send_cmd_raises_a_connection_error_when_never_connected(): + """`self._connection.write(cmd)` on a None transport raises AttributeError, which the + integration cannot distinguish from a genuine bug in its own code.""" + client = _mca() + + with pytest.raises(HtdConnectionError): + await client._send_cmd(1, 0x04, 0x00) + + +@pytest.mark.asyncio +async def test_send_cmd_raises_a_connection_error_after_the_link_drops(): + """A transport can still be set after connection_lost; `connected` is the real signal.""" + client = _mca() + client._connection = MagicMock() + client._connected = False + + with pytest.raises(HtdConnectionError): + await client._send_cmd(1, 0x04, 0x00) + + +@pytest.mark.asyncio +async def test_mca_refresh_raises_a_connection_error_when_disconnected(): + """Entities call refresh() as they are added. Offline that must be a catchable no-op, + not a traceback during every startup.""" + client = _mca() + + with pytest.raises(HtdConnectionError): + await client.refresh() + + +@pytest.mark.asyncio +async def test_lync_refresh_raises_a_connection_error_when_disconnected(): + """Same contract on the other protocol dialect.""" + client = _lync() + + with pytest.raises(HtdConnectionError): + await client.refresh() + + +@pytest.mark.asyncio +async def test_heartbeat_stops_quietly_when_the_connection_drops(): + """The 60s heartbeat runs for the life of the process. If a disconnected refresh escapes + it, an outage produces 'Task exception was never retrieved' spam until the amp returns.""" + client = _mca() + client._connected = True + client._connection = None + + await client._heartbeat() + + assert client._connected is True + + +@pytest.mark.asyncio +async def test_connection_lost_broadcasts_so_entities_can_repaint(): + """Every platform sets should_poll = False, so a card only ever repaints on a broadcast. + Without this, a dropped connection leaves stale live values on screen indefinitely.""" + client = _mca() + seen = [] + await client.async_subscribe(lambda zone: seen.append(zone)) + + client.connection_lost(None) + await asyncio.sleep(0.01) + + assert seen == [None] + + +@pytest.mark.asyncio +async def test_connection_made_broadcasts_so_entities_can_come_back(): + """The transition out of an outage is the moment restored assumed-state values must be + replaced by live ones.""" + client = _mca() + seen = [] + await client.async_subscribe(lambda zone: seen.append(zone)) + + client.connection_made(MagicMock()) + await asyncio.sleep(0.01) + + assert seen == [None] + + if client._heartbeat_task is not None: + client._heartbeat_task.cancel()