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

Retry subscription setup if necessary #873

Merged
merged 7 commits into from
Sep 4, 2024
Merged

Conversation

agners
Copy link
Collaborator

@agners agners commented Sep 3, 2024

With the update of the Matter SDK wheels to 2024.7.1 (#809) the SDK no longer auto-resubscribes when the read of the subscription fails (see project-chip/connectedhomeip#34370). The Server had a comment that it relies on mDNS rediscovery to re-establish the subscription. But this is not always the case. This change adds a retry mechanism to the subscription setup for a fixed period of time after seeing it via mDNS.

Additionally, it avoids recreating the subscription if the liveness timeout did not occur yet. This puts more trust into the SDK handling the subscriptions.

Fixes: home-assistant/core#124503

With the update of the Matter SDK wheels to 2024.7.1 (#809) the SDK
no longer auto-resubscribes when the read of the subscription fails
(see project-chip/connectedhomeip#34370).
The Server had a comment that it relies on mDNS rediscovery to
re-establish the subscription. But this is not always the case. This
change adds a retry mechanism to the subscription setup for a fixed
period of time after seeing it via mDNS.

Additionally, it avoids recreating the subscription if the liveness
timeout did not occur yet. This puts more trust into the SDK handling
the subscriptions.
@agners agners added the bugfix Pull request that fixes a (known) issue/bug label Sep 3, 2024
agners and others added 3 commits September 3, 2024 21:12
Use a task in all cases when we (re-)setup the node. Use the task to
also make sure that only a single setup is running at a time. And
finally it allows us to abort any currently running setup task at the
end.
We'll have to implement retry on our end, so use this as the new normal
naming.
Copy link
Contributor

@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.

Looks good!

@agners agners merged commit 68754b0 into main Sep 4, 2024
4 checks passed
@agners agners deleted the retry-subscription-if-necessary branch September 4, 2024 07:46
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Matter - Device state not updating
2 participants