Skip to content

Commit

Permalink
[Darwin] MTRDeviceController to limit concurrent subscriptions to Thr…
Browse files Browse the repository at this point in the history 
…ead-enabled devices (#33472)

* [Darwin] MTRDeviceController to limit concurrent subscriptions to Thread-enabled devices

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Addressed review comments and added unit test

* Update darwin CI workflow to match unit test

* Chagned Thread-enabled check to use NetworkCommissioning feature map, and fix affected unit test

* Fix compiler warning

* Update src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm

* Fix affected unit test

* Update MTRDevice.mm

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update MTRDeviceControllerParameters.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update MTRPerControllerStorageTests.m

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update MTRPerControllerStorageTests.m

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDevice.mm

* Update src/darwin/Framework/CHIP/MTRDevice.mm

* Update src/darwin/Framework/CHIP/MTRDevice.mm

* Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m

* Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h

* Restyled by whitespace

* Restyled by clang-format

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Updating name

---------

Co-authored-by: Justin Wood <woody@apple.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
@jtung-apple @woody-apple
@bzbarsky-apple @restyled-commits @pull
4 people authored and pull[bot] committed Jun 23, 2024
1 parent 548d0c8 commit 3778958
Show file tree
Hide file tree
Showing 13 changed files with 484 additions and 69 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/darwin.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ jobs:
echo "This is a simple log" > /tmp/darwin/framework-tests/end_user_support_log.txt
../../../out/debug/chip-all-clusters-app --interface-id -1 --end_user_support_log /tmp/darwin/framework-tests/end_user_support_log.txt > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
../../../out/debug/chip-all-clusters-app --interface-id -1 --dac_provider ../../../credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json --product-id 32768 --discriminator 3839 --secured-device-port 5539 --KVS /tmp/chip-all-clusters-app-kvs2 > >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid-err.log >&2) &
../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 101 --passcode 1001 --KVS /tmp/chip-all-clusters-app-kvs101 --secured-device-port 5531 &
../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 102 --passcode 1002 --KVS /tmp/chip-all-clusters-app-kvs102 --secured-device-port 5532 &
../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 103 --passcode 1003 --KVS /tmp/chip-all-clusters-app-kvs103 --secured-device-port 5533 &
../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 104 --passcode 1004 --KVS /tmp/chip-all-clusters-app-kvs104 --secured-device-port 5534 &
../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 105 --passcode 1005 --KVS /tmp/chip-all-clusters-app-kvs105 --secured-device-port 5535 &
export TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1
Expand Down
9 changes: 2 additions & 7 deletions src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
mClusterStateCache = std::move(aClusterStateCache);
}

// Used to reset Resubscription backoff on events that indicate likely availability of device to come back online
void ResetResubscriptionBackoff() { mResubscriptionNumRetries = 0; }

protected:
// Report an error, which may be due to issues in our own internal state or
// due to the OnError callback happening.
Expand Down Expand Up @@ -147,6 +144,8 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
NSMutableArray * _Nullable mAttributeReports = nil;
NSMutableArray * _Nullable mEventReports = nil;

void CallResubscriptionScheduledHandler(NSError * error, NSNumber * resubscriptionDelay);

private:
DataReportCallback _Nullable mAttributeReportCallback = nil;
DataReportCallback _Nullable mEventReportCallback = nil;
Expand Down Expand Up @@ -181,10 +180,6 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
bool mHaveQueuedDeletion = false;
OnDoneHandler _Nullable mOnDoneHandler = nil;
dispatch_block_t mInterimReportBlock = nil;

// Copied from ReadClient and customized for
uint32_t ComputeTimeTillNextSubscription();
uint32_t mResubscriptionNumRetries = 0;
};

NS_ASSUME_NONNULL_END
46 changes: 9 additions & 37 deletions src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,57 +125,29 @@

void MTRBaseSubscriptionCallback::OnSubscriptionEstablished(SubscriptionId aSubscriptionId)
{
// ReadClient resets it at ProcessSubscribeResponse after calling OnSubscriptionEstablished, so this is equivalent
mResubscriptionNumRetries = 0;
if (mSubscriptionEstablishedHandler) {
auto subscriptionEstablishedHandler = mSubscriptionEstablishedHandler;
subscriptionEstablishedHandler();
}
}

uint32_t MTRBaseSubscriptionCallback::ComputeTimeTillNextSubscription()
CHIP_ERROR MTRBaseSubscriptionCallback::OnResubscriptionNeeded(ReadClient * apReadClient, CHIP_ERROR aTerminationCause)
{
uint32_t maxWaitTimeInMsec = 0;
uint32_t waitTimeInMsec = 0;
uint32_t minWaitTimeInMsec = 0;

if (mResubscriptionNumRetries <= CHIP_RESUBSCRIBE_MAX_FIBONACCI_STEP_INDEX) {
maxWaitTimeInMsec = GetFibonacciForIndex(mResubscriptionNumRetries) * CHIP_RESUBSCRIBE_WAIT_TIME_MULTIPLIER_MS;
} else {
maxWaitTimeInMsec = CHIP_RESUBSCRIBE_MAX_RETRY_WAIT_INTERVAL_MS;
}
CHIP_ERROR err = ClusterStateCache::Callback::OnResubscriptionNeeded(apReadClient, aTerminationCause);
ReturnErrorOnFailure(err);

if (maxWaitTimeInMsec != 0) {
minWaitTimeInMsec = (CHIP_RESUBSCRIBE_MIN_WAIT_TIME_INTERVAL_PERCENT_PER_STEP * maxWaitTimeInMsec) / 100;
waitTimeInMsec = minWaitTimeInMsec + (Crypto::GetRandU32() % (maxWaitTimeInMsec - minWaitTimeInMsec));
}

return waitTimeInMsec;
auto error = [MTRError errorForCHIPErrorCode:aTerminationCause];
auto delayMs = @(apReadClient->ComputeTimeTillNextSubscription());
CallResubscriptionScheduledHandler(error, delayMs);
return CHIP_NO_ERROR;
}

CHIP_ERROR MTRBaseSubscriptionCallback::OnResubscriptionNeeded(ReadClient * apReadClient, CHIP_ERROR aTerminationCause)
void MTRBaseSubscriptionCallback::CallResubscriptionScheduledHandler(NSError * error, NSNumber * resubscriptionDelay)
{
// No need to check ReadClient internal state is Idle because ReadClient only calls OnResubscriptionNeeded after calling ClearActiveSubscriptionState(), which sets the state to Idle.

// This part is copied from ReadClient's DefaultResubscribePolicy:
auto timeTillNextResubscription = ComputeTimeTillNextSubscription();
ChipLogProgress(DataManagement,
"Will try to resubscribe to %02x:" ChipLogFormatX64 " at retry index %" PRIu32 " after %" PRIu32
"ms due to error %" CHIP_ERROR_FORMAT,
apReadClient->GetFabricIndex(), ChipLogValueX64(apReadClient->GetPeerNodeId()), mResubscriptionNumRetries, timeTillNextResubscription,
aTerminationCause.Format());
ReturnErrorOnFailure(apReadClient->ScheduleResubscription(timeTillNextResubscription, NullOptional, aTerminationCause == CHIP_ERROR_TIMEOUT));

// Not as good a place to increment as when resubscription timer fires, but as is, this should be as good, because OnResubscriptionNeeded is only called from ReadClient's Close() while Idle, and nothing should cause this to happen
mResubscriptionNumRetries++;

if (mResubscriptionCallback != nil) {
auto callback = mResubscriptionCallback;
auto error = [MTRError errorForCHIPErrorCode:aTerminationCause];
auto delayMs = @(apReadClient->ComputeTimeTillNextSubscription());
callback(error, delayMs);
callback(error, resubscriptionDelay);
}
return CHIP_NO_ERROR;
}

void MTRBaseSubscriptionCallback::OnUnsolicitedMessageFromPublisher(ReadClient *)
Expand Down
Loading

0 comments on commit 3778958

Please sign in to comment.