Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Incorporate Home Assistant device registry attributes #646

Closed
cbrherms opened this issue Dec 2, 2018 · 19 comments
Closed
Labels
stale Stale issues

Comments

@cbrherms
Copy link
Contributor

cbrherms commented Dec 2, 2018

As of homeassistant 0.81 we can now see entities grouped together as their devices within the front end when they are added using MQTT discovery.

If some further attributes can be added to Mqtt messages when the homeassistant option is enabled then this feature could be implemented.
The documentation for it is here:
https://developers.home-assistant.io/docs/en/device_registry_index.html

Mac address of device can be unique identifier, plus we could provide the device type and manufacturer.

@cbrherms
Copy link
Contributor Author

cbrherms commented Dec 2, 2018

Put together how i'd think the mqtt config payload would likely output using the pressure entity from a Xiaomi temp/pressure/humidity sensor I have as an example:

{
  "unit_of_measurement": "hPa",
  "device_class": "pressure",
  "value_template": "{{ value_json.pressure }}",
  "json_attributes": [
    "linkquality",
    "battery",
    "voltage"
  ],
  "state_topic": "zigbee2mqtt/0x00158d00023cfa23",
  "availability_topic": "zigbee2mqtt/bridge/state",
  "name": "livingroom_xiaomi_multisensor_pressure",
  "unique_id": "0x00158d00023cfa23_pressure_zigbee2mqtt"
  "device": {
    "identifiers": "0x00158d00023cfa23",
    "name": "Livingroom Xiaomi Multisensor",
    "sw_version": "Zigbee2mqtt 0.2.0",
    "model": "WSDCGQ11LM",
    "manufacturer": "Xiaomi"
}

One problem I can see though would be the unique name / friendly name for the entire device that could be used for said device. Believe there would always need to be a unique_id set though at present it's only set if friendly name isn't?

The main thing needed though would just be identifier being deviceID as that is what is used to differentiate between devices and pair up entities with devices within MQTT discovery now.

@Koenkk
Copy link
Owner

Koenkk commented Dec 3, 2018

This should be implemented, but I don't see anything that changed in home assistant. Where should I be looking for?

@cbrherms
Copy link
Contributor Author

cbrherms commented Dec 3, 2018

Hi @Koenkk this was implemented by OttoWinter in commit 16943
It would appear it didn't get as much of a shout about it in the release notes as it should of, as it was multiple commits over various mqtt platforms after that commit (which contains the majority of the info in the description)

@Koenkk
Copy link
Owner

Koenkk commented Dec 3, 2018

I see, but what changes are visible in HA beacuse of this? Does it add any functionallity at all?

@cbrherms
Copy link
Contributor Author

cbrherms commented Dec 3, 2018

HA is moving to a point of allowing more and more components to be set up in the front end (rather than configuration.yaml) via the integrations platform. components set up via this get a nice config front end showing entities that use that integration, and now also grouped in to devices. So, device X has sensor Y and switch Z.

Without the commit you just added that page looked like this:

without device registry

With the single device i'd added via zigbee2mqtt showing under "Entities without devices". As i add more and more that box would grow with all the entities grouped up under the same heading.

I've just installed the edge addon to test that commit and after a restart of homeassistant that page now looks like this:

with device registry

As i add more devices they will then get their own cards listing the entities associated with that device. Much cleaner :D

I'm hoping HA will add a way to edit the names of devices though. As in ESPhomeLib (which the other devices are from) i edit the device name in the config before i write the firmware to the device.

One other aspect of this registry is that you can also add devices without entities, such as hubs, and then link the devices to that hub using the via_hub attribute. I've not seen a working example of that anywhere yet but it's listed at the bottom of the dev entry i linked to in my first post.

Admittedly this is all just to make the integrations page cleaner but i'm assuming it'll be used more and more and get expanded further as the project gets bigger.

@cbrherms
Copy link
Contributor Author

cbrherms commented Dec 3, 2018

Sorry for the wall of comments, but done some further testing of the commit.

This will not currently work if friendly name is set as a uniqueID is not generated and added to the config payload.

UniqueID is required for the entity to be added to the entity registry (or looked up within it), and the device will not be registered if it cannot find the entity in the entity registry.

Without friendly name set, the config payload of the temperature sensor looks like this:

{
  "unit_of_measurement": "°C",
  "device_class": "temperature",
  "value_template": "{{ value_json.temperature }}",
  "json_attributes": [
    "linkquality",
    "battery",
    "voltage"
  ],
  "state_topic": "zigbee2mqtt/0x00158d00023cfa23",
  "availability_topic": "zigbee2mqtt/bridge/state",
  "name": "0x00158d00023cfa23_temperature",
  "unique_id": "0x00158d00023cfa23_temperature_zigbee2mqtt",
  "device": {
    "identifiers": "0x00158d00023cfa23",
    "name": "Aqara temperature, humidity and pressure sensor",
    "sw_version": "test",
    "model": "WSDCGQ11LM",
    "manufacturer": "Xiaomi"
  }
}

The entity is created, looked up in the entity registry, if it doesn't exist it is added, and then the device is added or updated in the device registry and both are associated with each other in the front end.

If i change the friendly name for the device to livingroom_aqara_tph the config payload for the temperature sensor now looks like this:

{
  "unit_of_measurement": "°C",
  "device_class": "temperature",
  "value_template": "{{ value_json.temperature }}",
  "json_attributes": [
    "linkquality",
    "battery",
    "voltage"
  ],
  "state_topic": "zigbee2mqtt/livingroom_aqara_tph",
  "availability_topic": "zigbee2mqtt/bridge/state",
  "name": "livingroom_aqara_tph_temperature",
  "device": {
    "identifiers": "0x00158d00023cfa23",
    "name": "Aqara temperature, humidity and pressure sensor",
    "sw_version": "test",
    "model": "WSDCGQ11LM",
    "manufacturer": "Xiaomi"
  }
}

The entities themselves are created, but are not added to the entity registry as there is no uniqueID, and thus no device is associated with it. Meaning it will not show up in the integrations config page for mqtt discovery.

If #419 is merged that should fix the entity registry issue, allowing the device itself to be registered as well. I don't see how the uniqueID being present would cause any issue with entity creation which i believe was the concern in that pull?

Also, I propose that the device name be changed to the friendly name if that is supplied, to differentiate between multiple connected devices of the same type. It will still work if there are multiple devices of the same name though, so having the default be the description should still be fine.

eg:

{
  "unit_of_measurement": "°C",
  "device_class": "temperature",
  "value_template": "{{ value_json.temperature }}",
  "json_attributes": [
    "linkquality",
    "battery",
    "voltage"
  ],
  "state_topic": "zigbee2mqtt/livingroom_aqara_tph",
  "availability_topic": "zigbee2mqtt/bridge/state",
  "name": "livingroom_aqara_tph_temperature",
  "unique_id": "0x00158d00023cfa23_temperature_zigbee2mqtt",
  "device": {
    "identifiers": "0x00158d00023cfa23",
    "name": "livingroom_aqara_tph",
    "sw_version": "test",
    "model": "WSDCGQ11LM",
    "manufacturer": "Xiaomi"
  }
}

I modified the payloads for each entity via a publish to each ones config path (without restarting zigbee2mqtt) to test and the result was the device showing up with the correct friendly name, and with the entities having the correct friendly name too.

friendly changed

Once again, apologies for the long comments, just trying to be thorough. Also apologies for asking for these changes to be made rather doing them myself but at present I do not have an easily modifiable test setup as i'm running through the hassio addon.

@Koenkk
Copy link
Owner

Koenkk commented Dec 4, 2018

Even with the unique id in the payload, I don't see anything being added:
image
image
image
image

@cbrherms
Copy link
Contributor Author

cbrherms commented Dec 4, 2018

Looks like mqtt light is one component not yet fully integrated with the device registry. PR for that is found here 19013

Also looks like there has been a change with how mqtt light is discovered in mqtt discovery, which might be why the entities themselves aren't appearing though backwards compatibility was apparently implemented? PR to that is here 18227

EDIT:

Ran your example, posting that payload to homeassistant/light/0x000b57fffec86dbb/light/config and ran in to the same issue. Didn't show up in integrations, though did appear in my core.entity_registry.

Had a bit of a look around and does look like it's due to the change i mention above, and that added the ability for an mqtt light to show up in integrations when the new schema attribute is used.
So platform has been dropped from the lights discovery payload and instead schema is used.

Looks like light is the only one to use it within zigbee2mqtt so would have to replace
"platform":"mqtt_json",
with
"schema": "json",

MQTT lights will then get an entity entry in the current public version of HA (though if changed now it'll only appear as a basic light) and also a device entry when 19013 is merged.

schema_change
tradfri_light

I'll submit a PR shortly to add support for lights in the integrations panel.

Also, if you test changing the attribute on the config payload make sure to delete the /config/.storage/core.entity_registry entity with that uniqueID is otherwise when you restart HA it'll create a second entity entry with the same UniqueID and be unable to figure out which one to show in the integrations panel, resulting in nothing showing up.

@cbrherms
Copy link
Contributor Author

cbrherms commented Dec 5, 2018

added #656

@stale
Copy link

stale bot commented Feb 4, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale issues label Feb 4, 2019
@stoinov
Copy link
Contributor

stoinov commented Mar 4, 2019

@cbrherms and @Koenkk sorry I missed the fun discussion last year, but just wanted to understand the reasoning behind having unique_id be like 0x00158d0002321010_pressure_zigbee2mqtt?
Why is the base_topic added at the end? Is there a case where you might have overlap of devices that use the same unique ID of the ZigBee device and the specified item that is being reported outside of ZigBee2MQTT? I really can't imagine it.

I'm just asking since I am using somewhat of a complicated naming scheme and the base_topic is like place/location/position which adds slashes to the name and apart from being unsightly, could cause problems down the road if it's not escaped properly.

@Koenkk
Copy link
Owner

Koenkk commented Mar 4, 2019

As far as I remember there is not a real reason for this, perhaps we should always use zigbee2mqtt instead of the base topic. (but this will be a breaking change for users that are not using zigbee2mqtt as the base topic).

@stoinov
Copy link
Contributor

stoinov commented Mar 4, 2019

The breaking change is the reason why I am regretful that I haven't been discussing this with you last year.

I don't even see a reason for having this value hardcoded - it only adds to the length but does not make it more unique than it already is. And given that in the HA documentation they explicitly try to make the MQTT payloads as small as possible by providing aliases and way to substitute base_topics, I was thinking that this should be a common goal for this project too.

@Koenkk
Copy link
Owner

Koenkk commented Mar 4, 2019

I completely agree, it was a wrong decision to include it.

@stoinov
Copy link
Contributor

stoinov commented Mar 4, 2019

While we're at it - the "identifiers" field also includes hardcoded zigbee2mqtt which I think I understand the thinking behind it, but ultimately does not really help with anything and could just require more work if you try to filter based on the values.
if you want to keep it I think it should be best implemented as array entry, but given that SW already has reference to the project, I don't see the value too.

@Koenkk
Copy link
Owner

Koenkk commented Mar 4, 2019

The identifiers thing is not a problem, the real pain is the unique_id using the base topic (there wouldn't be a problem if this always used zigbee2mqtt instead of the base topic.

@stoinov
Copy link
Contributor

stoinov commented Mar 4, 2019

I understand your concern that this might cause unexpected behavior if and when someone updates his base_topic. I was just pointing out the places which have superfluous information that is not strictly required and could be problematic in certain situations.
For instance I am trying to get all my sensor data into external influxDB, but those additional strings are requiring some additional sanitization in order to make them uniform and ease the queries.

@stoinov
Copy link
Contributor

stoinov commented Mar 10, 2019

As a workaround I am using the newly introduced #1030 to overwrite the unique_id value.

@stoinov
Copy link
Contributor

stoinov commented Mar 10, 2019

the latest change allows me to overwrite identifier too:

devices:
  '0x00158d0000000000':
    friendly_name: room/device/sensor
    retain: false
    homeassistant:
      device:
        identifiers: '0x00158d0000000000'
        name: 'Room Sensor'
      temperature:
        name: 'Room Sensor Temperature'
        unique_id: room_sensor_temperature'

qm3ster pushed a commit to zigbeer/zigbeer that referenced this issue Mar 11, 2019
…kk/zigbee2mqtt#646"

This reverts commit 7ea1fc66c27f1ff7fb88df66f288dc28f0228289.
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issues
Projects
None yet
Development

No branches or pull requests

3 participants