-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
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
Implement base for MQTT device registry integration #16943
Conversation
tests/components/sensor/test_mqtt.py
Outdated
assert device.name == 'Beer' | ||
assert device.model == 'Glass' | ||
assert device.sw_version == '0.1-beta' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line at end of file
vol.Optional(CONF_CONNECTIONS, default=list): | ||
vol.All(cv.ensure_list, [vol.Schema({ | ||
vol.Required(CONF_TYPE): vol.Any(*device_registry.CONNECTION_KEYS), | ||
vol.Required(CONF_IDENTIFIER): cv.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like the name of this option (identifier
) in this context. Its name (base off of https://developers.home-assistant.io/docs/en/device_registry_index.html) is very close to the identifiers
option but it has a totally different format. Should this be renamed to value
or should it rather stick to the dev docs' identifier
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a dict to begin with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well primarily to make it more consistent with the device registry naming. One option would be:
{
...
"device": {
"connections": {
"mac": "F7:19:18:29:93:1A",
"other": "connection_id"
}
}
}
but that would for example not support devices with multiple MAC addresses (probably doesn't come up too often, but I think it's good to support it from the start to prevent breaking changes).
The other option would be:
{
...
"device": {
"connections": [
{"mac": "F7:19:18:29:93:1A"},
{"mac": "E8:2A:29:3A:A4:2B"}
]
}
}
or:
{
...
"device": {
"connections": [
["mac", "F7:19:18:29:93:1A"],
["mac", "E8:2A:29:3A:A4:2B"]
]
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the last one. That is also how we store it internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, updated to that.
from homeassistant.core import Event, ServiceCall, callback | ||
from homeassistant.exceptions import HomeAssistantError | ||
from homeassistant.helpers import config_validation as cv | ||
from homeassistant.helpers import config_validation as cv, device_registry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'homeassistant.helpers.device_registry' imported but unused
Description:
Now after #16918, we can start implementing the device registry for MQTT platforms. This PR adds a new parameter to the MQTT config of
sensor
s:device
. With this, MQTT-enabled devices can send along adevice:
section in the MQTT discovery payload which uniquely identifies the device.This PR only adds the device registry feature to the
sensor.mqtt
platform as a sample implementation, but others will follow once this PR is done.A discovery payload could for example look like this:
Please note that this only works with devices that supply a
unique_id
and only through MQTT discovery.Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#6513
Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices: