Skip to content

Commit

Permalink
Merge pull request #93 from home-assistant-libs/make-subscription-set…
Browse files Browse the repository at this point in the history
…up-fail-early

Update Python controller bindings with latest Subscription fixes
  • Loading branch information
agners authored Jul 18, 2024
2 parents e48b141 + 86acde5 commit b7ec5d9
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 0 deletions.
64 changes: 64 additions & 0 deletions 0034-Python-Fix-subscription-error-handling-and-re-subscr.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
From 5ab917909f15fec74778cc2e805b254aaccef1c3 Mon Sep 17 00:00:00 2001
From: Stefan Agner <[email protected]>
Date: Thu, 18 Jul 2024 07:39:38 +0100
Subject: [PATCH] [Python] Fix subscription error handling and re-subscription
(#34372)

* [Python] Fix error callback in AsyncReadTransaction

Currently the error callback is only called when the future is not done
yet and the subscription handler exists. However, the subscription
handler only gets initialized on successful subscription, which is also
where the future gets set to done. So there is no situation where the
error callback is being called, currently.

Fix this by calling the error callback straight from the Matter SDK
Thread when the subscription handler exists. This makes it independent
of the future.

* [Python] Update subscription id on re-subscribe

Make sure we update the subscription ID in the subscription established
callback when the subscription handler already exists. This makes sure
that we have the correct subscription ID stored in the
`SubscriptionTransaction` object after successfully re-subscribe too.
---
src/controller/python/chip/clusters/Attribute.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py
index 31e8444c70..63ca02d8df 100644
--- a/src/controller/python/chip/clusters/Attribute.py
+++ b/src/controller/python/chip/clusters/Attribute.py
@@ -743,6 +743,8 @@ class AsyncReadTransaction:
LOGGER.exception(ex)

def handleError(self, chipError: PyChipError):
+ if self._subscription_handler:
+ self._subscription_handler.OnErrorCb(chipError.code, self._subscription_handler)
self._resultError = chipError

def _handleSubscriptionEstablished(self, subscriptionId):
@@ -751,6 +753,7 @@ class AsyncReadTransaction:
self, subscriptionId, self._devCtrl)
self._future.set_result(self._subscription_handler)
else:
+ self._subscription_handler._subscriptionId = subscriptionId
if self._subscription_handler._onResubscriptionSucceededCb is not None:
if (self._subscription_handler._onResubscriptionSucceededCb_isAsync):
self._event_loop.create_task(
@@ -807,10 +810,7 @@ class AsyncReadTransaction:
#
if not self._future.done():
if self._resultError is not None:
- if self._subscription_handler:
- self._subscription_handler.OnErrorCb(self._resultError.code, self._subscription_handler)
- else:
- self._future.set_exception(self._resultError.to_exception())
+ self._future.set_exception(self._resultError.to_exception())
else:
self._future.set_result(AsyncReadTransaction.ReadResponse(
attributes=self._cache.attributeCache, events=self._events, tlvAttributes=self._cache.attributeTLVCache))
--
2.45.2

63 changes: 63 additions & 0 deletions 0035-Python-Only-auto-re-subscribe-after-initial-subscrip.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
From 1ef35acf92542d14bf34061f8013c678743c12a4 Mon Sep 17 00:00:00 2001
From: Stefan Agner <[email protected]>
Date: Thu, 18 Jul 2024 07:49:05 +0100
Subject: [PATCH] [Python] Only auto re-subscribe after initial subscription
(#34370)

The subscription logic waits for the first successful subscription
before the Read() call is being returned (the future is awaited which
is only released on handleSubscriptionEstablished). If the first
subscription attempt fails (e.g. because the CASE session doesn't
establish) the Read() never returns, not with an error but also not
with a subscription transaction. And since the Python side has no
access to the SubscriptionTransaction object at this point yet,
there is also no way to stop this subscription attempt.

With this change, we only resubscribe if the initial subscription was
successful. This changes semantics slightly, but really allows the
caller to decide if it wants to continue try to establish the
subscription.
---
src/controller/python/chip/clusters/attribute.cpp | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp
index 7c5b2c906a..421284a0ae 100644
--- a/src/controller/python/chip/clusters/attribute.cpp
+++ b/src/controller/python/chip/clusters/attribute.cpp
@@ -145,18 +145,20 @@ public:

void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override
{
+ // Only enable auto resubscribe if the subscription is established successfully.
+ mAutoResubscribeNeeded = mAutoResubscribe;
gOnSubscriptionEstablishedCallback(mAppContext, aSubscriptionId);
}

CHIP_ERROR OnResubscriptionNeeded(ReadClient * apReadClient, CHIP_ERROR aTerminationCause) override
{
- if (mAutoResubscribe)
+ if (mAutoResubscribeNeeded)
{
ReturnErrorOnFailure(ReadClient::Callback::OnResubscriptionNeeded(apReadClient, aTerminationCause));
}
gOnResubscriptionAttemptedCallback(mAppContext, ToPyChipError(aTerminationCause),
apReadClient->ComputeTimeTillNextSubscription());
- if (mAutoResubscribe)
+ if (mAutoResubscribeNeeded)
{
return CHIP_NO_ERROR;
}
@@ -242,7 +244,8 @@ private:
PyObject * mAppContext;

std::unique_ptr<ReadClient> mReadClient;
- bool mAutoResubscribe = true;
+ bool mAutoResubscribe = true;
+ bool mAutoResubscribeNeeded = false;
};

extern "C" {
--
2.45.2

0 comments on commit b7ec5d9

Please sign in to comment.