Skip to content

Comments

Improve Matter DNS-SD service parsing#1144

Merged
agners merged 4 commits intomainfrom
improve-dns-sd-service-parsing
May 8, 2025
Merged

Improve Matter DNS-SD service parsing#1144
agners merged 4 commits intomainfrom
improve-dns-sd-service-parsing

Conversation

@agners
Copy link
Collaborator

@agners agners commented May 7, 2025

Services are now parsed in a more robust way, to avoid issues in cases where the service name is not exactly as expected. This has been observed to be the case for Thread devices, presumably because two Thread border routers attempt to announce the same service name. The service name then becomes a " (2)" suffix, which the old code did not handle well.

We could parse this duplicated messages as well, but generally only a single node should ever announce a service name. If a service name gets announced by two independent devices, the two services likely stem from the same underlying device but simply got duplicated by some environmental factor (e.g. Thread border routers not being in sync on who announces the service name, or badly configured mDNS reflectors duplicating messages on the same network). Only processing the correctly formatted service name is sufficient in these cases.

@agners
Copy link
Collaborator Author

agners commented May 7, 2025

This prevents errors like this:

2025-04-24 13:46:58.616 (MainThread) ERROR [matter_server.server] Error doing task: Exception in callback MatterDeviceController._on_mdns_operational_node_state('A3604AC231F8...r._tcp.local.', <ServiceState...ge.Updated: 3>)
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/asyncio/events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/local/lib/python3.12/site-packages/matter_server/server/device_controller.py", line 1544, in _on_mdns_operational_node_state
    node_id = int(name.split("-")[1].split(".")[0], 16)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: invalid literal for int() with base 16: '00000000000000AD (6)'

Copy link
Collaborator

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

BTW: At some point we should write a simple test to test a bunch of dns names against this regex

@agners agners added maintenance Code (quality) improvement or small enhancement which not a new feature bugfix Pull request that fixes a (known) issue/bug labels May 7, 2025
agners and others added 3 commits May 8, 2025 09:27
Services are now parsed in a more robust way, to avoid issues in cases
where the service name is not exactly as expected. This has been
observed to be the case for Thread devices, presumably because two
Thread border routers attempt to announce the same service name.
The service name then becomes a " (2)" suffix, which the old code
did not handle well.

We could parse this duplicated messages as well, but generally only
a single node should ever announce a service name. If a service name
gets announced by two independent devices, the two services likely
stem from the same underlying device but simply got duplicated by
some environmental factor (e.g. Thread border routers not being in
sync on who announces the service name, or badly configured mDNS
reflectors duplicating messages on the same network). Only processing
the correctly formatted service name is sufficient in these cases.
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
@agners agners force-pushed the improve-dns-sd-service-parsing branch from a288836 to 7499a7b Compare May 8, 2025 07:27
Instead of relying on pre-commit to pass all filenames, call pylint
explicitly on the directories we want to check. This avoids that
pylint potentially adds directories to the Python path, leading to
unexpected imports (e.g. Module 'json' has no 'load' member, when
pylint gets called with `pylint matter_server/server/ota/__init__.py matter_server/common/helpers/util.py`).
@agners agners force-pushed the improve-dns-sd-service-parsing branch from 1581483 to 3a6489b Compare May 8, 2025 08:50
@agners agners merged commit 747a320 into main May 8, 2025
4 checks passed
@agners agners deleted the improve-dns-sd-service-parsing branch May 8, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a (known) issue/bug maintenance Code (quality) improvement or small enhancement which not a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants