Skip to content

Commit

Permalink
Avoid probing ipp printers for unique_id when it is available via mdns (
Browse files Browse the repository at this point in the history
#99982)

* Avoid probing ipp printers for unique_id when it is available via mdns

We would always probe the device in the ipp flow and than
abort if it was already configured. We avoid the probe for
most printers.

* dry

* coverage

* fix test

* add test for updating host
  • Loading branch information
bdraco authored and balloob committed Sep 12, 2023
1 parent f200ba7 commit 3d09e85
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 14 deletions.
36 changes: 24 additions & 12 deletions homeassistant/components/ipp/config_flow.py
Expand Up @@ -116,8 +116,7 @@ async def async_step_zeroconf(
name = discovery_info.name.replace(f".{zctype}", "")
tls = zctype == "_ipps._tcp.local."
base_path = discovery_info.properties.get("rp", "ipp/print")

self.context.update({"title_placeholders": {"name": name}})
unique_id = discovery_info.properties.get("UUID")

self.discovery_info.update(
{
Expand All @@ -127,10 +126,18 @@ async def async_step_zeroconf(
CONF_VERIFY_SSL: False,
CONF_BASE_PATH: f"/{base_path}",
CONF_NAME: name,
CONF_UUID: discovery_info.properties.get("UUID"),
CONF_UUID: unique_id,
}
)

if unique_id:
# If we already have the unique id, try to set it now
# so we can avoid probing the device if its already
# configured or ignored
await self._async_set_unique_id_and_abort_if_already_configured(unique_id)

self.context.update({"title_placeholders": {"name": name}})

try:
info = await validate_input(self.hass, self.discovery_info)
except IPPConnectionUpgradeRequired:
Expand All @@ -147,7 +154,6 @@ async def async_step_zeroconf(
_LOGGER.debug("IPP Error", exc_info=True)
return self.async_abort(reason="ipp_error")

unique_id = self.discovery_info[CONF_UUID]
if not unique_id and info[CONF_UUID]:
_LOGGER.debug(
"Printer UUID is missing from discovery info. Falling back to IPP UUID"
Expand All @@ -164,18 +170,24 @@ async def async_step_zeroconf(
"Unable to determine unique id from discovery info and IPP response"
)

if unique_id:
await self.async_set_unique_id(unique_id)
self._abort_if_unique_id_configured(
updates={
CONF_HOST: self.discovery_info[CONF_HOST],
CONF_NAME: self.discovery_info[CONF_NAME],
},
)
if unique_id and self.unique_id != unique_id:
await self._async_set_unique_id_and_abort_if_already_configured(unique_id)

await self._async_handle_discovery_without_unique_id()
return await self.async_step_zeroconf_confirm()

async def _async_set_unique_id_and_abort_if_already_configured(
self, unique_id: str
) -> None:
"""Set the unique ID and abort if already configured."""
await self.async_set_unique_id(unique_id)
self._abort_if_unique_id_configured(
updates={
CONF_HOST: self.discovery_info[CONF_HOST],
CONF_NAME: self.discovery_info[CONF_NAME],
},
)

async def async_step_zeroconf_confirm(
self, user_input: dict[str, Any] | None = None
) -> FlowResult:
Expand Down
35 changes: 35 additions & 0 deletions tests/components/ipp/fixtures/printer_without_uuid.json
@@ -0,0 +1,35 @@
{
"printer-state": "idle",
"printer-name": "Test Printer",
"printer-location": null,
"printer-make-and-model": "Test HA-1000 Series",
"printer-device-id": "MFG:TEST;CMD:ESCPL2,BDC,D4,D4PX,ESCPR7,END4,GENEP,URF;MDL:HA-1000 Series;CLS:PRINTER;DES:TEST HA-1000 Series;CID:EpsonRGB;FID:FXN,DPA,WFA,ETN,AFN,DAN,WRA;RID:20;DDS:022500;ELG:1000;SN:555534593035345555;URF:CP1,PQ4-5,OB9,OFU0,RS360,SRGB24,W8,DM3,IS1-7-6,V1.4,MT1-3-7-8-10-11-12;",
"printer-uri-supported": [
"ipps://192.168.1.31:631/ipp/print",
"ipp://192.168.1.31:631/ipp/print"
],
"uri-authentication-supported": ["none", "none"],
"uri-security-supported": ["tls", "none"],
"printer-info": "Test HA-1000 Series",
"printer-up-time": 30,
"printer-firmware-string-version": "20.23.06HA",
"printer-more-info": "http://192.168.1.31:80/PRESENTATION/BONJOUR",
"marker-names": [
"Black ink",
"Photo black ink",
"Cyan ink",
"Yellow ink",
"Magenta ink"
],
"marker-types": [
"ink-cartridge",
"ink-cartridge",
"ink-cartridge",
"ink-cartridge",
"ink-cartridge"
],
"marker-colors": ["#000000", "#000000", "#00FFFF", "#FFFF00", "#FF00FF"],
"marker-levels": [58, 98, 91, 95, 73],
"marker-low-levels": [10, 10, 10, 10, 10],
"marker-high-levels": [100, 100, 100, 100, 100]
}
103 changes: 101 additions & 2 deletions tests/components/ipp/test_config_flow.py
@@ -1,13 +1,15 @@
"""Tests for the IPP config flow."""
import dataclasses
from unittest.mock import MagicMock
import json
from unittest.mock import MagicMock, patch

from pyipp import (
IPPConnectionError,
IPPConnectionUpgradeRequired,
IPPError,
IPPParseError,
IPPVersionNotSupportedError,
Printer,
)
import pytest

Expand All @@ -23,7 +25,7 @@
MOCK_ZEROCONF_IPPS_SERVICE_INFO,
)

from tests.common import MockConfigEntry
from tests.common import MockConfigEntry, load_fixture

pytestmark = pytest.mark.usefixtures("mock_setup_entry")

Expand Down Expand Up @@ -316,6 +318,31 @@ async def test_zeroconf_with_uuid_device_exists_abort(
assert result["reason"] == "already_configured"


async def test_zeroconf_with_uuid_device_exists_abort_new_host(
hass: HomeAssistant,
mock_config_entry: MockConfigEntry,
mock_ipp_config_flow: MagicMock,
) -> None:
"""Test we abort zeroconf flow if printer already configured."""
mock_config_entry.add_to_hass(hass)

discovery_info = dataclasses.replace(MOCK_ZEROCONF_IPP_SERVICE_INFO, host="1.2.3.9")
discovery_info.properties = {
**MOCK_ZEROCONF_IPP_SERVICE_INFO.properties,
"UUID": "cfe92100-67c4-11d4-a45f-f8d027761251",
}

result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_ZEROCONF},
data=discovery_info,
)

assert result["type"] == FlowResultType.ABORT
assert result["reason"] == "already_configured"
assert mock_config_entry.data[CONF_HOST] == "1.2.3.9"


async def test_zeroconf_empty_unique_id(
hass: HomeAssistant,
mock_ipp_config_flow: MagicMock,
Expand All @@ -337,6 +364,21 @@ async def test_zeroconf_empty_unique_id(

assert result["type"] == FlowResultType.FORM

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
user_input={CONF_HOST: "192.168.1.31", CONF_BASE_PATH: "/ipp/print"},
)

assert result["type"] == FlowResultType.CREATE_ENTRY
assert result["title"] == "EPSON XP-6000 Series"

assert result["data"]
assert result["data"][CONF_HOST] == "192.168.1.31"
assert result["data"][CONF_UUID] == "cfe92100-67c4-11d4-a45f-f8d027761251"

assert result["result"]
assert result["result"].unique_id == "cfe92100-67c4-11d4-a45f-f8d027761251"


async def test_zeroconf_no_unique_id(
hass: HomeAssistant,
Expand All @@ -355,6 +397,21 @@ async def test_zeroconf_no_unique_id(

assert result["type"] == FlowResultType.FORM

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
user_input={CONF_HOST: "192.168.1.31", CONF_BASE_PATH: "/ipp/print"},
)

assert result["type"] == FlowResultType.CREATE_ENTRY
assert result["title"] == "EPSON XP-6000 Series"

assert result["data"]
assert result["data"][CONF_HOST] == "192.168.1.31"
assert result["data"][CONF_UUID] == "cfe92100-67c4-11d4-a45f-f8d027761251"

assert result["result"]
assert result["result"].unique_id == "cfe92100-67c4-11d4-a45f-f8d027761251"


async def test_full_user_flow_implementation(
hass: HomeAssistant,
Expand Down Expand Up @@ -448,3 +505,45 @@ async def test_full_zeroconf_tls_flow_implementation(

assert result["result"]
assert result["result"].unique_id == "cfe92100-67c4-11d4-a45f-f8d027761251"


async def test_zeroconf_empty_unique_id_uses_serial(hass: HomeAssistant) -> None:
"""Test zeroconf flow if printer lacks (empty) unique identification with serial fallback."""
fixture = await hass.async_add_executor_job(
load_fixture, "ipp/printer_without_uuid.json"
)
mock_printer_without_uuid = Printer.from_dict(json.loads(fixture))
mock_printer_without_uuid.unique_id = None

discovery_info = dataclasses.replace(MOCK_ZEROCONF_IPP_SERVICE_INFO)
discovery_info.properties = {
**MOCK_ZEROCONF_IPP_SERVICE_INFO.properties,
"UUID": "",
}
with patch(
"homeassistant.components.ipp.config_flow.IPP", autospec=True
) as ipp_mock:
client = ipp_mock.return_value
client.printer.return_value = mock_printer_without_uuid
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_ZEROCONF},
data=discovery_info,
)

assert result["type"] == FlowResultType.FORM

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
user_input={CONF_HOST: "192.168.1.31", CONF_BASE_PATH: "/ipp/print"},
)

assert result["type"] == FlowResultType.CREATE_ENTRY
assert result["title"] == "EPSON XP-6000 Series"

assert result["data"]
assert result["data"][CONF_HOST] == "192.168.1.31"
assert result["data"][CONF_UUID] == ""

assert result["result"]
assert result["result"].unique_id == "555534593035345555"

0 comments on commit 3d09e85

Please sign in to comment.