Fix ONVIF config entry unique ID

homeassistant/components/onvif/base.py

@@ -2,6 +2,7 @@
 from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
 from homeassistant.helpers.entity import Entity
 
+from .const import DOMAIN
 from .device import ONVIFDevice
 from .models import Profile
 
@@ -11,8 +12,8 @@ class ONVIFBaseEntity(Entity):
 
     def __init__(self, device: ONVIFDevice, profile: Profile = None) -> None:
         """Initialize the ONVIF entity."""
-        self.device = device
-        self.profile = profile
+        self.device: ONVIFDevice = device
+        self.profile: Profile = profile
 
     @property
     def available(self):
@@ -22,10 +23,17 @@ def available(self):
     @property
     def device_info(self):
         """Return a device description for device registry."""
-        return {
-            "connections": {(CONNECTION_NETWORK_MAC, self.device.info.mac)},
+        device_info = {
+            "identifiers": {(DOMAIN, self.device.info.serial_number)},
             "manufacturer": self.device.info.manufacturer,
             "model": self.device.info.model,
             "name": self.device.name,
             "sw_version": self.device.info.fw_version,
         }
+
+        if self.device.info.mac:
+            device_info["connections"] = {
+                (CONNECTION_NETWORK_MAC, self.device.info.mac)
+            }
+
+        return device_info

homeassistant/components/onvif/camera.py

@@ -96,8 +96,8 @@ def name(self) -> str:
     def unique_id(self) -> str:
         """Return a unique ID."""
         if self.profile.index:
-            return f"{self.device.info.mac}_{self.profile.index}"
-        return self.device.info.mac
+            return f"{self.device.info.mac or self.device.info.serial_number}_{self.profile.index}"
+        return self.device.info.mac or self.device.info.serial_number
 
     @property
     def entity_registry_enabled_default(self) -> bool:

homeassistant/components/onvif/config_flow.py

@@ -172,7 +172,10 @@ async def async_step_auth(self, user_input=None):
         return self.async_show_form(
             step_id="auth",
             data_schema=vol.Schema(
-                {vol.Required(CONF_USERNAME): str, vol.Required(CONF_PASSWORD): str}
+                {
+                    vol.Required(CONF_USERNAME): str,
+                    vol.Optional(CONF_PASSWORD, default=""): str,
+                }
             ),
         )
 
@@ -195,15 +198,21 @@ async def async_step_profiles(self, user_input=None):
         await device.update_xaddrs()
 
         try:
+            devicemgmt = device.create_devicemgmt_service()
+
             # Get the MAC address to use as the unique ID for the config flow
             if not self.device_id:
-                devicemgmt = device.create_devicemgmt_service()
                 network_interfaces = await devicemgmt.GetNetworkInterfaces()
                 for interface in network_interfaces:
                     if interface.Enabled:
                         self.device_id = interface.Info.HwAddress
 
-            if self.device_id is None:
+            # If no network interfaces are exposed, fallback to serial number
+            if not self.device_id:
+                device_info = await devicemgmt.GetDeviceInformation()
+                self.device_id = device_info.SerialNumber
+
+            if not self.device_id:
                 return self.async_abort(reason="no_mac")
 
             await self.async_set_unique_id(self.device_id, raise_on_progress=False)

homeassistant/components/onvif/device.py

@@ -214,11 +214,20 @@ async def async_get_device_info(self) -> DeviceInfo:
         """Obtain information about this device."""
         devicemgmt = self.device.create_devicemgmt_service()
         device_info = await devicemgmt.GetDeviceInformation()
+
+        # Grab the last MAC address for backwards compatibility
+        mac = None
+        network_interfaces = await devicemgmt.GetNetworkInterfaces()
+        for interface in network_interfaces:
+            if interface.Enabled:
+                mac = interface.Info.HwAddress
+
         return DeviceInfo(
             device_info.Manufacturer,
             device_info.Model,
             device_info.FirmwareVersion,
-            self.config_entry.unique_id,
+            device_info.SerialNumber,
+            mac,
         )
 
     async def async_get_capabilities(self):

homeassistant/components/onvif/event.py

@@ -62,7 +62,8 @@ def remove_listener() -> None:
     @callback
     def async_remove_listener(self, update_callback: CALLBACK_TYPE) -> None:
         """Remove data update."""
-        self._listeners.remove(update_callback)
+        if update_callback in self._listeners:
+            self._listeners.remove(update_callback)
 
         if not self._listeners and self._unsub_refresh:
             self._unsub_refresh()
@@ -93,6 +94,8 @@ async def async_start(self) -> bool:
 
     async def async_stop(self) -> None:
         """Unsubscribe from events."""
+        self._listeners = []
+
         if not self._subscription:
             return
 

homeassistant/components/onvif/models.py

@@ -10,6 +10,7 @@ class DeviceInfo:
     manufacturer: str = None
     model: str = None
     fw_version: str = None
+    serial_number: str = None
     mac: str = None
 
 

tests/components/onvif/test_config_flow.py

@@ -17,6 +17,7 @@
 USERNAME = "admin"
 PASSWORD = "12345"
 MAC = "aa:bb:cc:dd:ee"
+SERIAL_NUMBER = "ABCDEFGHIJK"
 
 DISCOVERY = [
     {
@@ -37,11 +38,20 @@
 
 
 def setup_mock_onvif_camera(
-    mock_onvif_camera, with_h264=True, two_profiles=False, with_interfaces=True
+    mock_onvif_camera,
+    with_h264=True,
+    two_profiles=False,
+    with_interfaces=True,
+    with_serial=True,
 ):
     """Prepare mock onvif.ONVIFCamera."""
     devicemgmt = MagicMock()
 
+    device_info = MagicMock()
+    device_info.SerialNumber = SERIAL_NUMBER if with_serial else None
+    devicemgmt.GetDeviceInformation.return_value = Future()
+    devicemgmt.GetDeviceInformation.return_value.set_result(device_info)
+
     interface = MagicMock()
     interface.Enabled = True
     interface.Info.HwAddress = MAC
@@ -390,11 +400,48 @@ async def test_flow_manual_entry(hass):
 
 
 async def test_flow_import_no_mac(hass):
-    """Test that config flow fails when no MAC available."""
+    """Test that config flow uses Serial Number when no MAC available."""
     with patch(
         "homeassistant.components.onvif.config_flow.get_device"
-    ) as mock_onvif_camera:
+    ) as mock_onvif_camera, patch(
+        "homeassistant.components.onvif.ONVIFDevice"
+    ) as mock_device:
         setup_mock_onvif_camera(mock_onvif_camera, with_interfaces=False)
+        setup_mock_device(mock_device)
+
+        result = await hass.config_entries.flow.async_init(
+            config_flow.DOMAIN,
+            context={"source": config_entries.SOURCE_IMPORT},
+            data={
+                config_flow.CONF_NAME: NAME,
+                config_flow.CONF_HOST: HOST,
+                config_flow.CONF_PORT: PORT,
+                config_flow.CONF_USERNAME: USERNAME,
+                config_flow.CONF_PASSWORD: PASSWORD,
+            },
+        )
+
+        await hass.async_block_till_done()
+
+        assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
+        assert result["title"] == f"{NAME} - {SERIAL_NUMBER}"
+        assert result["data"] == {
+            config_flow.CONF_NAME: NAME,
+            config_flow.CONF_HOST: HOST,
+            config_flow.CONF_PORT: PORT,
+            config_flow.CONF_USERNAME: USERNAME,
+            config_flow.CONF_PASSWORD: PASSWORD,
+        }
+
+
+async def test_flow_import_no_mac_or_serial(hass):
+    """Test that config flow fails when no MAC or Serial Number available."""
+    with patch(
+        "homeassistant.components.onvif.config_flow.get_device"
+    ) as mock_onvif_camera:
+        setup_mock_onvif_camera(
+            mock_onvif_camera, with_interfaces=False, with_serial=False
+        )
 
         result = await hass.config_entries.flow.async_init(
             config_flow.DOMAIN,