Move QoS setting to shared device properties in MQTT device subentries configuration (#141369)

* Move QoS setting to shared device properties in MQTT device subentries configuration

* Use kwargs for validate_user_input helper

homeassistant/components/mqtt/config_flow.py

@@ -134,6 +134,7 @@ from .const import (
     DEFAULT_PORT,
     DEFAULT_PREFIX,
     DEFAULT_PROTOCOL,
+    DEFAULT_QOS,
     DEFAULT_TRANSPORT,
     DEFAULT_WILL,
     DEFAULT_WS_PATH,
@@ -368,10 +369,6 @@ COMMON_ENTITY_FIELDS = {
     CONF_ENTITY_PICTURE: PlatformField(TEXT_SELECTOR, False, cv.url, "invalid_url"),
 }
 
-COMMON_MQTT_FIELDS = {
-    CONF_QOS: PlatformField(QOS_SELECTOR, False, valid_qos_schema, default=0),
-}
-
 PLATFORM_ENTITY_FIELDS = {
     Platform.NOTIFY.value: {},
     Platform.SENSOR.value: {
@@ -431,16 +428,17 @@ ENTITY_CONFIG_VALIDATOR: dict[
     Platform.SENSOR.value: validate_sensor_platform_config,
 }
 
-MQTT_DEVICE_SCHEMA = vol.Schema(
+MQTT_DEVICE_PLATFORM_FIELDS = {
-    {
+    ATTR_NAME: PlatformField(TEXT_SELECTOR, False, str),
-        vol.Required(ATTR_NAME): TEXT_SELECTOR,
+    ATTR_SW_VERSION: PlatformField(TEXT_SELECTOR, False, str),
-        vol.Optional(ATTR_SW_VERSION): TEXT_SELECTOR,
+    ATTR_HW_VERSION: PlatformField(TEXT_SELECTOR, False, str),
-        vol.Optional(ATTR_HW_VERSION): TEXT_SELECTOR,
+    ATTR_MODEL: PlatformField(TEXT_SELECTOR, False, str),
-        vol.Optional(ATTR_MODEL): TEXT_SELECTOR,
+    ATTR_MODEL_ID: PlatformField(TEXT_SELECTOR, False, str),
-        vol.Optional(ATTR_MODEL_ID): TEXT_SELECTOR,
+    ATTR_CONFIGURATION_URL: PlatformField(TEXT_SELECTOR, False, cv.url, "invalid_url"),
-        vol.Optional(ATTR_CONFIGURATION_URL): TEXT_SELECTOR,
+    CONF_QOS: PlatformField(
-    }
+        QOS_SELECTOR, False, int, default=DEFAULT_QOS, section="mqtt_settings"
-)
+    ),
+}
 
 REAUTH_SCHEMA = vol.Schema(
     {
@@ -527,7 +525,8 @@ def calculate_merged_config(
 def validate_user_input(
     user_input: dict[str, Any],
     data_schema_fields: dict[str, PlatformField],
-    component_data: dict[str, Any] | None,
+    *,
+    component_data: dict[str, Any] | None = None,
     config_validator: Callable[[dict[str, Any]], dict[str, str]] | None = None,
 ) -> tuple[dict[str, Any], dict[str, str]]:
     """Validate user input."""
@@ -566,11 +565,21 @@ def data_schema_from_fields(
     reconfig: bool,
     component_data: dict[str, Any] | None = None,
     user_input: dict[str, Any] | None = None,
+    device_data: MqttDeviceData | None = None,
 ) -> vol.Schema:
-    """Generate custom data schema from platform fields."""
+    """Generate custom data schema from platform fields or device data."""
-    component_data_with_user_input = deepcopy(component_data)
+    if device_data is not None:
+        component_data_with_user_input: dict[str, Any] | None = dict(device_data)
+        if TYPE_CHECKING:
+            assert component_data_with_user_input is not None
+        component_data_with_user_input.update(
+            component_data_with_user_input.pop("mqtt_settings", {})
+        )
+    else:
+        component_data_with_user_input = deepcopy(component_data)
     if component_data_with_user_input is not None and user_input is not None:
         component_data_with_user_input |= user_input
+
     sections: dict[str | None, None] = {
         field_details.section: None for field_details in data_schema_fields.values()
     }
@@ -1221,17 +1230,26 @@ class MQTTSubentryFlowHandler(ConfigSubentryFlow):
         self, user_input: dict[str, Any] | None = None
     ) -> SubentryFlowResult:
         """Add a new MQTT device."""
-        errors: dict[str, str] = {}
+        errors: dict[str, Any] = {}
-        validate_field("configuration_url", cv.url, user_input, errors, "invalid_url")
+        device_data = self._subentry_data[CONF_DEVICE]
-        if not errors and user_input is not None:
+        data_schema = data_schema_from_fields(
-            self._subentry_data[CONF_DEVICE] = cast(MqttDeviceData, user_input)
+            MQTT_DEVICE_PLATFORM_FIELDS,
-            if self.source == SOURCE_RECONFIGURE:
+            device_data=device_data,
-                return await self.async_step_summary_menu()
+            reconfig=True,
-            return await self.async_step_entity()
+        )
-
+        if user_input is not None:
+            merged_user_input, errors = validate_user_input(
+                user_input, MQTT_DEVICE_PLATFORM_FIELDS
+            )
+            if not errors:
+                self._subentry_data[CONF_DEVICE] = cast(
+                    MqttDeviceData, merged_user_input
+                )
+                if self.source == SOURCE_RECONFIGURE:
+                    return await self.async_step_summary_menu()
+                return await self.async_step_entity()
         data_schema = self.add_suggested_values_to_schema(
-            MQTT_DEVICE_SCHEMA,
+            data_schema, device_data if user_input is None else user_input
-            self._subentry_data[CONF_DEVICE] if user_input is None else user_input,
         )
         return self.async_show_form(
             step_id=CONF_DEVICE,
@@ -1257,7 +1275,7 @@ class MQTTSubentryFlowHandler(ConfigSubentryFlow):
         data_schema = data_schema_from_fields(data_schema_fields, reconfig=reconfig)
         if user_input is not None:
             merged_user_input, errors = validate_user_input(
-                user_input, data_schema_fields, component_data
+                user_input, data_schema_fields, component_data=component_data
             )
             if not errors:
                 if self._component_id is None:
@@ -1357,8 +1375,8 @@ class MQTTSubentryFlowHandler(ConfigSubentryFlow):
             merged_user_input, errors = validate_user_input(
                 user_input,
                 data_schema_fields,
-                component_data,
+                component_data=component_data,
-                ENTITY_CONFIG_VALIDATOR[platform],
+                config_validator=ENTITY_CONFIG_VALIDATOR[platform],
             )
             if not errors:
                 self.update_component_fields(data_schema_fields, merged_user_input)
@@ -1395,7 +1413,7 @@ class MQTTSubentryFlowHandler(ConfigSubentryFlow):
             assert self._component_id is not None
         component_data = self._subentry_data["components"][self._component_id]
         platform = component_data[CONF_PLATFORM]
-        data_schema_fields = PLATFORM_MQTT_FIELDS[platform] | COMMON_MQTT_FIELDS
+        data_schema_fields = PLATFORM_MQTT_FIELDS[platform]
         data_schema = data_schema_from_fields(
             data_schema_fields,
             reconfig=bool(
@@ -1408,8 +1426,8 @@ class MQTTSubentryFlowHandler(ConfigSubentryFlow):
             merged_user_input, errors = validate_user_input(
                 user_input,
                 data_schema_fields,
-                component_data,
+                component_data=component_data,
-                ENTITY_CONFIG_VALIDATOR[platform],
+                config_validator=ENTITY_CONFIG_VALIDATOR[platform],
             )
             if not errors:
                 self.update_component_fields(data_schema_fields, merged_user_input)

homeassistant/components/mqtt/entity.py

@@ -300,6 +300,7 @@ def async_setup_entity_entry_helper(
             availability_config = subentry_data.get("availability", {})
             subentry_entities: list[Entity] = []
             device_config = subentry_data["device"].copy()
+            device_mqtt_options = device_config.pop("mqtt_settings", {})
             device_config["identifiers"] = config_subentry_id
             for component_id, component_data in subentry_data["components"].items():
                 if component_data["platform"] != domain:
@@ -311,6 +312,7 @@ def async_setup_entity_entry_helper(
                 component_config[CONF_DEVICE] = device_config
                 component_config.pop("platform")
                 component_config.update(availability_config)
+                component_config.update(device_mqtt_options)
 
                 try:
                     config = platform_schema_modern(component_config)

homeassistant/components/mqtt/models.py

@@ -420,6 +420,12 @@ class MqttComponentConfig:
     discovery_payload: MQTTDiscoveryPayload
 
 
+class DeviceMqttOptions(TypedDict, total=False):
+    """Hold the shared MQTT specific options for an MQTT device."""
+
+    qos: int
+
+
 class MqttDeviceData(TypedDict, total=False):
     """Hold the data for an MQTT device."""
 
@@ -430,6 +436,7 @@ class MqttDeviceData(TypedDict, total=False):
     hw_version: str
     model: str
     model_id: str
+    mqtt_settings: DeviceMqttOptions
 
 
 class MqttAvailabilityData(TypedDict, total=False):

homeassistant/components/mqtt/strings.json

@@ -150,6 +150,17 @@
             "hw_version": "The hardware version of the device. E.g. 'v1.0 rev a'.",
             "model": "E.g. 'Cleanmaster Pro'.",
             "model_id": "E.g. '123NK2PRO'."
+          },
+          "sections": {
+            "mqtt_settings": {
+              "name": "MQTT Settings",
+              "data": {
+                "qos": "QoS"
+              },
+              "data_description": {
+                "qos": "The QoS value the device's entities should use."
+              }
+            }
           }
         },
         "summary_menu": {
@@ -235,8 +246,7 @@
             "value_template": "Value template",
             "last_reset_value_template": "Last reset value template",
             "force_update": "Force update",
-            "retain": "Retain",
+            "retain": "Retain"
-            "qos": "QoS"
           },
           "data_description": {
             "command_topic": "The publishing topic that will be used to control the {platform} entity. [Learn more.]({url}#command_topic)",
@@ -245,8 +255,7 @@
             "value_template": "Defines a [template](https://www.home-assistant.io/docs/configuration/templating/#using-value-templates-with-mqtt) to extract the {platform} entity value.",
             "last_reset_value_template": "Defines a [template](https://www.home-assistant.io/docs/configuration/templating/#using-value-templates-with-mqtt) to extract the last reset. When Last reset template is set, the State class option must be Total. [Learn more.]({url}#last_reset_value_template)",
             "force_update": "Sends update events even if the value hasn’t changed. Useful if you want to have meaningful value graphs in history. [Learn more.]({url}#force_update)",
-            "retain": "Select if values published by the {platform} entity should be retained at the MQTT broker.",
+            "retain": "Select if values published by the {platform} entity should be retained at the MQTT broker."
-            "qos": "The QoS value {platform} entity should use."
           },
           "sections": {
             "advanced_settings": {

tests/components/mqtt/common.py

@@ -70,7 +70,6 @@ MOCK_SUBENTRY_NOTIFY_COMPONENT1 = {
     "363a7ecad6be4a19b939a016ea93e994": {
         "platform": "notify",
         "name": "Milkman alert",
-        "qos": 0,
         "command_topic": "test-topic",
         "command_template": "{{ value }}",
         "entity_picture": "https://example.com/363a7ecad6be4a19b939a016ea93e994",
@@ -81,7 +80,6 @@ MOCK_SUBENTRY_NOTIFY_COMPONENT2 = {
     "6494827dac294fa0827c54b02459d309": {
         "platform": "notify",
         "name": "The second notifier",
-        "qos": 0,
         "command_topic": "test-topic2",
         "entity_picture": "https://example.com/6494827dac294fa0827c54b02459d309",
     },
@@ -89,7 +87,6 @@ MOCK_SUBENTRY_NOTIFY_COMPONENT2 = {
 MOCK_SUBENTRY_NOTIFY_COMPONENT_NO_NAME = {
     "5269352dd9534c908d22812ea5d714cd": {
         "platform": "notify",
-        "qos": 0,
         "command_topic": "test-topic",
         "command_template": "{{ value }}",
         "entity_picture": "https://example.com/5269352dd9534c908d22812ea5d714cd",
@@ -102,7 +99,6 @@ MOCK_SUBENTRY_SENSOR_COMPONENT = {
         "platform": "sensor",
         "name": "Energy",
         "device_class": "enum",
-        "qos": 1,
         "state_topic": "test-topic",
         "options": ["low", "medium", "high"],
         "expire_after": 30,
@@ -117,7 +113,6 @@ MOCK_SUBENTRY_SENSOR_COMPONENT_STATE_CLASS = {
         "state_class": "measurement",
         "state_topic": "test-topic",
         "entity_picture": "https://example.com/a0f85790a95d4889924602effff06b6e",
-        "qos": 0,
     },
 }
 MOCK_SUBENTRY_SENSOR_COMPONENT_LAST_RESET = {
@@ -128,7 +123,6 @@ MOCK_SUBENTRY_SENSOR_COMPONENT_LAST_RESET = {
         "last_reset_value_template": "{{ value_json.value }}",
         "state_topic": "test-topic",
         "entity_picture": "https://example.com/e9261f6feed443e7b7d5f3fbe2a47412",
-        "qos": 0,
     },
 }
 
@@ -139,7 +133,6 @@ MOCK_SUBENTRY_LIGHT_COMPONENT = {
     "8131babc5e8d4f44b82e0761d39091a2": {
         "platform": "light",
         "name": "Test light",
-        "qos": 1,
         "command_topic": "test-topic4",
         "schema": "basic",
         "entity_picture": "https://example.com/8131babc5e8d4f44b82e0761d39091a2",
@@ -149,7 +142,6 @@ MOCK_SUBENTRY_NOTIFY_BAD_SCHEMA = {
     "b10b531e15244425a74bb0abb1e9d2c6": {
         "platform": "notify",
         "name": "Test",
-        "qos": 1,
         "command_topic": "bad#topic",
     },
 }
@@ -183,6 +175,7 @@ MOCK_NOTIFY_SUBENTRY_DATA_SINGLE = {
         "model": "Model XL",
         "model_id": "mn002",
         "configuration_url": "https://example.com",
+        "mqtt_settings": {"qos": 1},
     },
     "components": MOCK_SUBENTRY_NOTIFY_COMPONENT1,
 }

tests/components/mqtt/test_config_flow.py

@@ -2616,6 +2616,7 @@ async def test_migrate_of_incompatible_config_entry(
 @pytest.mark.parametrize(
     (
         "config_subentries_data",
+        "mock_device_user_input",
         "mock_entity_user_input",
         "mock_entity_details_user_input",
         "mock_entity_details_failed_user_input",
@@ -2626,13 +2627,13 @@ async def test_migrate_of_incompatible_config_entry(
     [
         (
             MOCK_NOTIFY_SUBENTRY_DATA_SINGLE,
+            {"name": "Milk notifier", "mqtt_settings": {"qos": 1}},
             {"name": "Milkman alert"},
             None,
             None,
             {
                 "command_topic": "test-topic",
                 "command_template": "{{ value }}",
-                "qos": 0,
                 "retain": False,
             },
             (
@@ -2645,13 +2646,13 @@ async def test_migrate_of_incompatible_config_entry(
         ),
         (
             MOCK_NOTIFY_SUBENTRY_DATA_NO_NAME,
+            {"name": "Milk notifier", "mqtt_settings": {"qos": 0}},
             {},
             None,
             None,
             {
                 "command_topic": "test-topic",
                 "command_template": "{{ value }}",
-                "qos": 0,
                 "retain": False,
             },
             (
@@ -2664,6 +2665,7 @@ async def test_migrate_of_incompatible_config_entry(
         ),
         (
             MOCK_SENSOR_SUBENTRY_DATA_SINGLE,
+            {"name": "Test sensor", "mqtt_settings": {"qos": 0}},
             {"name": "Energy"},
             {"device_class": "enum", "options": ["low", "medium", "high"]},
             (
@@ -2708,7 +2710,6 @@ async def test_migrate_of_incompatible_config_entry(
                 "state_topic": "test-topic",
                 "value_template": "{{ value_json.value }}",
                 "advanced_settings": {"expire_after": 30},
-                "qos": 1,
             },
             (
                 (
@@ -2720,6 +2721,7 @@ async def test_migrate_of_incompatible_config_entry(
         ),
         (
             MOCK_SENSOR_SUBENTRY_DATA_SINGLE_STATE_CLASS,
+            {"name": "Test sensor", "mqtt_settings": {"qos": 0}},
             {"name": "Energy"},
             {
                 "state_class": "measurement",
@@ -2743,6 +2745,7 @@ async def test_subentry_configflow(
     hass: HomeAssistant,
     mqtt_mock_entry: MqttMockHAClientGenerator,
     config_subentries_data: dict[str, Any],
+    mock_device_user_input: dict[str, Any],
     mock_entity_user_input: dict[str, Any],
     mock_entity_details_user_input: dict[str, Any],
     mock_entity_details_failed_user_input: tuple[
@@ -2753,7 +2756,7 @@ async def test_subentry_configflow(
     entity_name: str,
 ) -> None:
     """Test the subentry ConfigFlow."""
-    device_name = config_subentries_data["device"]["name"]
+    device_name = mock_device_user_input["name"]
     component = next(iter(config_subentries_data["components"].values()))
 
     await mqtt_mock_entry()
@@ -2780,14 +2783,7 @@ async def test_subentry_configflow(
 
     result = await hass.config_entries.subentries.async_configure(
         result["flow_id"],
-        user_input={
+        user_input=mock_device_user_input,
-            "name": device_name,
-            "sw_version": "1.0",
-            "hw_version": "2.1 rev a",
-            "model": "Model XL",
-            "model_id": "mn002",
-            "configuration_url": "https://example.com",
-        },
     )
     assert result["type"] is FlowResultType.FORM
     assert result["step_id"] == "entity"
@@ -3471,7 +3467,6 @@ async def test_subentry_reconfigure_edit_entity_reset_fields(
             },
             {
                 "command_topic": "test-topic2",
-                "qos": 0,
             },
         )
     ],

tests/components/mqtt/test_mixins.py

@@ -1,7 +1,7 @@
 """The tests for shared code of the MQTT platform."""
 
 from typing import Any
-from unittest.mock import patch
+from unittest.mock import call, patch
 
 import pytest
 
@@ -21,7 +21,11 @@ from homeassistant.helpers import (
 )
 from homeassistant.util import slugify
 
-from .common import MOCK_SUBENTRY_DATA_BAD_COMPONENT_SCHEMA, MOCK_SUBENTRY_DATA_SET_MIX
+from .common import (
+    MOCK_NOTIFY_SUBENTRY_DATA_SINGLE,
+    MOCK_SUBENTRY_DATA_BAD_COMPONENT_SCHEMA,
+    MOCK_SUBENTRY_DATA_SET_MIX,
+)
 
 from tests.common import MockConfigEntry, async_capture_events, async_fire_mqtt_message
 from tests.typing import MqttMockHAClientGenerator
@@ -547,3 +551,39 @@ async def test_loading_subentry_with_bad_component_schema(
         "Schema violation occurred when trying to set up entity from subentry"
         in caplog.text
     )
+
+
+@pytest.mark.parametrize(
+    "mqtt_config_subentries_data",
+    [
+        (
+            ConfigSubentryData(
+                data=MOCK_NOTIFY_SUBENTRY_DATA_SINGLE,
+                subentry_type="device",
+                title="Mock subentry",
+            ),
+        )
+    ],
+)
+async def test_qos_on_mqt_device_from_subentry(
+    hass: HomeAssistant,
+    mqtt_mock_entry: MqttMockHAClientGenerator,
+    mqtt_config_subentries_data: tuple[dict[str, Any]],
+    device_registry: dr.DeviceRegistry,
+) -> None:
+    """Test QoS is set correctly on entities from MQTT device."""
+    mqtt_mock = await mqtt_mock_entry()
+    entry = hass.config_entries.async_entries(mqtt.DOMAIN)[0]
+    subentry_id = next(iter(entry.subentries))
+    # Each subentry has one device
+    device = device_registry.async_get_device({("mqtt", subentry_id)})
+    assert device is not None
+    assert hass.states.get("notify.milk_notifier_milkman_alert") is not None
+    await hass.services.async_call(
+        "notify",
+        "send_message",
+        {"entity_id": "notify.milk_notifier_milkman_alert", "message": "Test message"},
+    )
+    await hass.async_block_till_done()
+    assert len(mqtt_mock.async_publish.mock_calls) == 1
+    mqtt_mock.async_publish.mock_calls[0] = call("test-topic", "Test message", 1, False)