From ff8b9040975ce71dfd8eab7679619fe7859aff19 Mon Sep 17 00:00:00 2001
From: kahrendt <kahrendt@gmail.com>
Date: Sun, 5 Nov 2023 20:19:03 -0500
Subject: [PATCH] Null topic_prefix disables MQTT publishing/subscription
 unless topic is explicitly configured (#5644)

---
 esphome/components/mqtt/__init__.py        | 60 +++++++++++++---------
 esphome/components/mqtt/mqtt_component.cpp | 33 ++++++++----
 tests/test4.yaml                           | 27 ++++++----
 3 files changed, 77 insertions(+), 43 deletions(-)

diff --git a/esphome/components/mqtt/__init__.py b/esphome/components/mqtt/__init__.py
index 6d0846edad..baf32097fa 100644
--- a/esphome/components/mqtt/__init__.py
+++ b/esphome/components/mqtt/__init__.py
@@ -133,33 +133,47 @@ def validate_config(value):
     # Populate default fields
     out = value.copy()
     topic_prefix = value[CONF_TOPIC_PREFIX]
+    # If the topic prefix is not null and these messages are not configured, then set them to the default
+    # If the topic prefix is null and these messages are not configured, then set them to null
     if CONF_BIRTH_MESSAGE not in value:
-        out[CONF_BIRTH_MESSAGE] = {
-            CONF_TOPIC: f"{topic_prefix}/status",
-            CONF_PAYLOAD: "online",
-            CONF_QOS: 0,
-            CONF_RETAIN: True,
-        }
+        if topic_prefix != "":
+            out[CONF_BIRTH_MESSAGE] = {
+                CONF_TOPIC: f"{topic_prefix}/status",
+                CONF_PAYLOAD: "online",
+                CONF_QOS: 0,
+                CONF_RETAIN: True,
+            }
+        else:
+            out[CONF_BIRTH_MESSAGE] = {}
     if CONF_WILL_MESSAGE not in value:
-        out[CONF_WILL_MESSAGE] = {
-            CONF_TOPIC: f"{topic_prefix}/status",
-            CONF_PAYLOAD: "offline",
-            CONF_QOS: 0,
-            CONF_RETAIN: True,
-        }
+        if topic_prefix != "":
+            out[CONF_WILL_MESSAGE] = {
+                CONF_TOPIC: f"{topic_prefix}/status",
+                CONF_PAYLOAD: "offline",
+                CONF_QOS: 0,
+                CONF_RETAIN: True,
+            }
+        else:
+            out[CONF_WILL_MESSAGE] = {}
     if CONF_SHUTDOWN_MESSAGE not in value:
-        out[CONF_SHUTDOWN_MESSAGE] = {
-            CONF_TOPIC: f"{topic_prefix}/status",
-            CONF_PAYLOAD: "offline",
-            CONF_QOS: 0,
-            CONF_RETAIN: True,
-        }
+        if topic_prefix != "":
+            out[CONF_SHUTDOWN_MESSAGE] = {
+                CONF_TOPIC: f"{topic_prefix}/status",
+                CONF_PAYLOAD: "offline",
+                CONF_QOS: 0,
+                CONF_RETAIN: True,
+            }
+        else:
+            out[CONF_SHUTDOWN_MESSAGE] = {}
     if CONF_LOG_TOPIC not in value:
-        out[CONF_LOG_TOPIC] = {
-            CONF_TOPIC: f"{topic_prefix}/debug",
-            CONF_QOS: 0,
-            CONF_RETAIN: True,
-        }
+        if topic_prefix != "":
+            out[CONF_LOG_TOPIC] = {
+                CONF_TOPIC: f"{topic_prefix}/debug",
+                CONF_QOS: 0,
+                CONF_RETAIN: True,
+            }
+        else:
+            out[CONF_LOG_TOPIC] = {}
     return out
 
 
diff --git a/esphome/components/mqtt/mqtt_component.cpp b/esphome/components/mqtt/mqtt_component.cpp
index 95dc082e84..f9f8c850e9 100644
--- a/esphome/components/mqtt/mqtt_component.cpp
+++ b/esphome/components/mqtt/mqtt_component.cpp
@@ -23,8 +23,13 @@ std::string MQTTComponent::get_discovery_topic_(const MQTTDiscoveryInfo &discove
 }
 
 std::string MQTTComponent::get_default_topic_for_(const std::string &suffix) const {
-  return global_mqtt_client->get_topic_prefix() + "/" + this->component_type() + "/" + this->get_default_object_id_() +
-         "/" + suffix;
+  const std::string &topic_prefix = global_mqtt_client->get_topic_prefix();
+  if (topic_prefix.empty()) {
+    // If the topic_prefix is null, the default topic should be null
+    return "";
+  }
+
+  return topic_prefix + "/" + this->component_type() + "/" + this->get_default_object_id_() + "/" + suffix;
 }
 
 std::string MQTTComponent::get_state_topic_() const {
@@ -245,17 +250,25 @@ std::string MQTTComponent::friendly_name() const { return this->get_entity()->ge
 std::string MQTTComponent::get_icon() const { return this->get_entity()->get_icon(); }
 bool MQTTComponent::is_disabled_by_default() const { return this->get_entity()->is_disabled_by_default(); }
 bool MQTTComponent::is_internal() {
-  if ((this->get_state_topic_().empty()) || (this->get_command_topic_().empty())) {
-    // If both state_topic and command_topic are empty, then the entity is internal to mqtt
+  if (this->has_custom_state_topic_) {
+    // If the custom state_topic is null, return true as it is internal and should not publish
+    // else, return false, as it is explicitly set to a topic, so it is not internal and should publish
+    return this->get_state_topic_().empty();
+  }
+
+  if (this->has_custom_command_topic_) {
+    // If the custom command_topic is null, return true as it is internal and should not publish
+    // else, return false, as it is explicitly set to a topic, so it is not internal and should publish
+    return this->get_command_topic_().empty();
+  }
+
+  // No custom topics have been set
+  if (this->get_default_topic_for_("").empty()) {
+    // If the default topic prefix is null, then the component, by default, is internal and should not publish
     return true;
   }
 
-  if (this->has_custom_state_topic_ || this->has_custom_command_topic_) {
-    // If a custom state_topic or command_topic is set, then the entity is not internal to mqtt
-    return false;
-  }
-
-  // Use ESPHome's entity internal state
+  // Use ESPHome's component internal state if topic_prefix is not null with no custom state_topic or command_topic
   return this->get_entity()->is_internal();
 }
 
diff --git a/tests/test4.yaml b/tests/test4.yaml
index 3d0ed2f658..a5e5b05e8b 100644
--- a/tests/test4.yaml
+++ b/tests/test4.yaml
@@ -24,6 +24,13 @@ ethernet:
 network:
   enable_ipv6: true
 
+mqtt:
+  broker: test.mosquitto.org
+  port: 1883
+  discovery: true
+  discovery_prefix: homeassistant
+  topic_prefix:
+
 api:
 
 i2c:
@@ -32,15 +39,15 @@ i2c:
   scan: false
 
 spi:
-- id: spi_id_1
-  clk_pin: GPIO21
-  mosi_pin: GPIO22
-  miso_pin: GPIO23
-  interface: hardware
-- id: spi_id_2
-  clk_pin: GPIO32
-  mosi_pin: GPIO33
-  interface: hardware
+  - id: spi_id_1
+    clk_pin: GPIO21
+    mosi_pin: GPIO22
+    miso_pin: GPIO23
+    interface: hardware
+  - id: spi_id_2
+    clk_pin: GPIO32
+    mosi_pin: GPIO33
+    interface: hardware
 
 uart:
   - id: uart115200
@@ -281,6 +288,7 @@ sensor:
     id: a01nyub_sensor
     name: "a01nyub Distance"
     uart_id: uart9600
+    state_topic: "esphome/sensor/a01nyub_sensor/state"
 
 #
 # platform sensor.apds9960 requires component apds9960
@@ -764,7 +772,6 @@ speaker:
     i2s_dout_pin: GPIO25
     mode: mono
 
-
 voice_assistant:
   microphone: mic_id_external
   speaker: speaker_id