RFC: Device-level discovery overrides

[kpfleming]

As I get deeper into using MQTT Discovery for my devices, I find that I'd like to be able to 'inject' device-level details into the discovery configuration. For example, HA supports setting the icon, and setting a suggested area, among other attributes.

I don't think it would be practical to try to add support for these in a discovery_class (via templating) because if the user doesn't provide a value the config object would end up with an empty attribute.

This can be done by creating a discovery_class for each device, but that's a large amount of work and it also means that changes to the base discovery classes in newer versions of insteon-mqtt would have to be merged by hand into those customized versions.

Instead, I think a 'merging' strategy, as used already elsewhere, might make sense.

Proposal

1. Give names (keys) to the objects in the discovery_entities list so that they can be referred to from device configuration. This would be especially important for keypad_linc which generates a large number of entities. Example of switch:

discovery_entities:
  switch:
    component: 'switch'
    config: |-
      {
        "uniq_id": "{{address}}_switch",
        "name": "{{name_user_case}}",
        "avty_t": "{{availability_topic}}",
        "cmd_t": "{{on_off_topic}}",
        "stat_t": "{{state_topic}}",
        "device": {{device_info_template}},
        "json_attr_t": "{{state_topic}}",
        "json_attr_tpl": "{%- raw -%}
        { \"timestamp\" : {{value_json.timestamp}}, \"mode\" : \"{{value_json.mode}}\", \"reason\" : \"{{value_json.reason}}\" }
        {%- endraw -%}",
        "val_tpl": "{% raw %}{{value_json.state}}{% endraw %}"
      }

2. Device configuration:

devices:
  switch:
    - aa.bb.cc: "Switch (normal)"
    - dd.ee.ff: "Switch (coffee maker)"
      discovery_details:
        device:
          sa: "Kitchen"
        switch:
          config:
            icon: "mdi:coffee-maker"

3. When the discovery config is being composed to send over MQTT, any keys in discovery_details would replace identical keys in the discovery_class (with special handling for config since it is an 'embedded' object) before templating happens. Any new keys provided would be added in their respective locations. The 'device' section would be used to modify the details provided by the device_info_template which apply to all entities for that device.

I can envision other 'features' of this as well, such as suppressing default entities (so a 6-button KPL won't generate entities for buttons 2, 7, and 8). My goal here is to provide the maximum amount of detail through MQTT Discovery, and avoid having to override/change details in HA itself after the devices/entities have been discovered by the integration (which means that if they need to be removed and rediscovered the details would not be lost).

All feedback very welcome... and for avoidance of doubt, I'm happy to implement this.

[krkeegan]

I had always assumed that the GUI in HA was an easier place to make these changes. If you want to be able to define and control all of this from yaml this seems like a reasonable approach. When coding this please include as many tests as possible just in case something breaks far off in the future and you are around.

[kpfleming]

I have begun working on this in #484. I expect I'll have something ready for review in a few days.

[kpfleming]

So... I've got this mostly working, but have run into a stumbling block.

I want to be able to override (and add) entries to the config structure in a discovery entity. However, that structure is actually a string which contains something that looks like JSON, but isn't JSON (and can't be parsed with a JSON decoder). For my purposes at least it would be much better if this was actually a mapping (dictionary) in the YAML file, which was then converted into JSON before being passed to Jinja2 for template substitution. Essentially the same problem exists with device_info_template, although at the moment the standard version of it is a JSON-compliant string so it can be parsed.

I'm going to experiment with this a bit to see if it's really practical; if it works I think I can provide backwards compatibility for existing configurations, except those configurations won't permit overriding entries in config. That's an acceptable trade-off in my opinion.

When I've got an example of what the new structure would look like I'll post it here for discussion.

[kpfleming]

Well, this of course resulted in quite a bit more code (and lots of new tests), but the end result is good. Here is what device_info_template looks like:

Old

  device_info_template: |-
    {
      "ids": "{{address}}",
      "mf": "Insteon",
      "mdl": "{%- if model_number != 'Unknown' -%}
                {{model_number}} - {{model_description}}
              {%- elif dev_cat_name != 'Unknown' -%}
                {{dev_cat_name}} - 0x{{'%0x' % sub_cat|int }}
              {%- elif dev_cat == 0 and sub_cat == 0 -%}
                No Info
              {%- else -%}
                0x{{'%0x' % dev_cat|int }} - 0x{{'%0x' % sub_cat|int }}
              {%- endif -%}",
      "sw": "0x{{'%0x' % firmware|int }} - {{engine}}",
      "name": "{{name_user_case}}",
      "via_device": "{{modem_addr}}"
    }

New

  device_info_template:
    ids: "{{address}}"
    mf: "Insteon"
    mdl: "{%- if model_number != 'Unknown' -%}
            {{model_number}} - {{model_description}}
          {%- elif dev_cat_name != 'Unknown' -%}
            {{dev_cat_name}} - 0x{{'%0x' % sub_cat|int }}
          {%- elif dev_cat == 0 and sub_cat == 0 -%}
            No Info
          {%- else -%}
            0x{{'%0x' % dev_cat|int }} - 0x{{'%0x' % sub_cat|int }}
          {%- endif -%}"
    sw: "0x{{'%0x' % firmware|int }} - {{engine}}"
    name: "{{name_user_case}}"
    via_device: "{{modem_addr}}"

The config blocks in discovery_entities have similar changes, and everything works smoothly. I've got some more tests to write, and a bunch of documentation to update, but then I'll move the PR out of 'draft' state for discussion.

[kpfleming]

For anyone who may be following this discussion and is interested in a preview... I've completed the code and tests and user-facing documentation, just have a few other things to wrap up. If you'd like to see the documentation for how to use the override feature (as it stands today, it may change during PR review of course!), you can check it out here.