Correctly allow mqtt topics to be none so ESPHome does not sub/pub to them

[jesserockz]

What does this implement/fix?

This allows disabling of sensor publishing via mqtt while not forcing that sensor to be internal (if the user is using both api and mqtt in config).

Replaces and closes #5195

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Other

Related issue or feature (if applicable): fixes esphome/feature-requests#375

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

• ESP32
• ESP32 IDF
• ESP8266
• RP2040

Example entry for config.yaml:

# Example config.yaml
sensor:
  - platform: ...
    name: My sensor
    state_topic:

switch:
  - platform: ...
    name: My Switch
    state_topic:
    command_topic:

Checklist:

• The code change is tested and works locally.
• Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in esphome-docs.

[kahrendt]

This PR seems like a more natural solution than #5195 for disabling MQTT for particular entities, especially since it doesn't require changes to the core. One suggestion is to add a default option. One possible way is if the base MQTT configuration has topic_prefix as none, then all entities would not publish via MQTT unless explicitly overridden. If people have a lot of entities but only want one published via MQTT, then it would be tedious to add a blank state_topic or command_topic to each one.

However, this PR doesn't handle the other case where the entity should only publish via MQTT and not via the API. Perhaps the MQTT component's is_internal() function could check if either has_custom_state_topic_ or has_custom_command_topic_ are true, then it should return false. Otherwise, it should return the entity's is_internal() method as it currently does. That way, an entity marked internal but with a custom topic defined would still be published via MQTT.

Here is how I use that case: I use MQTT to collect long-term statistics in a separate database independent of HomeAssistant. I use the data sent to HA for quick informational purposes/minor automations that do not require frequent updates. I have configured it so that the entities update over the API much less frequently than entities updating via MQTT. I would prefer not to expose the large number of more frequently updated MQTT entities to HA, even if they are disabled_by_default.

If people like these ideas (or some variation), I can implement them after this PR is merged if you don't want to add them yourself.

[jesserockz]

Hi @kahrendt thanks for the feedback

One possible way is if the base MQTT configuration has topic_prefix as none, then all entities would not publish via MQTT unless explicitly overridden. If people have a lot of entities but only want one published via MQTT, then it would be tedious to add a blank state_topic or command_topic to each one.

Yes this would be fine. This PR will actually allow it to become blank ie: topic_prefix: which probably just needs to be checked inside get_default_topic_for_(...) to return "" if the prefix is also ""

Perhaps the MQTT component's is_internal() function could check if either has_custom_state_topic_ or has_custom_command_topic_ are true, then it should return false. Otherwise, it should return the entity's is_internal() method as it currently does. That way, an entity marked internal but with a custom topic defined would still be published via MQTT.

I think this would be an acceptable change

[kahrendt]

I was able to test this out today, and it works as intended with state_topic set to None. However, it still publishes a discovery message with the "state_t": "", as part of the of the JSON. I think there should be a check in thesend_discovery_() function for a None topic, and if so, do not publish the discovery JSON message.

Edit: After further testing, it seems like it is attempting to repeatedly send the discovery message every loop when the topic is set to None.

[kahrendt]

If you modify the is_internal() function as follows, it fixes the issues I mentioned before. It won't send a discovery message if the state_topic or command_topic is null. It also allows the component to be internal with regard to the API, but it will still publish to custom topics if they are explicitly defined. This approach seems like the easiest solution to do all these things, but there may be a better way!

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
    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
  return this->get_entity()->is_internal();
}

[jesserockz]

I dont use mqtt at all, so relying on your testing for this =)