-
-
Notifications
You must be signed in to change notification settings - Fork 35k
Add mac address as connection for matter device #121257
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
Add mac address as connection for matter device #121257
Conversation
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.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Hey there @home-assistant/matter, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
38efa9c
to
11a296d
Compare
I've marked this PR, as changes are requested that need to be processed. Thanks! 👍 ../Frenck |
11a296d
to
e3f84fd
Compare
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.
Nice work!
@agners Oh no, that doesn't look right. I suspect the devices all share the invalid |
Yeah I have at least two devices which return I guess technically, this is a valid MAC address. But these devices don't use this as physical address (according to my AP). Technically, the Matter standard says Anyhow, given that Matter devices return such addresses, this is more a Matter problem. I'd suggest to filter them out on Matter integration side. |
Btw, what is the exact use case/motivation behind this PR? Do you have a device where this connects up with other integrations or something? I am a bit worried since MAC address privacy extensions become more and more commonplace. The Matter standard also mentions that when it comes to host name generation. It seems we have to expect that devices rotate MAC addresses:
|
Unfortunately, I can't work on this today. @agners I still have the checks that I removed here: def is_NULL(self) -> bool:
"""Check if the address is NULL value/should not be used.
See: https://standards.ieee.org/wp-content/uploads/import/documents/tutorials/eui.pdf
'Values based on a zero-valued OUI [...] shall not be used as identifiers.'
"""
return self.address[:3] == b"\x00\x00\x00"
def is_individual(self) -> bool:
"""Check if the address is an individual address, not a group address."""
return self.address[0] & 0x01 == 0
def _valid_connection(self) -> bool:
return self.is_individual() and not self.is_NULL() |
Oh I see. I just looked up OUI, and with 00:00:00 Xerox came up, so assumed it is valid. |
@agners I have a matter device that wants to connect to the manufacturer to make auto-updates, which I don't want. My router integration adds a switch for every connected device to block its internet access. And it annoys me that I regularly have duplicate devices in my list. |
@agners You are right to point out that there will be more devices with address rotation in the future, which could be an issue for mac addresses to be used as unique device identifiers. However, this should not cause any severe problems in HA. Also, as I understand it, the addresses are only stored until the integration that provides the entity reloads. So even when the addresses rotate, it wouldn't spam the device registry with addresses. (Also, I guess that most Matter devices won't move from network to network regularly, and therefore manufacturers won't bother with implementing privacy enhancing rotating addresses.) |
@marcelveldt Another issue. This time it's an issue with the frontend. The Fritz!Box integration sets This breaks this frontend code: export const getMatterDeviceActions = async (
el: HTMLElement,
hass: HomeAssistant,
device: DeviceRegistryEntry
): Promise<DeviceAction[]> => {
if (device.via_device_id !== null) {
// only show device actions for top level nodes (so not bridged)
return [];
}
// ... The semantic of https://developers.home-assistant.io/docs/device_registry_index/#what-is-a-device
This sounds like the Fritz!Box integration shouldn't set But https://developers.home-assistant.io/docs/device_registry_index#device-properties states:
And this WIFI-Router is definitely "a device that routes messages between this device and Home Assistant". |
Hm, in the Matter integration we use via_device for bridges. This is a single Matter node, which generates multiple devices: one root device (the bridge), and multiple bridged devices. The latter all have via_device set. We definitely make assumption that those are all are part of one Matter node, e.g. when deleting the bridge device. But I guess that still should be fine as the device which makes trouble here is from another integration? 🤔 |
That assumption that a device is a bridged matter device if it has That might be against the original idea of This is a hard problem to solve, when you think about that lower network layers are transparent to the upper levels. Neither my SmartPlug nor my Home Assistant can tell how the two are connected. The only chance you have is that if every device involved shouts: "I know that one, it's connected through me!" That's what the Fritz and the Philips Hue integration are doing. Thinking about meshed networks like in this graphic: What would be the |
IMHO, |
We are going to revert this PR because mac address is not a stable identifier and it has proven to cause side effects. |
And about via_device: that is designed in HA for bridges/hubs and nothing else. So the current implementation in Matter, Hue etc. is correct. |
If you think the architecture needs to be changed, you can file an architecture proposal in our architecture repository. |
I agree, that this PR should be reverted (for the moment), but I don't agree that Mac addresses not being stable identifiers. (I mean they are called "Extended Unique Identifiers" and are standardized by the IEEE since forever.) The issue in #121257 (comment) comes from insufficient filtering by the HA core of addresses that are specified not to be used as identifiers. And my issue in #121257 (comment) comes from the Fritz!Box integration incorrectly setting the I think that being able to merge devices in HA is an important organizational feature, and unfortunately there is no general solution to do this manually yet. (Although there is some progress. See the Blog Post for version 2024.7) But you are right, this is not the place to discuss this.
I guess it could be better documented. Currently, the docs say:
Which would include routers, switches, "mesh extenders", ... One could say: The Hue Bridge translates HueBridge API messages into their ZigBee protocol. But a (Wi-Fi) router, thread border router or a mesh extender only acts on the network layer or below. |
This reverts commit 2723ab3.
For now, this PR has been reverted. First the core side needs to be fixed to filter out invalid mac addresses (such as the 00:00:00:00:00) case and then we could try it again but I will keep an eye on the various (cheap) devices how well they set the mac address within the network diagnostics cluster. Somehow I have a bad feeling about this as we already ran into multiple cases where the mac address was just filled with a bogus mac. So step 1 is adjust core to at least filter out the all-zeros mac, maybe we can even find a better test to ensure the mac is 100% valid. Step 2 would be inspecting various matter devices for the mac address and if we feel comfortable, add back the change that was now reverted in Matter. As for "via_device" your conclusion is correct and this should maybe be better defined in our docs. |
@marcelveldt Regarding the validation of addresses suitable for identification, I opened a discussion in the architecture repo: home-assistant/architecture#1113 |
Proposed change
This PR adds a mac address connection for matter devices that have a mac address, so that they can be matched with existing devices in the registry.
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: