Skip to content

Commit 86acde5

Browse files
committed
Update Python controller bindings with latest Subscription fixes
This adds Subscription fixes from the master branch to our 1.3 branch. Specifically, this only auto re-subscribes after an initial subscription succeeded. With that the Read call no longer hangs forver in case there is a communication issue with the device. Specifically, this integrates changes from the following PRs - project-chip/connectedhomeip#34370 - project-chip/connectedhomeip#34372
1 parent e48b141 commit 86acde5

2 files changed

+127
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
From 5ab917909f15fec74778cc2e805b254aaccef1c3 Mon Sep 17 00:00:00 2001
2+
From: Stefan Agner <stefan@agner.ch>
3+
Date: Thu, 18 Jul 2024 07:39:38 +0100
4+
Subject: [PATCH] [Python] Fix subscription error handling and re-subscription
5+
(#34372)
6+
7+
* [Python] Fix error callback in AsyncReadTransaction
8+
9+
Currently the error callback is only called when the future is not done
10+
yet and the subscription handler exists. However, the subscription
11+
handler only gets initialized on successful subscription, which is also
12+
where the future gets set to done. So there is no situation where the
13+
error callback is being called, currently.
14+
15+
Fix this by calling the error callback straight from the Matter SDK
16+
Thread when the subscription handler exists. This makes it independent
17+
of the future.
18+
19+
* [Python] Update subscription id on re-subscribe
20+
21+
Make sure we update the subscription ID in the subscription established
22+
callback when the subscription handler already exists. This makes sure
23+
that we have the correct subscription ID stored in the
24+
`SubscriptionTransaction` object after successfully re-subscribe too.
25+
---
26+
src/controller/python/chip/clusters/Attribute.py | 8 ++++----
27+
1 file changed, 4 insertions(+), 4 deletions(-)
28+
29+
diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py
30+
index 31e8444c70..63ca02d8df 100644
31+
--- a/src/controller/python/chip/clusters/Attribute.py
32+
+++ b/src/controller/python/chip/clusters/Attribute.py
33+
@@ -743,6 +743,8 @@ class AsyncReadTransaction:
34+
LOGGER.exception(ex)
35+
36+
def handleError(self, chipError: PyChipError):
37+
+ if self._subscription_handler:
38+
+ self._subscription_handler.OnErrorCb(chipError.code, self._subscription_handler)
39+
self._resultError = chipError
40+
41+
def _handleSubscriptionEstablished(self, subscriptionId):
42+
@@ -751,6 +753,7 @@ class AsyncReadTransaction:
43+
self, subscriptionId, self._devCtrl)
44+
self._future.set_result(self._subscription_handler)
45+
else:
46+
+ self._subscription_handler._subscriptionId = subscriptionId
47+
if self._subscription_handler._onResubscriptionSucceededCb is not None:
48+
if (self._subscription_handler._onResubscriptionSucceededCb_isAsync):
49+
self._event_loop.create_task(
50+
@@ -807,10 +810,7 @@ class AsyncReadTransaction:
51+
#
52+
if not self._future.done():
53+
if self._resultError is not None:
54+
- if self._subscription_handler:
55+
- self._subscription_handler.OnErrorCb(self._resultError.code, self._subscription_handler)
56+
- else:
57+
- self._future.set_exception(self._resultError.to_exception())
58+
+ self._future.set_exception(self._resultError.to_exception())
59+
else:
60+
self._future.set_result(AsyncReadTransaction.ReadResponse(
61+
attributes=self._cache.attributeCache, events=self._events, tlvAttributes=self._cache.attributeTLVCache))
62+
--
63+
2.45.2
64+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
From 1ef35acf92542d14bf34061f8013c678743c12a4 Mon Sep 17 00:00:00 2001
2+
From: Stefan Agner <stefan@agner.ch>
3+
Date: Thu, 18 Jul 2024 07:49:05 +0100
4+
Subject: [PATCH] [Python] Only auto re-subscribe after initial subscription
5+
(#34370)
6+
7+
The subscription logic waits for the first successful subscription
8+
before the Read() call is being returned (the future is awaited which
9+
is only released on handleSubscriptionEstablished). If the first
10+
subscription attempt fails (e.g. because the CASE session doesn't
11+
establish) the Read() never returns, not with an error but also not
12+
with a subscription transaction. And since the Python side has no
13+
access to the SubscriptionTransaction object at this point yet,
14+
there is also no way to stop this subscription attempt.
15+
16+
With this change, we only resubscribe if the initial subscription was
17+
successful. This changes semantics slightly, but really allows the
18+
caller to decide if it wants to continue try to establish the
19+
subscription.
20+
---
21+
src/controller/python/chip/clusters/attribute.cpp | 9 ++++++---
22+
1 file changed, 6 insertions(+), 3 deletions(-)
23+
24+
diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp
25+
index 7c5b2c906a..421284a0ae 100644
26+
--- a/src/controller/python/chip/clusters/attribute.cpp
27+
+++ b/src/controller/python/chip/clusters/attribute.cpp
28+
@@ -145,18 +145,20 @@ public:
29+
30+
void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override
31+
{
32+
+ // Only enable auto resubscribe if the subscription is established successfully.
33+
+ mAutoResubscribeNeeded = mAutoResubscribe;
34+
gOnSubscriptionEstablishedCallback(mAppContext, aSubscriptionId);
35+
}
36+
37+
CHIP_ERROR OnResubscriptionNeeded(ReadClient * apReadClient, CHIP_ERROR aTerminationCause) override
38+
{
39+
- if (mAutoResubscribe)
40+
+ if (mAutoResubscribeNeeded)
41+
{
42+
ReturnErrorOnFailure(ReadClient::Callback::OnResubscriptionNeeded(apReadClient, aTerminationCause));
43+
}
44+
gOnResubscriptionAttemptedCallback(mAppContext, ToPyChipError(aTerminationCause),
45+
apReadClient->ComputeTimeTillNextSubscription());
46+
- if (mAutoResubscribe)
47+
+ if (mAutoResubscribeNeeded)
48+
{
49+
return CHIP_NO_ERROR;
50+
}
51+
@@ -242,7 +244,8 @@ private:
52+
PyObject * mAppContext;
53+
54+
std::unique_ptr<ReadClient> mReadClient;
55+
- bool mAutoResubscribe = true;
56+
+ bool mAutoResubscribe = true;
57+
+ bool mAutoResubscribeNeeded = false;
58+
};
59+
60+
extern "C" {
61+
--
62+
2.45.2
63+

0 commit comments

Comments
 (0)