From ddef681dd2b1136cffeeb3141cda489e2eaed7a4 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Tue, 7 Apr 2020 18:38:22 +0200 Subject: [PATCH] Improve MQTT debug info for subscriptions with wildcard (#33752) --- homeassistant/components/mqtt/debug_info.py | 37 +++++++++------ homeassistant/components/mqtt/subscription.py | 10 ++-- tests/components/mqtt/test_common.py | 46 +++++++++++-------- tests/components/mqtt/test_init.py | 21 +++++---- 4 files changed, 67 insertions(+), 47 deletions(-) diff --git a/homeassistant/components/mqtt/debug_info.py b/homeassistant/components/mqtt/debug_info.py index b51ee619a12..796453ec9e7 100644 --- a/homeassistant/components/mqtt/debug_info.py +++ b/homeassistant/components/mqtt/debug_info.py @@ -21,8 +21,10 @@ def log_messages(hass: HomeAssistantType, entity_id: str) -> MessageCallbackType def _log_message(msg): """Log message.""" debug_info = hass.data[DATA_MQTT_DEBUG_INFO] - messages = debug_info["entities"][entity_id]["topics"][msg.subscribed_topic] - messages.append(msg.payload) + messages = debug_info["entities"][entity_id]["subscriptions"][ + msg.subscribed_topic + ] + messages.append((msg.payload, msg.topic)) def _decorator(msg_callback: MessageCallbackType): @wraps(msg_callback) @@ -37,24 +39,26 @@ def log_messages(hass: HomeAssistantType, entity_id: str) -> MessageCallbackType return _decorator -def add_topic(hass, message_callback, topic): - """Prepare debug data for topic.""" +def add_subscription(hass, message_callback, subscription): + """Prepare debug data for subscription.""" entity_id = getattr(message_callback, "__entity_id", None) if entity_id: debug_info = hass.data.setdefault( DATA_MQTT_DEBUG_INFO, {"entities": {}, "triggers": {}} ) entity_info = debug_info["entities"].setdefault( - entity_id, {"topics": {}, "discovery_data": {}} + entity_id, {"subscriptions": {}, "discovery_data": {}} ) - entity_info["topics"][topic] = deque([], STORED_MESSAGES) + entity_info["subscriptions"][subscription] = deque([], STORED_MESSAGES) -def remove_topic(hass, message_callback, topic): - """Remove debug data for topic.""" +def remove_subscription(hass, message_callback, subscription): + """Remove debug data for subscription.""" entity_id = getattr(message_callback, "__entity_id", None) if entity_id and entity_id in hass.data[DATA_MQTT_DEBUG_INFO]["entities"]: - hass.data[DATA_MQTT_DEBUG_INFO]["entities"][entity_id]["topics"].pop(topic) + hass.data[DATA_MQTT_DEBUG_INFO]["entities"][entity_id]["subscriptions"].pop( + subscription + ) def add_entity_discovery_data(hass, discovery_data, entity_id): @@ -63,7 +67,7 @@ def add_entity_discovery_data(hass, discovery_data, entity_id): DATA_MQTT_DEBUG_INFO, {"entities": {}, "triggers": {}} ) entity_info = debug_info["entities"].setdefault( - entity_id, {"topics": {}, "discovery_data": {}} + entity_id, {"subscriptions": {}, "discovery_data": {}} ) entity_info["discovery_data"] = discovery_data @@ -117,9 +121,14 @@ async def info_for_device(hass, device_id): continue entity_info = mqtt_debug_info["entities"][entry.entity_id] - topics = [ - {"topic": topic, "messages": list(messages)} - for topic, messages in entity_info["topics"].items() + subscriptions = [ + { + "topic": topic, + "messages": [ + {"payload": msg[0], "topic": msg[1]} for msg in list(messages) + ], + } + for topic, messages in entity_info["subscriptions"].items() ] discovery_data = { "topic": entity_info["discovery_data"].get(ATTR_DISCOVERY_TOPIC, ""), @@ -128,7 +137,7 @@ async def info_for_device(hass, device_id): mqtt_info["entities"].append( { "entity_id": entry.entity_id, - "topics": topics, + "subscriptions": subscriptions, "discovery_data": discovery_data, } ) diff --git a/homeassistant/components/mqtt/subscription.py b/homeassistant/components/mqtt/subscription.py index b4793a49dca..c1de08d5be8 100644 --- a/homeassistant/components/mqtt/subscription.py +++ b/homeassistant/components/mqtt/subscription.py @@ -34,14 +34,16 @@ class EntitySubscription: if other is not None and other.unsubscribe_callback is not None: other.unsubscribe_callback() # Clear debug data if it exists - debug_info.remove_topic(self.hass, other.message_callback, other.topic) + debug_info.remove_subscription( + self.hass, other.message_callback, other.topic + ) if self.topic is None: # We were asked to remove the subscription or not to create it return # Prepare debug data - debug_info.add_topic(self.hass, self.message_callback, self.topic) + debug_info.add_subscription(self.hass, self.message_callback, self.topic) self.unsubscribe_callback = await mqtt.async_subscribe( hass, self.topic, self.message_callback, self.qos, self.encoding @@ -96,7 +98,9 @@ async def async_subscribe_topics( if remaining.unsubscribe_callback is not None: remaining.unsubscribe_callback() # Clear debug data if it exists - debug_info.remove_topic(hass, remaining.message_callback, remaining.topic) + debug_info.remove_subscription( + hass, remaining.message_callback, remaining.topic + ) return new_state diff --git a/tests/components/mqtt/test_common.py b/tests/components/mqtt/test_common.py index 0d1b892611d..3e95e5a1afe 100644 --- a/tests/components/mqtt/test_common.py +++ b/tests/components/mqtt/test_common.py @@ -551,9 +551,9 @@ async def help_test_entity_debug_info(hass, mqtt_mock, domain, config): == f"homeassistant/{domain}/bla/config" ) assert debug_info_data["entities"][0]["discovery_data"]["payload"] == config - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 assert {"topic": "test-topic", "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] assert len(debug_info_data["triggers"]) == 0 @@ -581,24 +581,27 @@ async def help_test_entity_debug_info_max_messages(hass, mqtt_mock, domain, conf assert device is not None debug_info_data = await debug_info.info_for_device(hass, device.id) - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 assert {"topic": "test-topic", "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] for i in range(0, debug_info.STORED_MESSAGES + 1): async_fire_mqtt_message(hass, "test-topic", f"{i}") debug_info_data = await debug_info.info_for_device(hass, device.id) - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 assert ( - len(debug_info_data["entities"][0]["topics"][0]["messages"]) + len(debug_info_data["entities"][0]["subscriptions"][0]["messages"]) == debug_info.STORED_MESSAGES ) - messages = [f"{i}" for i in range(1, debug_info.STORED_MESSAGES + 1)] + messages = [ + {"topic": "test-topic", "payload": f"{i}"} + for i in range(1, debug_info.STORED_MESSAGES + 1) + ] assert {"topic": "test-topic", "messages": messages} in debug_info_data["entities"][ 0 - ]["topics"] + ]["subscriptions"] async def help_test_entity_debug_info_message( @@ -634,16 +637,19 @@ async def help_test_entity_debug_info_message( assert device is not None debug_info_data = await debug_info.info_for_device(hass, device.id) - assert len(debug_info_data["entities"][0]["topics"]) >= 1 - assert {"topic": topic, "messages": []} in debug_info_data["entities"][0]["topics"] + assert len(debug_info_data["entities"][0]["subscriptions"]) >= 1 + assert {"topic": topic, "messages": []} in debug_info_data["entities"][0][ + "subscriptions" + ] async_fire_mqtt_message(hass, topic, payload) debug_info_data = await debug_info.info_for_device(hass, device.id) - assert len(debug_info_data["entities"][0]["topics"]) >= 1 - assert {"topic": topic, "messages": [payload]} in debug_info_data["entities"][0][ - "topics" - ] + assert len(debug_info_data["entities"][0]["subscriptions"]) >= 1 + assert { + "topic": topic, + "messages": [{"topic": topic, "payload": payload}], + } in debug_info_data["entities"][0]["subscriptions"] async def help_test_entity_debug_info_remove(hass, mqtt_mock, domain, config): @@ -675,9 +681,9 @@ async def help_test_entity_debug_info_remove(hass, mqtt_mock, domain, config): == f"homeassistant/{domain}/bla/config" ) assert debug_info_data["entities"][0]["discovery_data"]["payload"] == config - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 assert {"topic": "test-topic", "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] assert len(debug_info_data["triggers"]) == 0 assert debug_info_data["entities"][0]["entity_id"] == f"{domain}.test" @@ -723,9 +729,9 @@ async def help_test_entity_debug_info_update_entity_id(hass, mqtt_mock, domain, ) assert debug_info_data["entities"][0]["discovery_data"]["payload"] == config assert debug_info_data["entities"][0]["entity_id"] == f"{domain}.test" - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 assert {"topic": "test-topic", "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] assert len(debug_info_data["triggers"]) == 0 @@ -741,9 +747,9 @@ async def help_test_entity_debug_info_update_entity_id(hass, mqtt_mock, domain, ) assert debug_info_data["entities"][0]["discovery_data"]["payload"] == config assert debug_info_data["entities"][0]["entity_id"] == f"{domain}.milk" - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 assert {"topic": "test-topic", "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] assert len(debug_info_data["triggers"]) == 0 assert ( diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index 637cefcf744..de8444446d5 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -1030,7 +1030,7 @@ async def test_mqtt_ws_get_device_debug_info( "entities": [ { "entity_id": "sensor.mqtt_sensor", - "topics": [{"topic": "foobar/sensor", "messages": []}], + "subscriptions": [{"topic": "foobar/sensor", "messages": []}], "discovery_data": { "payload": config, "topic": "homeassistant/sensor/bla/config", @@ -1110,10 +1110,10 @@ async def test_debug_info_multiple_devices(hass, mqtt_mock): assert len(debug_info_data["entities"]) == 1 assert len(debug_info_data["triggers"]) == 0 discovery_data = debug_info_data["entities"][0]["discovery_data"] - assert len(debug_info_data["entities"][0]["topics"]) == 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) == 1 topic = d["config"]["state_topic"] assert {"topic": topic, "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] else: assert len(debug_info_data["entities"]) == 0 @@ -1199,7 +1199,7 @@ async def test_debug_info_multiple_entities_triggers(hass, mqtt_mock): discovery_data = [e["discovery_data"] for e in debug_info_data["entities"]] topic = c["config"]["state_topic"] assert {"topic": topic, "messages": []} in [ - t for e in debug_info_data["entities"] for t in e["topics"] + t for e in debug_info_data["entities"] for t in e["subscriptions"] ] else: discovery_data = [e["discovery_data"] for e in debug_info_data["triggers"]] @@ -1260,15 +1260,16 @@ async def test_debug_info_wildcard(hass, mqtt_mock): assert device is not None debug_info_data = await debug_info.info_for_device(hass, device.id) - assert len(debug_info_data["entities"][0]["topics"]) >= 1 + assert len(debug_info_data["entities"][0]["subscriptions"]) >= 1 assert {"topic": "sensor/#", "messages": []} in debug_info_data["entities"][0][ - "topics" + "subscriptions" ] async_fire_mqtt_message(hass, "sensor/abc", "123") debug_info_data = await debug_info.info_for_device(hass, device.id) - assert len(debug_info_data["entities"][0]["topics"]) >= 1 - assert {"topic": "sensor/#", "messages": ["123"]} in debug_info_data["entities"][0][ - "topics" - ] + assert len(debug_info_data["entities"][0]["subscriptions"]) >= 1 + assert { + "topic": "sensor/#", + "messages": [{"topic": "sensor/abc", "payload": "123"}], + } in debug_info_data["entities"][0]["subscriptions"]