Improve androidtv mac address handling and test coverage (#65749)

* Better mac addr handling and improve test coverage

* Apply suggested changes

* Apply more suggested changes

homeassistant/components/androidtv/__init__.py

@@ -18,6 +18,7 @@ from homeassistant.const import (
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers import entity_registry as er
+from homeassistant.helpers.device_registry import format_mac
 from homeassistant.helpers.dispatcher import async_dispatcher_send
 from homeassistant.helpers.storage import STORAGE_DIR
 from homeassistant.helpers.typing import ConfigType
@@ -33,16 +34,30 @@ from .const import (
     DEVICE_ANDROIDTV,
     DEVICE_FIRETV,
     DOMAIN,
+    PROP_ETHMAC,
     PROP_SERIALNO,
+    PROP_WIFIMAC,
     SIGNAL_CONFIG_ENTITY,
 )
 
 PLATFORMS = [Platform.MEDIA_PLAYER]
 RELOAD_OPTIONS = [CONF_STATE_DETECTION_RULES]
 
+_INVALID_MACS = {"ff:ff:ff:ff:ff:ff"}
+
 _LOGGER = logging.getLogger(__name__)
 
 
+def get_androidtv_mac(dev_props):
+    """Return formatted mac from device properties."""
+    for prop_mac in (PROP_ETHMAC, PROP_WIFIMAC):
+        if if_mac := dev_props.get(prop_mac):
+            mac = format_mac(if_mac)
+            if mac not in _INVALID_MACS:
+                return mac
+    return None
+
+
 def _setup_androidtv(hass, config):
     """Generate an ADB key (if needed) and load it."""
     adbkey = config.get(CONF_ADBKEY, hass.config.path(STORAGE_DIR, "androidtv_adbkey"))

homeassistant/components/androidtv/config_flow.py

@@ -11,9 +11,8 @@ from homeassistant import config_entries
 from homeassistant.const import CONF_DEVICE_CLASS, CONF_HOST, CONF_NAME, CONF_PORT
 from homeassistant.core import callback
 from homeassistant.helpers import config_validation as cv
-from homeassistant.helpers.device_registry import format_mac
 
-from . import async_connect_androidtv
+from . import async_connect_androidtv, get_androidtv_mac
 from .const import (
     CONF_ADB_SERVER_IP,
     CONF_ADB_SERVER_PORT,
@@ -132,9 +131,7 @@ class AndroidTVFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
             PROP_WIFIMAC,
             dev_prop.get(PROP_WIFIMAC),
         )
-        unique_id = format_mac(
+        unique_id = get_androidtv_mac(dev_prop)
-            dev_prop.get(PROP_ETHMAC) or dev_prop.get(PROP_WIFIMAC, "")
-        )
         await aftv.adb_close()
         return None, unique_id
 

homeassistant/components/androidtv/media_player.py

@@ -51,12 +51,13 @@ from homeassistant.const import (
 )
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import config_validation as cv, entity_platform
-from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC, format_mac
+from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
 
+from . import get_androidtv_mac
 from .const import (
     ANDROID_DEV,
     ANDROID_DEV_OPT,
@@ -80,8 +81,6 @@ from .const import (
     DEVICE_ANDROIDTV,
     DEVICE_CLASSES,
     DOMAIN,
-    PROP_ETHMAC,
-    PROP_WIFIMAC,
     SIGNAL_CONFIG_ENTITY,
 )
 
@@ -343,7 +342,7 @@ class ADBDevice(MediaPlayerEntity):
             self._attr_device_info[ATTR_MANUFACTURER] = manufacturer
         if sw_version := info.get(ATTR_SW_VERSION):
             self._attr_device_info[ATTR_SW_VERSION] = sw_version
-        if mac := format_mac(info.get(PROP_ETHMAC) or info.get(PROP_WIFIMAC, "")):
+        if mac := get_androidtv_mac(info):
             self._attr_device_info[ATTR_CONNECTIONS] = {(CONNECTION_NETWORK_MAC, mac)}
 
         self._app_id_to_name = {}

tests/components/androidtv/patchers.py

@@ -1,13 +1,17 @@
 """Define patches used for androidtv tests."""
-
 from unittest.mock import mock_open, patch
 
+from androidtv.constants import CMD_DEVICE_PROPERTIES, CMD_MAC_ETH0, CMD_MAC_WLAN0
+
 KEY_PYTHON = "python"
 KEY_SERVER = "server"
 
 ADB_DEVICE_TCP_ASYNC_FAKE = "AdbDeviceTcpAsyncFake"
 DEVICE_ASYNC_FAKE = "DeviceAsyncFake"
 
+PROPS_DEV_INFO = "fake\nfake\n0123456\nfake"
+PROPS_DEV_MAC = "ether ab:cd:ef:gh:ij:kl brd"
+
 
 class AdbDeviceTcpAsyncFake:
     """A fake of the `adb_shell.adb_device_async.AdbDeviceTcpAsync` class."""
@@ -100,12 +104,18 @@ def patch_connect(success):
     }
 
 
-def patch_shell(response=None, error=False):
+def patch_shell(response=None, error=False, mac_eth=False):
     """Mock the `AdbDeviceTcpAsyncFake.shell` and `DeviceAsyncFake.shell` methods."""
 
     async def shell_success(self, cmd, *args, **kwargs):
         """Mock the `AdbDeviceTcpAsyncFake.shell` and `DeviceAsyncFake.shell` methods when they are successful."""
         self.shell_cmd = cmd
+        if cmd == CMD_DEVICE_PROPERTIES:
+            return PROPS_DEV_INFO
+        if cmd == CMD_MAC_WLAN0:
+            return PROPS_DEV_MAC
+        if cmd == CMD_MAC_ETH0:
+            return PROPS_DEV_MAC if mac_eth else None
         return response
 
     async def shell_fail_python(self, cmd, *args, **kwargs):
@@ -185,15 +195,3 @@ PATCH_ANDROIDTV_UPDATE_EXCEPTION = patch(
     "androidtv.androidtv.androidtv_async.AndroidTVAsync.update",
     side_effect=ZeroDivisionError,
 )
-
-PATCH_DEVICE_PROPERTIES = patch(
-    "androidtv.basetv.basetv_async.BaseTVAsync.get_device_properties",
-    return_value={
-        "manufacturer": "a",
-        "model": "b",
-        "serialno": "c",
-        "sw_version": "d",
-        "wifimac": "ab:cd:ef:gh:ij:kl",
-        "ethmac": None,
-    },
-)

tests/components/androidtv/test_config_flow.py

@@ -31,6 +31,7 @@ from homeassistant.components.androidtv.const import (
     DEFAULT_PORT,
     DOMAIN,
     PROP_ETHMAC,
+    PROP_WIFIMAC,
 )
 from homeassistant.components.media_player import DOMAIN as MP_DOMAIN
 from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER
@@ -42,6 +43,7 @@ from tests.components.androidtv.patchers import isfile
 
 ADBKEY = "adbkey"
 ETH_MAC = "a1:b1:c1:d1:e1:f1"
+WIFI_MAC = "a2:b2:c2:d2:e2:f2"
 HOST = "127.0.0.1"
 VALID_DETECT_RULE = [{"paused": {"media_session_state": 3}}]
 
@@ -84,18 +86,28 @@ PATCH_SETUP_ENTRY = patch(
 class MockConfigDevice:
     """Mock class to emulate Android TV device."""
 
-    def __init__(self, eth_mac=ETH_MAC):
+    def __init__(self, eth_mac=ETH_MAC, wifi_mac=None):
         """Initialize a fake device to test config flow."""
         self.available = True
-        self.device_properties = {PROP_ETHMAC: eth_mac}
+        self.device_properties = {PROP_ETHMAC: eth_mac, PROP_WIFIMAC: wifi_mac}
 
     async def adb_close(self):
         """Fake method to close connection."""
         self.available = False
 
 
-@pytest.mark.parametrize("config", [CONFIG_PYTHON_ADB, CONFIG_ADB_SERVER])
+@pytest.mark.parametrize(
-async def test_user(hass, config):
+    ["config", "eth_mac", "wifi_mac"],
+    [
+        (CONFIG_PYTHON_ADB, ETH_MAC, None),
+        (CONFIG_ADB_SERVER, ETH_MAC, None),
+        (CONFIG_PYTHON_ADB, None, WIFI_MAC),
+        (CONFIG_ADB_SERVER, None, WIFI_MAC),
+        (CONFIG_PYTHON_ADB, ETH_MAC, WIFI_MAC),
+        (CONFIG_ADB_SERVER, ETH_MAC, WIFI_MAC),
+    ],
+)
+async def test_user(hass, config, eth_mac, wifi_mac):
     """Test user config."""
     flow_result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": SOURCE_USER, "show_advanced_options": True}
@@ -106,7 +118,7 @@ async def test_user(hass, config):
     # test with all provided
     with patch(
         CONNECT_METHOD,
-        return_value=(MockConfigDevice(), None),
+        return_value=(MockConfigDevice(eth_mac, wifi_mac), None),
     ), PATCH_SETUP_ENTRY as mock_setup_entry, PATCH_GET_HOST_IP:
         result = await hass.config_entries.flow.async_configure(
             flow_result["flow_id"], user_input=config
@@ -273,7 +285,7 @@ async def test_invalid_serial(hass):
     """Test for invalid serialno."""
     with patch(
         CONNECT_METHOD,
-        return_value=(MockConfigDevice(eth_mac=""), None),
+        return_value=(MockConfigDevice(eth_mac=None), None),
     ), PATCH_GET_HOST_IP:
         result = await hass.config_entries.flow.async_init(
             DOMAIN,

tests/components/androidtv/test_media_player.py

@@ -142,29 +142,6 @@ def _setup(config):
     return patch_key, entity_id, config_entry
 
 
-async def test_setup_with_properties(hass):
-    """Test that setup succeeds with device properties.
-
-    the response must be a string with the following info separated with line break:
-    "manufacturer, model, serialno, version, mac_wlan0_output, mac_eth0_output"
-
-    """
-
-    patch_key, entity_id, config_entry = _setup(CONFIG_ANDROIDTV_ADB_SERVER)
-    config_entry.add_to_hass(hass)
-    response = "fake\nfake\n0123456\nfake\nether a1:b1:c1:d1:e1:f1 brd\nnone"
-
-    with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
-        patch_key
-    ], patchers.patch_shell(response)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
-            assert await hass.config_entries.async_setup(config_entry.entry_id)
-            await hass.async_block_till_done()
-
-        state = hass.states.get(entity_id)
-        assert state is not None
-
-
 @pytest.mark.parametrize(
     "config",
     [
@@ -190,7 +167,6 @@ async def test_reconnect(hass, caplog, config):
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[
         patch_key
     ], patchers.PATCH_KEYGEN, patchers.PATCH_ANDROIDTV_OPEN, patchers.PATCH_SIGNER:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -259,7 +235,6 @@ async def test_adb_shell_returns_none(hass, config):
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[
         patch_key
     ], patchers.PATCH_KEYGEN, patchers.PATCH_ANDROIDTV_OPEN, patchers.PATCH_SIGNER:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -289,7 +264,6 @@ async def test_setup_with_adbkey(hass):
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[
         patch_key
     ], patchers.PATCH_ANDROIDTV_OPEN, patchers.PATCH_SIGNER, PATCH_ISFILE, PATCH_ACCESS:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -324,7 +298,6 @@ async def test_sources(hass, config0):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -404,7 +377,6 @@ async def _test_exclude_sources(hass, config0, expected_sources):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -486,7 +458,6 @@ async def _test_select_source(hass, config0, source, expected_arg, method_patch)
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -714,7 +685,6 @@ async def test_setup_fail(hass, config):
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[
         patch_key
     ], patchers.PATCH_KEYGEN, patchers.PATCH_ANDROIDTV_OPEN, patchers.PATCH_SIGNER:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id) is False
         await hass.async_block_till_done()
 
@@ -733,7 +703,6 @@ async def test_adb_command(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -763,7 +732,6 @@ async def test_adb_command_unicode_decode_error(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -793,7 +761,6 @@ async def test_adb_command_key(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -823,7 +790,6 @@ async def test_adb_command_get_properties(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -853,7 +819,6 @@ async def test_learn_sendevent(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -882,7 +847,6 @@ async def test_update_lock_not_acquired(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -918,7 +882,6 @@ async def test_download(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -965,7 +928,6 @@ async def test_upload(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -1010,7 +972,6 @@ async def test_androidtv_volume_set(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -1038,7 +999,6 @@ async def test_get_image(hass, hass_ws_client):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -1115,7 +1075,6 @@ async def test_services_androidtv(hass):
 
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[patch_key]:
         with patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-            with patchers.PATCH_DEVICE_PROPERTIES:
             assert await hass.config_entries.async_setup(config_entry.entry_id)
             await hass.async_block_till_done()
 
@@ -1162,7 +1121,6 @@ async def test_services_firetv(hass):
 
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[patch_key]:
         with patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-            with patchers.PATCH_DEVICE_PROPERTIES:
             assert await hass.config_entries.async_setup(config_entry.entry_id)
             await hass.async_block_till_done()
 
@@ -1179,7 +1137,6 @@ async def test_volume_mute(hass):
 
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[patch_key]:
         with patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-            with patchers.PATCH_DEVICE_PROPERTIES:
             assert await hass.config_entries.async_setup(config_entry.entry_id)
             await hass.async_block_till_done()
 
@@ -1224,7 +1181,6 @@ async def test_connection_closed_on_ha_stop(hass):
     with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[
         patch_key
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[patch_key]:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
@@ -1249,7 +1205,6 @@ async def test_exception(hass):
     ], patchers.patch_shell(SHELL_RESPONSE_OFF)[
         patch_key
     ], patchers.PATCH_KEYGEN, patchers.PATCH_ANDROIDTV_OPEN, patchers.PATCH_SIGNER:
-        with patchers.PATCH_DEVICE_PROPERTIES:
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()