From 1b487988c9f6516ccf6b509feba9ac5da6b17f6d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 12 Nov 2025 09:29:40 -0600 Subject: [PATCH 1/5] [mqtt] Fix crash with empty broker during upload/logs --- esphome/mqtt.py | 13 +++++++-- tests/unit_tests/test_main.py | 50 +++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/esphome/mqtt.py b/esphome/mqtt.py index 093ee64df4..18526209f7 100644 --- a/esphome/mqtt.py +++ b/esphome/mqtt.py @@ -6,6 +6,7 @@ import logging import ssl import tempfile import time +from typing import Any import paho.mqtt.client as mqtt @@ -154,8 +155,12 @@ def show_discover(config, username=None, password=None, client_id=None): def get_esphome_device_ip( - config, username=None, password=None, client_id=None, timeout=25 -): + config: dict[str, Any], + username: str | None = None, + password: str | None = None, + client_id: str | None = None, + timeout=25, +) -> list[str]: if CONF_MQTT not in config: raise EsphomeError( "Cannot discover IP via MQTT as the config does not include the mqtt: " @@ -166,6 +171,10 @@ def get_esphome_device_ip( "Cannot discover IP via MQTT as the config does not include the device name: " "component" ) + if not config[CONF_MQTT].get(CONF_BROKER): + raise EsphomeError( + "Cannot discover IP via MQTT as the broker is not configured" + ) dev_name = config[CONF_ESPHOME][CONF_NAME] dev_ip = None diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 9e5f399381..ccbc5a1306 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -1166,6 +1166,56 @@ def test_upload_program_ota_with_mqtt_resolution( ) +def test_upload_program_ota_with_mqtt_empty_broker( + mock_mqtt_get_ip: Mock, + mock_is_ip_address: Mock, + mock_run_ota: Mock, + tmp_path: Path, + caplog: CaptureFixture, +) -> None: + """Test upload_program with OTA when MQTT broker is empty (issue #11653).""" + setup_core(address="192.168.1.50", platform=PLATFORM_ESP32, tmp_path=tmp_path) + + mock_is_ip_address.return_value = True + mock_mqtt_get_ip.side_effect = EsphomeError( + "Cannot discover IP via MQTT as the broker is not configured" + ) + mock_run_ota.return_value = (0, "192.168.1.50") + + config = { + CONF_OTA: [ + { + CONF_PLATFORM: CONF_ESPHOME, + CONF_PORT: 3232, + } + ], + CONF_MQTT: { + CONF_BROKER: "", + }, + CONF_MDNS: { + CONF_DISABLED: True, + }, + } + args = MockArgs(username="user", password="pass", client_id="client") + devices = ["MQTTIP", "192.168.1.50"] + + exit_code, host = upload_program(config, args, devices) + + assert exit_code == 0 + assert host == "192.168.1.50" + # Verify MQTT was attempted but failed gracefully + mock_mqtt_get_ip.assert_called_once_with(config, "user", "pass", "client") + # Verify we fell back to the IP address + expected_firmware = ( + tmp_path / ".esphome" / "build" / "test" / ".pioenvs" / "test" / "firmware.bin" + ) + mock_run_ota.assert_called_once_with( + ["192.168.1.50"], 3232, None, expected_firmware + ) + # Verify warning was logged + assert "MQTT IP discovery failed" in caplog.text + + @patch("esphome.__main__.importlib.import_module") def test_upload_program_platform_specific_handler( mock_import: Mock, From fb00f75192a172402af7a2e516d7a2713140b47f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 12 Nov 2025 09:30:46 -0600 Subject: [PATCH 2/5] [mqtt] Fix crash with empty broker during upload/logs --- esphome/mqtt.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/mqtt.py b/esphome/mqtt.py index 18526209f7..d24418cc8a 100644 --- a/esphome/mqtt.py +++ b/esphome/mqtt.py @@ -6,7 +6,6 @@ import logging import ssl import tempfile import time -from typing import Any import paho.mqtt.client as mqtt @@ -31,6 +30,7 @@ from esphome.const import ( from esphome.core import CORE, EsphomeError from esphome.helpers import get_int_env, get_str_env from esphome.log import AnsiFore, color +from esphome.types import ConfigType from esphome.util import safe_print _LOGGER = logging.getLogger(__name__) @@ -155,7 +155,7 @@ def show_discover(config, username=None, password=None, client_id=None): def get_esphome_device_ip( - config: dict[str, Any], + config: ConfigType, username: str | None = None, password: str | None = None, client_id: str | None = None, From d8454e7c0a42e9d81b336164e842baad520dc442 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 12 Nov 2025 09:33:29 -0600 Subject: [PATCH 3/5] Update esphome/mqtt.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/mqtt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/mqtt.py b/esphome/mqtt.py index d24418cc8a..0d50edbc2c 100644 --- a/esphome/mqtt.py +++ b/esphome/mqtt.py @@ -159,7 +159,7 @@ def get_esphome_device_ip( username: str | None = None, password: str | None = None, client_id: str | None = None, - timeout=25, + timeout: int | float = 25, ) -> list[str]: if CONF_MQTT not in config: raise EsphomeError( From 4b3d3c4ca2aefa8990cfe255ab94b2e8b8e69c5b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 12 Nov 2025 11:51:35 -0600 Subject: [PATCH 4/5] some basic tests --- .coveragerc | 5 -- tests/unit_tests/test_mqtt.py | 91 +++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) delete mode 100644 .coveragerc create mode 100644 tests/unit_tests/test_mqtt.py diff --git a/.coveragerc b/.coveragerc deleted file mode 100644 index c15e79a31b..0000000000 --- a/.coveragerc +++ /dev/null @@ -1,5 +0,0 @@ -[run] -omit = - esphome/components/* - esphome/analyze_memory/* - tests/integration/* diff --git a/tests/unit_tests/test_mqtt.py b/tests/unit_tests/test_mqtt.py new file mode 100644 index 0000000000..4c2c34dff1 --- /dev/null +++ b/tests/unit_tests/test_mqtt.py @@ -0,0 +1,91 @@ +"""Unit tests for esphome.mqtt module.""" + +from __future__ import annotations + +import pytest + +from esphome.const import CONF_BROKER, CONF_ESPHOME, CONF_MQTT, CONF_NAME +from esphome.core import EsphomeError +from esphome.mqtt import get_esphome_device_ip + + +def test_get_esphome_device_ip_empty_broker() -> None: + """Test that get_esphome_device_ip raises EsphomeError when broker is empty.""" + config = { + CONF_MQTT: { + CONF_BROKER: "", + }, + CONF_ESPHOME: { + CONF_NAME: "test-device", + }, + } + + with pytest.raises( + EsphomeError, + match="Cannot discover IP via MQTT as the broker is not configured", + ): + get_esphome_device_ip(config) + + +def test_get_esphome_device_ip_none_broker() -> None: + """Test that get_esphome_device_ip raises EsphomeError when broker is None.""" + config = { + CONF_MQTT: { + CONF_BROKER: None, + }, + CONF_ESPHOME: { + CONF_NAME: "test-device", + }, + } + + with pytest.raises( + EsphomeError, + match="Cannot discover IP via MQTT as the broker is not configured", + ): + get_esphome_device_ip(config) + + +def test_get_esphome_device_ip_missing_mqtt() -> None: + """Test that get_esphome_device_ip raises EsphomeError when mqtt config is missing.""" + config = { + CONF_ESPHOME: { + CONF_NAME: "test-device", + }, + } + + with pytest.raises( + EsphomeError, + match="Cannot discover IP via MQTT as the config does not include the mqtt:", + ): + get_esphome_device_ip(config) + + +def test_get_esphome_device_ip_missing_esphome() -> None: + """Test that get_esphome_device_ip raises EsphomeError when esphome config is missing.""" + config = { + CONF_MQTT: { + CONF_BROKER: "mqtt.local", + }, + } + + with pytest.raises( + EsphomeError, + match="Cannot discover IP via MQTT as the config does not include the device name:", + ): + get_esphome_device_ip(config) + + +def test_get_esphome_device_ip_missing_name() -> None: + """Test that get_esphome_device_ip raises EsphomeError when device name is missing.""" + config = { + CONF_MQTT: { + CONF_BROKER: "mqtt.local", + }, + CONF_ESPHOME: {}, + } + + with pytest.raises( + EsphomeError, + match="Cannot discover IP via MQTT as the config does not include the device name:", + ): + get_esphome_device_ip(config) From c299361753657ccc089d84e1c358f755f06a7ec8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 12 Nov 2025 11:51:56 -0600 Subject: [PATCH 5/5] some basic tests --- .coveragerc | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .coveragerc diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000000..c15e79a31b --- /dev/null +++ b/.coveragerc @@ -0,0 +1,5 @@ +[run] +omit = + esphome/components/* + esphome/analyze_memory/* + tests/integration/*