From eafeba792d3c3a101f06466776beefce2794975c Mon Sep 17 00:00:00 2001 From: Shay Levy Date: Sat, 31 Jan 2026 16:33:31 +0200 Subject: [PATCH] Fix Shelly CoIoT repair issue (#161973) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- homeassistant/components/shelly/__init__.py | 2 - .../components/shelly/coordinator.py | 25 +++--- homeassistant/components/shelly/repairs.py | 52 +---------- homeassistant/components/shelly/utils.py | 86 ++++++++++++++++++ tests/components/shelly/__init__.py | 11 +++ tests/components/shelly/conftest.py | 6 +- tests/components/shelly/test_coordinator.py | 12 +-- tests/components/shelly/test_diagnostics.py | 8 +- tests/components/shelly/test_repairs.py | 88 +++++++++++++------ 9 files changed, 184 insertions(+), 106 deletions(-) diff --git a/homeassistant/components/shelly/__init__.py b/homeassistant/components/shelly/__init__.py index 1b6a5c770de..8e4480f92d7 100644 --- a/homeassistant/components/shelly/__init__.py +++ b/homeassistant/components/shelly/__init__.py @@ -59,7 +59,6 @@ from .coordinator import ( ) from .repairs import ( async_manage_ble_scanner_firmware_unsupported_issue, - async_manage_coiot_unconfigured_issue, async_manage_deprecated_firmware_issue, async_manage_open_wifi_ap_issue, async_manage_outbound_websocket_incorrectly_enabled_issue, @@ -233,7 +232,6 @@ async def _async_setup_block_entry( await hass.config_entries.async_forward_entry_setups( entry, runtime_data.platforms ) - await async_manage_coiot_unconfigured_issue(hass, entry) remove_empty_sub_devices(hass, entry) elif ( sleep_period is None diff --git a/homeassistant/components/shelly/coordinator.py b/homeassistant/components/shelly/coordinator.py index a8cdc86efc1..2a832c4dba4 100644 --- a/homeassistant/components/shelly/coordinator.py +++ b/homeassistant/components/shelly/coordinator.py @@ -47,6 +47,7 @@ from .const import ( ATTR_DEVICE, ATTR_GENERATION, BATTERY_DEVICES_WITH_PERMANENT_CONNECTION, + COIOT_UNCONFIGURED_ISSUE_ID, CONF_BLE_SCANNER_MODE, CONF_SLEEP_PERIOD, DOMAIN, @@ -72,6 +73,7 @@ from .const import ( ) from .utils import ( async_create_issue_unsupported_firmware, + async_manage_coiot_issues_task, get_block_device_sleep_period, get_device_entry_gen, get_host, @@ -442,26 +444,19 @@ class ShellyBlockCoordinator(ShellyCoordinatorBase[BlockDevice]): DOMAIN, PUSH_UPDATE_ISSUE_ID.format(unique=self.mac), ) + ir.async_delete_issue( + self.hass, + DOMAIN, + COIOT_UNCONFIGURED_ISSUE_ID.format(unique=self.mac), + ) self._push_update_failures = 0 elif update_type is BlockUpdateType.COAP_REPLY: self._push_update_failures += 1 if self._push_update_failures == MAX_PUSH_UPDATE_FAILURES: - LOGGER.debug( - "Creating issue %s", PUSH_UPDATE_ISSUE_ID.format(unique=self.mac) - ) - ir.async_create_issue( + self.config_entry.async_create_background_task( self.hass, - DOMAIN, - PUSH_UPDATE_ISSUE_ID.format(unique=self.mac), - is_fixable=False, - is_persistent=False, - severity=ir.IssueSeverity.ERROR, - learn_more_url="https://www.home-assistant.io/integrations/shelly/#shelly-device-configuration-generation-1", - translation_key="push_update_failure", - translation_placeholders={ - "device_name": self.config_entry.title, - "ip_address": self.device.ip_address, - }, + async_manage_coiot_issues_task(self.hass, self.config_entry), + "coiot_issues", ) if self._push_update_failures: LOGGER.debug( diff --git a/homeassistant/components/shelly/repairs.py b/homeassistant/components/shelly/repairs.py index d78c4f0b2cb..3d6bcc493a9 100644 --- a/homeassistant/components/shelly/repairs.py +++ b/homeassistant/components/shelly/repairs.py @@ -5,12 +5,7 @@ from __future__ import annotations from typing import TYPE_CHECKING from aioshelly.block_device import BlockDevice -from aioshelly.const import ( - MODEL_OUT_PLUG_S_G3, - MODEL_PLUG, - MODEL_PLUG_S_G3, - RPC_GENERATIONS, -) +from aioshelly.const import MODEL_OUT_PLUG_S_G3, MODEL_PLUG_S_G3, RPC_GENERATIONS from aioshelly.exceptions import DeviceConnectionError, RpcCallError from aioshelly.rpc_device import RpcDevice from awesomeversion import AwesomeVersion @@ -24,7 +19,6 @@ from homeassistant.helpers import issue_registry as ir from .const import ( BLE_SCANNER_FIRMWARE_UNSUPPORTED_ISSUE_ID, BLE_SCANNER_MIN_FIRMWARE, - COIOT_UNCONFIGURED_ISSUE_ID, CONF_BLE_SCANNER_MODE, DEPRECATED_FIRMWARE_ISSUE_ID, DEPRECATED_FIRMWARES, @@ -162,50 +156,6 @@ def async_manage_outbound_websocket_incorrectly_enabled_issue( ir.async_delete_issue(hass, DOMAIN, issue_id) -async def async_manage_coiot_unconfigured_issue( - hass: HomeAssistant, - entry: ShellyConfigEntry, -) -> None: - """Manage the CoIoT unconfigured issue.""" - issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=entry.unique_id) - - if TYPE_CHECKING: - assert entry.runtime_data.block is not None - - device = entry.runtime_data.block.device - - if device.model == MODEL_PLUG: - # Shelly Plug Gen 1 does not have CoIoT settings - ir.async_delete_issue(hass, DOMAIN, issue_id) - return - - coiot_config = device.settings["coiot"] - coiot_enabled = coiot_config.get("enabled") - - coiot_peer = f"{await get_coiot_address(hass)}:{get_coiot_port(hass)}" - # Check if CoIoT is disabled or peer address is not correctly set - if not coiot_enabled or ( - (peer_config := coiot_config.get("peer")) and peer_config != coiot_peer - ): - ir.async_create_issue( - hass, - DOMAIN, - issue_id, - is_fixable=True, - is_persistent=False, - severity=ir.IssueSeverity.WARNING, - translation_key="coiot_unconfigured", - translation_placeholders={ - "device_name": device.name, - "ip_address": device.ip_address, - }, - data={"entry_id": entry.entry_id}, - ) - return - - ir.async_delete_issue(hass, DOMAIN, issue_id) - - @callback def async_manage_open_wifi_ap_issue( hass: HomeAssistant, diff --git a/homeassistant/components/shelly/utils.py b/homeassistant/components/shelly/utils.py index cced36502c0..b7da839e6fc 100644 --- a/homeassistant/components/shelly/utils.py +++ b/homeassistant/components/shelly/utils.py @@ -22,6 +22,7 @@ from aioshelly.const import ( MODEL_EM3, MODEL_I3, MODEL_NAMES, + MODEL_PLUG, RPC_GENERATIONS, ) from aioshelly.rpc_device import RpcDevice, WsServer @@ -55,6 +56,7 @@ from homeassistant.util.dt import utcnow from .const import ( API_WS_URL, BASIC_INPUTS_EVENTS_TYPES, + COIOT_UNCONFIGURED_ISSUE_ID, COMPONENT_ID_PATTERN, CONF_COAP_PORT, CONF_GEN, @@ -67,6 +69,7 @@ from .const import ( GEN2_RELEASE_URL, LOGGER, MAX_SCRIPT_SIZE, + PUSH_UPDATE_ISSUE_ID, ROLE_GENERIC, RPC_INPUTS_EVENTS_TYPES, SHAIR_MAX_WORK_HOURS, @@ -1004,3 +1007,86 @@ def is_rpc_ble_scanner_supported(entry: ConfigEntry) -> bool: entry.runtime_data.rpc_supports_scripts and not entry.runtime_data.rpc_zigbee_firmware ) + + +async def check_coiot_config(device: BlockDevice, hass: HomeAssistant) -> bool: + """Check if CoIoT is correctly configured.""" + if device.model == MODEL_PLUG: + # Shelly Plug Gen 1 does not have CoIoT settings + return True + + coiot_config = device.settings["coiot"] + + # Check if CoIoT is disabled + if not coiot_config.get("enabled"): + return False + + coiot_address = await get_coiot_address(hass) + if coiot_address is None: + LOGGER.debug( + "Skipping CoIoT peer check for device %s as no local address is available", + device.name, + ) + return True + + coiot_peer = f"{coiot_address}:{get_coiot_port(hass)}" + # Check if CoIoT address is not correctly set + if (peer_config := coiot_config.get("peer")) and peer_config != coiot_peer: + LOGGER.debug( + "CoIoT is unconfigured for device %s, peer_config: %s, coiot_peer: %s", + device.name, + peer_config, + coiot_peer, + ) + return False + + return True + + +async def async_manage_coiot_issues_task( + hass: HomeAssistant, entry: ConfigEntry +) -> None: + """CoIoT configuration or push updates issues task.""" + config_issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=entry.unique_id) + push_updates_issue_id = PUSH_UPDATE_ISSUE_ID.format(unique=entry.unique_id) + + if TYPE_CHECKING: + assert entry.runtime_data.block is not None + + device = entry.runtime_data.block.device + + if await check_coiot_config(device, hass): + # CoIoT is correctly configured, create push updates issue + ir.async_delete_issue(hass, DOMAIN, config_issue_id) + ir.async_create_issue( + hass, + DOMAIN, + push_updates_issue_id, + is_fixable=False, + is_persistent=False, + severity=ir.IssueSeverity.ERROR, + learn_more_url="https://www.home-assistant.io/integrations/shelly/#shelly-device-configuration-generation-1", + translation_key="push_update_failure", + translation_placeholders={ + "device_name": device.name, + "ip_address": device.ip_address, + }, + ) + return + + # CoIoT is not correctly configured, create config issue + ir.async_delete_issue(hass, DOMAIN, push_updates_issue_id) + ir.async_create_issue( + hass, + DOMAIN, + config_issue_id, + is_fixable=True, + is_persistent=False, + severity=ir.IssueSeverity.WARNING, + translation_key="coiot_unconfigured", + translation_placeholders={ + "device_name": device.name, + "ip_address": device.ip_address, + }, + data={"entry_id": entry.entry_id}, + ) diff --git a/tests/components/shelly/__init__.py b/tests/components/shelly/__init__.py index 8eaffe5af86..8ded53a6f58 100644 --- a/tests/components/shelly/__init__.py +++ b/tests/components/shelly/__init__.py @@ -22,6 +22,7 @@ from homeassistant.components.shelly.const import ( CONF_GEN, CONF_SLEEP_PERIOD, DOMAIN, + MAX_PUSH_UPDATE_FAILURES, REST_SENSORS_UPDATE_INTERVAL, RPC_SENSORS_POLLING_INTERVAL, ) @@ -71,6 +72,16 @@ async def init_integration( return entry +async def mock_block_device_push_update_failure( + hass: HomeAssistant, mock_block_device: Mock +) -> None: + """Create updates with COAP_REPLY indicating push update failure for block device.""" + for _ in range(MAX_PUSH_UPDATE_FAILURES): + mock_block_device.mock_update_reply() + await hass.async_block_till_done() + await hass.async_block_till_done(wait_background_tasks=True) + + def mutate_rpc_device_status( monkeypatch: pytest.MonkeyPatch, mock_rpc_device: Mock, diff --git a/tests/components/shelly/conftest.py b/tests/components/shelly/conftest.py index 9fbdfe375f9..f3a583035e7 100644 --- a/tests/components/shelly/conftest.py +++ b/tests/components/shelly/conftest.py @@ -39,7 +39,11 @@ MOCK_SETTINGS = { "num_inputs": 3, "num_outputs": 2, }, - "coiot": {"update_period": 15}, + "coiot": { + "update_period": 15, + "enabled": True, + "peer": "10.10.10.10:5683", + }, "fw": "20201124-092159/v1.9.0@57ac4ad8", "inputs": [ { diff --git a/tests/components/shelly/test_coordinator.py b/tests/components/shelly/test_coordinator.py index 83448e20a25..1fe881a3464 100644 --- a/tests/components/shelly/test_coordinator.py +++ b/tests/components/shelly/test_coordinator.py @@ -20,7 +20,6 @@ from homeassistant.components.shelly.const import ( CONF_SLEEP_PERIOD, DOMAIN, ENTRY_RELOAD_COOLDOWN, - MAX_PUSH_UPDATE_FAILURES, RPC_RECONNECT_INTERVAL, UPDATE_PERIOD_MULTIPLIER, BLEScannerMode, @@ -36,6 +35,7 @@ from . import ( MOCK_MAC, init_integration, inject_rpc_device_event, + mock_block_device_push_update_failure, mock_polling_rpc_update, mock_rest_update, register_device, @@ -332,15 +332,7 @@ async def test_block_device_push_updates_failure( ) -> None: """Test block device with push updates failure.""" await init_integration(hass, 1) - - # Updates with COAP_REPLAY type should create an issue - for _ in range(MAX_PUSH_UPDATE_FAILURES): - mock_block_device.mock_update_reply() - await hass.async_block_till_done() - - assert issue_registry.async_get_issue( - domain=DOMAIN, issue_id=f"push_update_{MOCK_MAC}" - ) + await mock_block_device_push_update_failure(hass, mock_block_device) # An update with COAP_PERIODIC type should clear the issue mock_block_device.mock_update() diff --git a/tests/components/shelly/test_diagnostics.py b/tests/components/shelly/test_diagnostics.py index 6bd44fa036a..cba5a986cc5 100644 --- a/tests/components/shelly/test_diagnostics.py +++ b/tests/components/shelly/test_diagnostics.py @@ -52,7 +52,13 @@ async def test_block_config_entry_diagnostics( "model": MODEL_25, "sw_version": "some fw string", }, - "device_settings": {"coiot": {"update_period": 15}}, + "device_settings": { + "coiot": { + "update_period": 15, + "enabled": True, + "peer": "10.10.10.10:5683", + } + }, "device_status": MOCK_STATUS_COAP, "last_error": "DeviceConnectionError()", } diff --git a/tests/components/shelly/test_repairs.py b/tests/components/shelly/test_repairs.py index 0d407b36d7b..d95b1846f3e 100644 --- a/tests/components/shelly/test_repairs.py +++ b/tests/components/shelly/test_repairs.py @@ -1,5 +1,6 @@ """Test repairs handling for Shelly.""" +from typing import Any from unittest.mock import Mock, patch from aioshelly.const import MODEL_PLUG, MODEL_WALL_DISPLAY @@ -14,6 +15,7 @@ from homeassistant.components.shelly.const import ( DOMAIN, OPEN_WIFI_AP_ISSUE_ID, OUTBOUND_WEBSOCKET_INCORRECTLY_ENABLED_ISSUE_ID, + PUSH_UPDATE_ISSUE_ID, BLEScannerMode, DeprecatedFirmwareInfo, ) @@ -22,7 +24,7 @@ from homeassistant.helpers import issue_registry as ir from homeassistant.helpers.network import NoURLAvailableError from homeassistant.setup import async_setup_component -from . import MOCK_MAC, init_integration +from . import MOCK_MAC, init_integration, mock_block_device_push_update_failure from tests.components.repairs import ( async_process_repairs_platforms, @@ -505,23 +507,28 @@ async def test_other_fixable_issues( assert result["type"] == "create_entry" -async def test_coiot_missing_or_wrong_peer_issue( +@pytest.mark.parametrize( + "coiot", + [ + {"enabled": False, "update_period": 15, "peer": "10.10.10.10:5683"}, + {"enabled": True, "update_period": 15, "peer": "7.7.7.7:5683"}, + ], +) +async def test_coiot_disabled_or_wrong_peer_issue( hass: HomeAssistant, hass_client: ClientSessionGenerator, mock_block_device: Mock, issue_registry: ir.IssueRegistry, monkeypatch: pytest.MonkeyPatch, + coiot: dict[str, Any], ) -> None: - """Test repair issues handling wrong or missing CoIoT configuration.""" - monkeypatch.setitem( - mock_block_device.settings, - "coiot", - {"enabled": False, "update_period": 15, "peer": "wrong.peer.address:5683"}, - ) + """Test repair issues handling wrong or disabled CoIoT configuration.""" + monkeypatch.setitem(mock_block_device.settings, "coiot", coiot) issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=MOCK_MAC) assert await async_setup_component(hass, "repairs", {}) await hass.async_block_till_done() await init_integration(hass, 1) + await mock_block_device_push_update_failure(hass, mock_block_device) assert issue_registry.async_get_issue(DOMAIN, issue_id) assert len(issue_registry.issues) == 1 @@ -551,16 +558,17 @@ async def test_coiot_exception( issue_registry: ir.IssueRegistry, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Test no repair issues handling up-to-date CoIoT configuration.""" + """Test CoIoT exception handling in fix flow.""" monkeypatch.setitem( mock_block_device.settings, "coiot", - {"enabled": True, "update_period": 15, "peer": "correct.peer.address:5683"}, + {"enabled": False, "update_period": 15, "peer": "7.7.7.7:5683"}, ) issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=MOCK_MAC) assert await async_setup_component(hass, "repairs", {}) await hass.async_block_till_done() await init_integration(hass, 1) + await mock_block_device_push_update_failure(hass, mock_block_device) assert issue_registry.async_get_issue(DOMAIN, issue_id) assert len(issue_registry.issues) == 1 @@ -598,12 +606,7 @@ async def test_coiot_configured_no_issue_created( monkeypatch: pytest.MonkeyPatch, raw_url: str, ) -> None: - """Test no repair issues when CoIoT configuration is missing.""" - monkeypatch.setitem( - mock_block_device.settings, - "coiot", - {"enabled": True, "update_period": 15, "peer": "10.10.10.10:5683"}, - ) + """Test no repair issues when CoIoT configuration is valid.""" issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=MOCK_MAC) assert await async_setup_component(hass, "repairs", {}) with patch( @@ -612,6 +615,7 @@ async def test_coiot_configured_no_issue_created( ): await hass.async_block_till_done() await init_integration(hass, 1) + await mock_block_device_push_update_failure(hass, mock_block_device) assert issue_registry.async_get_issue(DOMAIN, issue_id) is None @@ -635,23 +639,47 @@ async def test_coiot_key_missing_no_issue_created( assert issue_registry.async_get_issue(DOMAIN, issue_id) is None -async def test_coiot_no_hass_url( +async def test_coiot_push_issue_when_missing_hass_url( hass: HomeAssistant, hass_client: ClientSessionGenerator, mock_block_device: Mock, issue_registry: ir.IssueRegistry, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Test no repair issues handling up-to-date CoIoT configuration.""" + """Test CoIoT push update issue created when HA URL is not available.""" + issue_id = PUSH_UPDATE_ISSUE_ID.format(unique=MOCK_MAC) + assert await async_setup_component(hass, "repairs", {}) + await hass.async_block_till_done() + await init_integration(hass, 1) + + with patch( + "homeassistant.components.shelly.utils.get_url", + side_effect=NoURLAvailableError(), + ): + await mock_block_device_push_update_failure(hass, mock_block_device) + + assert issue_registry.async_get_issue(DOMAIN, issue_id) + assert len(issue_registry.issues) == 1 + + +async def test_coiot_fix_flow_no_hass_url( + hass: HomeAssistant, + hass_client: ClientSessionGenerator, + mock_block_device: Mock, + issue_registry: ir.IssueRegistry, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test CoIoT repair issue when HA URL is not available.""" monkeypatch.setitem( mock_block_device.settings, "coiot", - {"enabled": True, "update_period": 15, "peer": "correct.peer.address:5683"}, + {"enabled": False, "update_period": 15, "peer": "7.7.7.7:5683"}, ) issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=MOCK_MAC) assert await async_setup_component(hass, "repairs", {}) await hass.async_block_till_done() await init_integration(hass, 1) + await mock_block_device_push_update_failure(hass, mock_block_device) assert issue_registry.async_get_issue(DOMAIN, issue_id) assert len(issue_registry.issues) == 1 @@ -688,11 +716,17 @@ async def test_coiot_issue_ignore( monkeypatch: pytest.MonkeyPatch, ) -> None: """Test ignoring the CoIoT unconfigured issue.""" + monkeypatch.setitem( + mock_block_device.settings, + "coiot", + {"enabled": False, "update_period": 15, "peer": "7.7.7.7:5683"}, + ) issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=MOCK_MAC) assert await async_setup_component(hass, "repairs", {}) await hass.async_block_till_done() await init_integration(hass, 1) + await mock_block_device_push_update_failure(hass, mock_block_device) assert issue_registry.async_get_issue(DOMAIN, issue_id) assert len(issue_registry.issues) == 1 @@ -714,17 +748,19 @@ async def test_coiot_issue_ignore( assert issue.dismissed_version -async def test_coiot_plug_1_no_issue_created( +async def test_plug_1_push_update_issue_created( hass: HomeAssistant, mock_block_device: Mock, issue_registry: ir.IssueRegistry, + monkeypatch: pytest.MonkeyPatch, ) -> None: - """Test no repair issues when device is Shelly Plug 1.""" - - mock_block_device.model = MODEL_PLUG - issue_id = COIOT_UNCONFIGURED_ISSUE_ID.format(unique=MOCK_MAC) + """Test push update repair issue when device is Shelly Plug 1.""" + monkeypatch.setattr(mock_block_device, "model", MODEL_PLUG) + issue_id = PUSH_UPDATE_ISSUE_ID.format(unique=MOCK_MAC) assert await async_setup_component(hass, "repairs", {}) await hass.async_block_till_done() - await init_integration(hass, 1) + await init_integration(hass, 1, model=MODEL_PLUG) + await mock_block_device_push_update_failure(hass, mock_block_device) - assert issue_registry.async_get_issue(DOMAIN, issue_id) is None + assert issue_registry.async_get_issue(DOMAIN, issue_id) + assert len(issue_registry.issues) == 1