Don't prune blank entity names in overridden discovery data

[tstabrawa]

Proposed change

This change fixes a bug where entities using the default, blank-string entity names that are configured with device_override_class settings that modify their config fields would have their blank name fields removed from their MQTT discovery data, causing Home Assistant to name the entity "<device name> MQTT JSON Light" instead of the default "<device name>". I'm not really sure if this is the intended behavior of Home Assistant (i.e. this could be an HA bug, not ours), but we can preserve our desired naming by not pruning empty name fields from the MQTT discovery data.

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Here is a simple discovery_overrides config that can reproduce the problem using Insteon Dimmers:

  light_no_change:
    discovery_overrides:
      dimmer:
        config:
          brightness: true

With the above set as device_override_class for a dimmer named "Desk Lamp" Home Assistant produces the name "Desk Lamp MQTT JSON Light":

This appears to be because the "name" field is removed from the MQTT discovery data:

With the proposed change, the blank "name" field is preserved, and the entity name stays set to "Desk Lamp" as it would without using the device_override_class.

Checklist

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] Tests have been added to verify that the new code works.
• [ ] Code documentation was added where necessary - N/A

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

• [ ] Documentation added/updated - N/A

Confirmed no new flake8 errors, pylint errors, or code coverage gaps. All tests passing. Also, confirmed updated test (test_load_device_discovery_overrides) fails prior to applying fix:

================================================================================ FAILURES ================================================================================
________________________________________________________ Test_DiscoveryTopic.test_load_device_discovery_overrides ________________________________________________________

self = <test_DiscoveryTopic.Test_DiscoveryTopic object at 0x7f1edc97eaa0>
discovery_fan = <insteon_mqtt.mqtt.topic.DiscoveryTopic.DiscoveryTopic object at 0x7f1edc6eac20>, caplog = <_pytest.logging.LogCaptureFixture object at 0x7f1edc87ff40>

    def test_load_device_discovery_overrides(self, discovery_fan, caplog):
        discovery_fan.mqtt.discovery_enabled = True
        # build and request fake class to be used for tests
        config = {}
        config['fake_dev'] = {'discovery_entities': {
            'fake': {
                "component": "fan",
                "config": {
                    "name": "",
                    "unique_id": "unique",
                    "icon": "fake",
                },
            },
        }}
        discovery_fan.device.config_extra['discovery_class'] = 'fake_dev'
        # Override data from this point
        discovery_fan.discovery_template_data = mock.Mock(return_value={})

        # test empty override dict
        discovery_fan.device.config_extra['discovery_overrides'] = {}
        discovery_fan.load_discovery_data(config)
        assert len(discovery_fan.disc_templates) == 1
        discovery_fan.disc_templates = []

        # test empty device override dict
        discovery_fan.device.config_extra['discovery_overrides'] = { 'device': {} }
        discovery_fan.load_discovery_data(config)
        assert len(discovery_fan.disc_templates) == 1
        discovery_fan.disc_templates = []

        # test empty config override dict
        discovery_fan.device.config_extra['discovery_overrides'] = { 'fake': {
            "config": {},
        }}
        discovery_fan.load_discovery_data(config)
        assert len(discovery_fan.disc_templates) == 1
        discovery_fan.disc_templates = []

        # test for non-matching entity name
        discovery_fan.device.config_extra['discovery_overrides'] = { 'fakefail': {
        }}
        discovery_fan.load_discovery_data(config)
        assert 'Entity to override was not found' in caplog.text
        caplog.clear()

        # test for suppressing entity
        discovery_fan.device.config_extra['discovery_overrides'] = { 'fake': {
            "discoverable": False,
        }}
        discovery_fan.load_discovery_data(config)
        assert len(discovery_fan.disc_templates) == 0

        # test for overriding component
        discovery_fan.device.config_extra['discovery_overrides'] = { 'fake': {
           "component": "switch",
        }}
        discovery_fan.load_discovery_data(config)
        expected_topic = "homeassistant/switch/11_22_33/unique/config"
        assert discovery_fan.disc_templates[0].topic_str == expected_topic
        discovery_fan.disc_templates = []

        # test for overriding config unique_id
        discovery_fan.device.config_extra['discovery_overrides'] = { 'fake': {
            "config": {
                "unique_id": "override",
            },
        }}
        discovery_fan.load_discovery_data(config)
        expected_topic = "homeassistant/fan/11_22_33/override/config"
        assert discovery_fan.disc_templates[0].topic_str == expected_topic
        discovery_fan.disc_templates = []

        # test for adding config attribute
        # Also test that name isn't removed
        discovery_fan.device.config_extra['discovery_overrides'] = { 'fake': {
            "config": {
                "foo": "fake",
            },
        }}
        discovery_fan.load_discovery_data(config)
        payload = json.loads(discovery_fan.disc_templates[0].payload_str)
        assert payload.get("foo", None) == "fake"
>       assert "name" in payload
E       AssertionError: assert 'name' in {'foo': 'fake', 'icon': 'fake', 'unique_id': 'unique'}

tests/mqtt/topic/test_DiscoveryTopic.py:239: AssertionError
--------------------------------------------------------------------------- Captured log call ----------------------------------------------------------------------------
ERROR    insteon_mqtt:DiscoveryTopic.py:347 11.22.33 - Entity to override was not found - fakefail
======================================================================== short test summary info =========================================================================
FAILED tests/mqtt/topic/test_DiscoveryTopic.py::Test_DiscoveryTopic::test_load_device_discovery_overrides - AssertionError: assert 'name' in {'foo': 'fake', 'icon': 'fake', 'unique_id': 'unique'}
===================================================================== 1 failed, 706 passed in 10.65s =====================================================================

[tstabrawa]

Updating to note that with a recent change in the HA behavior (home-assistant/core#107188), my workaround for this issue is no longer effective.

Specifically, I used to be able to be able to set the name in my discovery_overrides entries to "{{name_user_case}}", and HA would present the entity name as the same as the device name. Now, in HA 2024.2.0, with this setting, the entity name is the device name repeated twice.

For example, with the following overrides:

  switch_as_light:
    # Maps a switch to a light, which is nicer in HA for actual lights
    discovery_overrides:
      switch:
        component: "light"
        config:
          name: "{{name_user_case}}"
          uniq_id: "{{address}}_light"
          brightness: false
          schema: "json"
          cmd_t: "{{on_off_topic}}"

I end up with entities with names like "Front Porch Light Front Porch Light".

Please let me know if there's anything else you need in order to get this fix merged. Thanks!

[f1d094]

Is this the same as what is being discussed in https://github.com/TD22057/insteon-mqtt/issues/514 ?

[tstabrawa]

Not really, though #514 is somewhat related. In particular #514 (via its associated merge request -- #516) changed insteon_mqtt/data/config-base.yaml / discovery_entities to set the default name value for simple devices (switches, lights, etc) and the default entities of more complex devices to a blank string (""). This has the effect of causing Home Assistant to assign the corresponding entity name to match the device name. This is the desired behavior.

#523 is about fixing a bug where if you use a discovery_overrides configuration to override some template values, then the blank name field gets completely removed from the MQTT discovery data, which causes Home Assistant to set the entity name to something like "<device name> MQTT JSON Light" instead of the preferred "<device name>".