Home-Assistant
/
core
mirror of https://github.com/home-assistant/core.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix ONVIF config entry unique ID (#36008) 

* fallback to device serial number if no mac available

* make password optional to fix #35904

* update tests to reflect new flow

* fix snake case and AsyncMock

* add comments around why weird things are being done
pull/36155/head
Jason Hunter 2020-05-24 15:50:50 -04:00 committed by Franck Nijhof
parent fad79046a8
commit 179e601966
No known key found for this signature in database
GPG Key ID: D62583BA8AB11CA3
7 changed files with 112 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
homeassistant/components/onvif/base.py
View File

@ -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,25 @@ class ONVIFBaseEntity(Entity):
@property
def device_info(self):
"""Return a device description for device registry."""
return {
"connections": {(CONNECTION_NETWORK_MAC, self.device.info.mac)},
device_info = {
"manufacturer": self.device.info.manufacturer,
"model": self.device.info.model,
"name": self.device.name,
"sw_version": self.device.info.fw_version,
}
# MAC address is not always available, and given the number
# of non-conformant ONVIF devices we have historically supported,
# we can not guarantee serial number either. Due to this, we have
# adopted an either/or approach in the config entry setup, and can
# guarantee that one or the other will be populated.
# See: https://github.com/home-assistant/core/issues/35883
if self.device.info.serial_number:
device_info["identifiers"] = {(DOMAIN, self.device.info.serial_number)}
if self.device.info.mac:
device_info["connections"] = {
(CONNECTION_NETWORK_MAC, self.device.info.mac)
}
return device_info

4
homeassistant/components/onvif/camera.py
View File

@ -96,8 +96,8 @@ class ONVIFCameraEntity(ONVIFBaseEntity, Camera):
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:

20
homeassistant/components/onvif/config_flow.py
View File

@ -169,10 +169,16 @@ class OnvifFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
self.onvif_config[CONF_PASSWORD] = user_input[CONF_PASSWORD]
return await self.async_step_profiles()
# Password is optional and default empty due to some cameras not
# allowing you to change ONVIF user settings.
# See https://github.com/home-assistant/core/issues/35904
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 +201,21 @@ class OnvifFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
await device.update_xaddrs()
try:
device_mgmt = 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()
network_interfaces = await device_mgmt.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 device_mgmt.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)

19
homeassistant/components/onvif/device.py
View File

@ -148,12 +148,12 @@ class ONVIFDevice:
async def async_check_date_and_time(self) -> None:
"""Warns if device and system date not synced."""
LOGGER.debug("Setting up the ONVIF device management service")
devicemgmt = self.device.create_devicemgmt_service()
device_mgmt = self.device.create_devicemgmt_service()
LOGGER.debug("Retrieving current device date/time")
try:
system_date = dt_util.utcnow()
device_time = await devicemgmt.GetSystemDateAndTime()
device_time = await device_mgmt.GetSystemDateAndTime()
if not device_time:
LOGGER.debug(
"""Couldn't get device '%s' date/time.
@ -212,13 +212,22 @@ class ONVIFDevice:
async def async_get_device_info(self) -> DeviceInfo:
"""Obtain information about this device."""
devicemgmt = self.device.create_devicemgmt_service()
device_info = await devicemgmt.GetDeviceInformation()
device_mgmt = self.device.create_devicemgmt_service()
device_info = await device_mgmt.GetDeviceInformation()
# Grab the last MAC address for backwards compatibility
mac = None
network_interfaces = await device_mgmt.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):

5
homeassistant/components/onvif/event.py
View File

@ -62,7 +62,8 @@ class EventManager:
@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 @@ class EventManager:
async def async_stop(self) -> None:
"""Unsubscribe from events."""
self._listeners = []
if not self._subscription:
return

1
homeassistant/components/onvif/models.py
View File

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

70
tests/components/onvif/test_config_flow.py
View File

@ -1,13 +1,11 @@
"""Test ONVIF config flow."""
from asyncio import Future
from asynctest import MagicMock, patch
from onvif.exceptions import ONVIFError
from zeep.exceptions import Fault
from homeassistant import config_entries, data_entry_flow
from homeassistant.components.onvif import config_flow
from tests.async_mock import AsyncMock, MagicMock, patch
from tests.common import MockConfigEntry
URN = "urn:uuid:123456789"
@ -17,6 +15,7 @@ PORT = 80
USERNAME = "admin"
PASSWORD = "12345"
MAC = "aa:bb:cc:dd:ee"
SERIAL_NUMBER = "ABCDEFGHIJK"
DISCOVERY = [
{
@ -37,18 +36,25 @@ DISCOVERY = [
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 = AsyncMock(return_value=device_info)
interface = MagicMock()
interface.Enabled = True
interface.Info.HwAddress = MAC
devicemgmt.GetNetworkInterfaces.return_value = Future()
devicemgmt.GetNetworkInterfaces.return_value.set_result(
[interface] if with_interfaces else []
devicemgmt.GetNetworkInterfaces = AsyncMock(
return_value=[interface] if with_interfaces else []
)
media_service = MagicMock()
@ -58,11 +64,9 @@ def setup_mock_onvif_camera(
profile2 = MagicMock()
profile2.VideoEncoderConfiguration.Encoding = "H264" if two_profiles else "MJPEG"
media_service.GetProfiles.return_value = Future()
media_service.GetProfiles.return_value.set_result([profile1, profile2])
media_service.GetProfiles = AsyncMock(return_value=[profile1, profile2])
mock_onvif_camera.update_xaddrs.return_value = Future()
mock_onvif_camera.update_xaddrs.return_value.set_result(True)
mock_onvif_camera.update_xaddrs = AsyncMock(return_value=True)
mock_onvif_camera.create_devicemgmt_service = MagicMock(return_value=devicemgmt)
mock_onvif_camera.create_media_service = MagicMock(return_value=media_service)
@ -116,8 +120,7 @@ def setup_mock_discovery(
def setup_mock_device(mock_device):
"""Prepare mock ONVIFDevice."""
mock_device.async_setup.return_value = Future()
mock_device.async_setup.return_value.set_result(True)
mock_device.async_setup = AsyncMock(return_value=True)
def mock_constructor(hass, config):
"""Fake the controller constructor."""
@ -390,11 +393,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, 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)
setup_mock_onvif_camera(
mock_onvif_camera, with_interfaces=False, with_serial=False
)
result = await hass.config_entries.flow.async_init(
config_flow.DOMAIN,