Repository: Jc2k/aiohomekit

Issue:
Invalid HomeKit Zeroconf record
Getting the following in home assistant logs...

022-11-02 17:33:30.752 DEBUG (MainThread) [aiohomekit.protocol.tlv] sending [
1 (Identifier): (2 bytes/<class 'bytes'>) b'\x01\x00'
2 (Salt): (1 bytes/<class 'bytes'>) b'\x01'
]
2022-11-02 17:33:33.767 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.767 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.767 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.768 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.768 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.768 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.768 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address
2022-11-02 17:33:33.769 DEBUG (MainThread) [aiohomekit.zeroconf] _hap._tcp.local.: Not a valid homekit device: Invalid HomeKit Zeroconf record: Missing address

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/Jc2k/aiohomekit/pull/233?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
Base: **66.98**% // Head: **66.93**% // Decreases project coverage by **`-0.05%`** :warning:
> Coverage data is based on head [(`639c7ba`)](https://codecov.io/gh/Jc2k/aiohomekit/pull/233?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) compared to base [(`0b84630`)](https://codecov.io/gh/Jc2k/aiohomekit/commit/0b846304e45bb9b8b6118abedddadac4e8d938ff?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> Patch coverage: 100.00% of modified lines in pull request are covered.

> :exclamation: Current head 639c7ba differs from pull request most recent head 639d2fa. Consider uploading reports for the commit 639d2fa to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #233      +/-   ##
==========================================
- Coverage   66.98%   66.93%   -0.06%     
==========================================
  Files          71       71              
  Lines        6585     6587       +2     
==========================================
- Hits         4411     4409       -2     
- Misses       2174     2178       +4     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `66.93% <100.00%> (-0.06%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/Jc2k/aiohomekit/pull/233?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [aiohomekit/zeroconf.py](https://codecov.io/gh/Jc2k/aiohomekit/pull/233/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-YWlvaG9tZWtpdC96ZXJvY29uZi5weQ==) | `73.71% <100.00%> (+0.30%)` | :arrow_up: |
| [aiohomekit/controller/ble/controller.py](https://codecov.io/gh/Jc2k/aiohomekit/pull/233/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-YWlvaG9tZWtpdC9jb250cm9sbGVyL2JsZS9jb250cm9sbGVyLnB5) | `33.58% <0.00%> (-2.99%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/Jc2k/aiohomekit/pull/233?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).

bdraco: I'm going to fix #234 as well before pushing a tag

Patch:
diff --git a/aiohomekit/zeroconf.py b/aiohomekit/zeroconf.py
index 51b6b48..030b99c 100644
--- a/aiohomekit/zeroconf.py
+++ b/aiohomekit/zeroconf.py
@@ -25,7 +25,7 @@ from ipaddress import ip_address
 import logging
 
 import async_timeout
-from zeroconf import ServiceListener, ServiceStateChange, Zeroconf
+from zeroconf import DNSPointer, ServiceListener, ServiceStateChange, Zeroconf
 from zeroconf.asyncio import AsyncServiceBrowser, AsyncServiceInfo, AsyncZeroconf
 
 from aiohomekit.characteristic_cache import CharacteristicCacheType
@@ -216,12 +216,16 @@ class ZeroconfController(AbstractController):
             self._async_zeroconf_instance, self.hap_type
         )
         self._browser.service_state_changed.register_handler(self._handle_service)
+        await self._async_update_from_cache(zc)
 
+        return self
+
+    async def _async_update_from_cache(self, zc: Zeroconf) -> None:
+        """Load the records from the cache."""
         infos = [
-            AsyncServiceInfo(self.hap_type, record.name)
-            for record in zc.cache.get_all_by_details(self.hap_type, TYPE_PTR, CLASS_IN)
+            AsyncServiceInfo(self.hap_type, record.alias)
+            for record in self._async_get_ptr_records(zc)
         ]
-
         tasks = []
         for info in infos:
             if info.load_from_cache(self._async_zeroconf_instance.zeroconf):
@@ -232,7 +236,9 @@ class ZeroconfController(AbstractController):
         if tasks:
             await asyncio.gather(*tasks)
 
-        return self
+    def _async_get_ptr_records(self, zc: Zeroconf) -> list[DNSPointer]:
+        """Return all PTR records for the HAP type."""
+        return zc.cache.async_all_by_details(self.hap_type, TYPE_PTR, CLASS_IN)
 
     def _handle_service(
         self,


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 7472d30..2fbb52f 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -10,7 +10,7 @@ from unittest import mock
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
-from zeroconf import SignalRegistrationInterface
+from zeroconf import DNSCache, SignalRegistrationInterface
 
 from aiohomekit import Controller
 from aiohomekit.controller.ip import IpPairing
@@ -70,6 +70,7 @@ def mock_asynczeroconf():
             zc.async_register_service = AsyncMock()
             zc.async_close = AsyncMock()
             zeroconf = MagicMock(name="zeroconf_mock")
+            zeroconf.cache = DNSCache()
             zeroconf.async_wait_for_start = AsyncMock()
             zeroconf.listeners = [AsyncServiceBrowserStub()]
             zc.zeroconf = zeroconf
diff --git a/tests/test_controller_ip_controller.py b/tests/test_controller_ip_controller.py
index ea8f477..385d397 100644
--- a/tests/test_controller_ip_controller.py
+++ b/tests/test_controller_ip_controller.py
@@ -1,10 +1,12 @@
 from collections.abc import Iterable
 import contextlib
 import socket
-from unittest.mock import MagicMock, patch
+from typing import Optional
+from unittest.mock import patch
 
 import pytest
-from zeroconf.asyncio import AsyncServiceInfo
+from zeroconf import DNSQuestionType, Zeroconf
+from zeroconf.asyncio import AsyncServiceInfo, AsyncZeroconf
 
 from aiohomekit.characteristic_cache import CharacteristicCacheMemory
 from aiohomekit.controller.ip.controller import IpController
@@ -12,9 +14,23 @@ from aiohomekit.exceptions import AccessoryNotFoundError
 from aiohomekit.model.categories import Categories
 from aiohomekit.model.status_flags import StatusFlags
 
+HAP_TYPE_TCP = "_hap._tcp.local."
+HAP_TYPE_UDP = "_hap._udp.local."
+CLASS_IN = 1
+TYPE_PTR = 12
 
-@contextlib.contextmanager
-def _install_mock_service_info(mock_asynczeroconf) -> Iterable[AsyncServiceInfo]:
+
+class MockedAsyncServiceInfo(AsyncServiceInfo):
+    async def async_request(
+        self,
+        zc: "Zeroconf",
+        timeout: float,
+        question_type: Optional[DNSQuestionType] = None,
+    ) -> bool:
+        return self.load_from_cache(zc)
+
+
+def _get_mock_service_info():
     desc = {
         b"c#": b"1",
         b"id": b"00:00:01:00:00:02",
@@ -23,10 +39,9 @@ def _install_mock_service_info(mock_asynczeroconf) -> Iterable[AsyncServiceInfo]
         b"ci": b"5",
         b"sf": b"0",
     }
-
-    info = AsyncServiceInfo(
-        "_hap._tcp.local.",
-        "foo._hap._tcp.local.",
+    return MockedAsyncServiceInfo(
+        HAP_TYPE_TCP,
+        f"foo.{HAP_TYPE_TCP}",
         addresses=[socket.inet_aton("127.0.0.1")],
         port=1234,
         properties=desc,
@@ -34,26 +49,33 @@ def _install_mock_service_info(mock_asynczeroconf) -> Iterable[AsyncServiceInfo]
         priority=0,
     )
 
-    mock_asynczeroconf.zeroconf.cache = MagicMock(
-        get_all_by_details=MagicMock(
-            return_value=[
-                MagicMock(alias="foo._hap._tcp.local."),
-            ]
-        )
+
+@contextlib.contextmanager
+def _install_mock_service_info(
+    mock_asynczeroconf: AsyncZeroconf, info: MockedAsyncServiceInfo
+) -> Iterable[AsyncServiceInfo]:
+    zeroconf: Zeroconf = mock_asynczeroconf.zeroconf
+    zeroconf.cache.async_add_records(
+        [*info.dns_addresses(), info.dns_pointer(), info.dns_service(), info.dns_text()]
+    )
+
+    assert (
+        zeroconf.cache.async_all_by_details(HAP_TYPE_TCP, TYPE_PTR, CLASS_IN)
+        is not None
     )
 
-    with patch("aiohomekit.zeroconf.AsyncServiceInfo", side_effect=[info]):
-        yield info
+    with patch("aiohomekit.zeroconf.AsyncServiceInfo", MockedAsyncServiceInfo):
+        yield
 
 
-async def test_discover_find_one(mock_asynczeroconf):
+async def test_discover_find_one(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
-
-    with _install_mock_service_info(mock_asynczeroconf):
+    with _install_mock_service_info(mock_asynczeroconf, _get_mock_service_info()):
         async with controller:
             result = await controller.async_find("00:00:01:00:00:02")
+        await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
 
     assert result.description.id == "00:00:01:00:00:02"
     assert result.description.category == Categories.LIGHTBULB
@@ -64,15 +86,18 @@ async def test_discover_find_one(mock_asynczeroconf):
     assert result.paired is True
 
 
-async def test_discover_find_one_unpaired(mock_asynczeroconf):
+async def test_discover_find_one_unpaired(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc:
-        svc.properties[b"sf"] = b"1"
+    svc = _get_mock_service_info()
+    svc.properties[b"sf"] = b"1"
+    svc._set_properties(svc.properties)
+    with _install_mock_service_info(mock_asynczeroconf, svc):
         async with controller:
             result = await controller.async_find("00:00:01:00:00:02")
+        await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
 
     assert result.description.id == "00:00:01:00:00:02"
     assert result.description.status_flags == StatusFlags.UNPAIRED
@@ -86,56 +111,73 @@ async def test_discover_find_none(mock_asynczeroconf):
 
     async with controller:
         with pytest.raises(AccessoryNotFoundError):
-            await controller.async_find("00:00:00:00:00:00")
+            await controller.async_find("00:00:00:00:00:00", timeout=0.001)
 
 
-async def test_find_device_id_case_lower(mock_asynczeroconf):
+async def test_find_device_id_case_lower(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        svc_info.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
+    svc_info_1 = _get_mock_service_info()
+    svc_info_1.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
+    svc_info_1._set_properties(svc_info_1.properties)
 
+    with _install_mock_service_info(mock_asynczeroconf, svc_info_1):
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             res = await controller.async_find("AA:AA:AA:AA:AA:AA")
             assert res.description.id == "aa:aa:aa:aa:aa:aa"
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        svc_info.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
+    svc_info_2 = _get_mock_service_info()
+    svc_info_2.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
+    svc_info_2._set_properties(svc_info_2.properties)
+
+    with _install_mock_service_info(mock_asynczeroconf, svc_info_2):
+        svc_info_2.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
 
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             res = await controller.async_find("aa:aa:aa:aa:aa:aa")
             assert res.description.id == "aa:aa:aa:aa:aa:aa"
 
 
-async def test_find_device_id_case_upper(mock_asynczeroconf):
+async def test_find_device_id_case_upper(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        svc_info.properties[b"id"] = b"AA:AA:aa:aa:AA:AA"
+    svc_info = _get_mock_service_info()
+    svc_info.properties[b"id"] = b"AA:AA:aa:aa:AA:AA"
+    svc_info._set_properties(svc_info.properties)
 
+    with _install_mock_service_info(mock_asynczeroconf, svc_info):
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             res = await controller.async_find("AA:AA:AA:AA:AA:AA")
             assert res.description.id == "aa:aa:aa:aa:aa:aa"
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        svc_info.properties[b"id"] = b"AA:AA:aa:aa:AA:AA"
+    svc_info = _get_mock_service_info()
+    svc_info.properties[b"id"] = b"AA:AA:aa:aa:AA:AA"
+    svc_info._set_properties(svc_info.properties)
+
+    with _install_mock_service_info(mock_asynczeroconf, svc_info):
 
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             res = await controller.async_find("aa:aa:aa:aa:aa:aa")
             assert res.description.id == "aa:aa:aa:aa:aa:aa"
 
 
-async def test_discover_discover_one(mock_asynczeroconf):
+async def test_discover_discover_one(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf):
+    srv_info = _get_mock_service_info()
+    with _install_mock_service_info(mock_asynczeroconf, srv_info):
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             results = [d async for d in controller.async_discover()]
 
     assert results[0].description.id == "00:00:01:00:00:02"
@@ -156,58 +198,75 @@ async def test_discover_none(mock_asynczeroconf):
     assert results == []
 
 
-async def test_discover_missing_csharp(mock_asynczeroconf):
+async def test_discover_missing_csharp(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        del svc_info.properties[b"c#"]
+    svc_info = _get_mock_service_info()
+    del svc_info.properties[b"c#"]
+    svc_info._set_properties(svc_info.properties)
+
+    with _install_mock_service_info(mock_asynczeroconf, svc_info):
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             results = [d async for d in controller.async_discover()]
 
     assert results[0].description.id == "00:00:01:00:00:02"
     assert results[0].description.config_num == 0
 
 
-async def test_discover_csharp_case(mock_asynczeroconf):
+async def test_discover_csharp_case(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        del svc_info.properties[b"c#"]
-        svc_info.properties[b"C#"] = b"1"
+    svc_info = _get_mock_service_info()
+    del svc_info.properties[b"c#"]
+    svc_info.properties[b"C#"] = b"1"
+    svc_info._set_properties(svc_info.properties)
 
+    with _install_mock_service_info(mock_asynczeroconf, svc_info):
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
             results = [d async for d in controller.async_discover()]
 
     assert results[0].description.config_num == 1
 
 
-async def test_discover_device_id_case_lower(mock_asynczeroconf):
+async def test_discover_device_id_case_lower(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        svc_info.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
+    svc_info = _get_mock_service_info()
+    svc_info.properties[b"id"] = b"aa:aa:aa:aa:aa:aa"
+    svc_info._set_properties(svc_info.properties)
+
+    with _install_mock_service_info(mock_asynczeroconf, svc_info):
 
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
+
             results = [d async for d in controller.async_discover()]
 
     assert results[0].description.id == "aa:aa:aa:aa:aa:aa"
 
 
-async def test_discover_device_id_case_upper(mock_asynczeroconf):
+async def test_discover_device_id_case_upper(mock_asynczeroconf: AsyncZeroconf):
     controller = IpController(
         char_cache=CharacteristicCacheMemory(), zeroconf_instance=mock_asynczeroconf
     )
 
-    with _install_mock_service_info(mock_asynczeroconf) as svc_info:
-        svc_info.properties[b"id"] = b"AA:AA:aa:aa:AA:AA"
+    svc_info = _get_mock_service_info()
+    svc_info.properties[b"id"] = b"AA:AA:aa:aa:AA:AA"
+    svc_info._set_properties(svc_info.properties)
+
+    with _install_mock_service_info(mock_asynczeroconf, svc_info):
 
         async with controller:
+            await controller._async_update_from_cache(mock_asynczeroconf.zeroconf)
+
             results = [d async for d in controller.async_discover()]
 
     assert results[0].description.id == "aa:aa:aa:aa:aa:aa"