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

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

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
95cd7c5
[Darwin] MTRDeviceController to limit concurrent subscriptions to Thr…
jtung-apple May 15, 2024
f0fe96f
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 15, 2024
1e5e6c6
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 15, 2024
3193dd8
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 15, 2024
ef228df
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 15, 2024
7d90f68
Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h
woody-apple May 15, 2024
34f86cd
Addressed review comments and added unit test
jtung-apple May 16, 2024
13a16c0
Update darwin CI workflow to match unit test
jtung-apple May 16, 2024
e4668ca
Chagned Thread-enabled check to use NetworkCommissioning feature map,…
jtung-apple May 16, 2024
9c6c68f
Fix compiler warning
jtung-apple May 16, 2024
9b1c0ca
Update src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm
woody-apple May 16, 2024
4d205c4
Fix affected unit test
jtung-apple May 16, 2024
70bb26b
Update MTRDevice.mm
jtung-apple May 16, 2024
31586b3
Update MTRDeviceControllerParameters.h
jtung-apple May 16, 2024
f17ad19
Update MTRPerControllerStorageTests.m
jtung-apple May 16, 2024
c92d8e5
Update MTRPerControllerStorageTests.m
jtung-apple May 16, 2024
adc2f3d
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 16, 2024
83bf054
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 16, 2024
4311b90
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 16, 2024
ed9efe7
Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
woody-apple May 16, 2024
264deab
Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h
woody-apple May 16, 2024
e584201
Restyled by whitespace
restyled-commits May 16, 2024
b8df5fd
Restyled by clang-format
restyled-commits May 16, 2024
6636baf
Update src/darwin/Framework/CHIP/MTRDevice.mm
woody-apple May 16, 2024
81d7b7b
Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h
woody-apple May 16, 2024
ea9597e
Update src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h
woody-apple May 16, 2024
a59d18e
Updating name
woody-apple May 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/darwin.yaml
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 &
woody-apple marked this conversation as resolved.
Show resolved Hide resolved
../../../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
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
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());
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
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
Loading