Skip to content

Commit

Permalink
Remove no-longer-used MTRDevice logic for truncating data version lists
Browse files Browse the repository at this point in the history 
After project-chip#34111, ReadClient
handles this logic itself, so the attempted truncation in MTRDevice was now dead
code.
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple committed Jul 3, 2024
1 parent 5a6d0f9 commit 493c5f8
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 75 deletions.
5 changes: 5 additions & 0 deletions src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ CHIP_ERROR ReadClient::BuildDataVersionFilterList(DataVersionFilterIBs::Builder
const Span<DataVersionFilter> & aDataVersionFilters,
bool & aEncodedDataVersionList)
{
ChipLogProgress(DataManagement, "Attempting to encode %lu data version filters", static_cast<unsigned long>(aDataVersionFilters.size()));
for (auto & filter : aDataVersionFilters)
{
VerifyOrReturnError(filter.IsValidDataVersionFilter(), CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -434,6 +435,10 @@ CHIP_ERROR ReadClient::BuildDataVersionFilterList(DataVersionFilterIBs::Builder
{
// Packet is full, ignore the rest of the list
aDataVersionFilterIBsBuilder.Rollback(backup);
ssize_t nonSkippedFilters = &filter - aDataVersionFilters.data();
size_t skippedFilters = aDataVersionFilters.size() - static_cast<size_t>(nonSkippedFilters);
ChipLogProgress(DataManagement, "Skipped encoding %lu out of %lu data version filters due to lack of space",
static_cast<unsigned long>(skippedFilters), static_cast<unsigned long>(aDataVersionFilters.size()));
return CHIP_NO_ERROR;
}
else
Expand Down
127 changes: 52 additions & 75 deletions src/darwin/Framework/CHIP/MTRDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2223,17 +2223,16 @@ - (void)_removeCachedAttribute:(NSNumber *)attributeID fromCluster:(MTRClusterPa
[clusterData removeValueForAttribute:attributeID];
}

- (void)_createDataVersionFilterListFromDictionary:(NSDictionary<MTRClusterPath *, NSNumber *> *)dataVersions dataVersionFilterList:(DataVersionFilter **)dataVersionFilterList count:(size_t *)count sizeReduction:(size_t)sizeReduction
- (void)_createDataVersionFilterListFromDictionary:(NSDictionary<MTRClusterPath *, NSNumber *> *)dataVersions dataVersionFilterList:(DataVersionFilter **)dataVersionFilterList count:(size_t *)count
{
size_t maxDataVersionFilterSize = dataVersions.count;

// Check if any filter list should be generated
if (!dataVersions.count || (maxDataVersionFilterSize <= sizeReduction)) {
if (!dataVersions.count) {
*count = 0;
*dataVersionFilterList = nullptr;
return;
}
maxDataVersionFilterSize -= sizeReduction;

DataVersionFilter * dataVersionFilterArray = new DataVersionFilter[maxDataVersionFilterSize];
size_t i = 0;
Expand All @@ -2242,13 +2241,13 @@ - (void)_createDataVersionFilterListFromDictionary:(NSDictionary<MTRClusterPath
if (dataVersionNumber) {
dataVersionFilterArray[i++] = DataVersionFilter(static_cast<chip::EndpointId>(path.endpoint.unsignedShortValue), static_cast<chip::ClusterId>(path.cluster.unsignedLongValue), static_cast<chip::DataVersion>(dataVersionNumber.unsignedLongValue));
}
if (i == maxDataVersionFilterSize) {
break;
}
}

*dataVersionFilterList = dataVersionFilterArray;
*count = maxDataVersionFilterSize;
// Note that we might have i < maxDataVersionFilterSize here if some of the
// dictionary entries had a null dataVersionNumber. The correct size of the
// valid entried in our array is "i".
*count = i;
}

- (void)_setupConnectivityMonitoring
Expand Down Expand Up @@ -2443,75 +2442,55 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
auto readClient = std::make_unique<ReadClient>(InteractionModelEngine::GetInstance(), exchangeManager,
clusterStateCache->GetBufferedCallback(), ReadClient::InteractionType::Subscribe);

// Subscribe with data version filter list and retry with smaller list if out of packet space
CHIP_ERROR err;
NSDictionary<MTRClusterPath *, NSNumber *> * dataVersions = [self _getCachedDataVersions];
size_t dataVersionFilterListSizeReduction = 0;
for (;;) {
// Wildcard endpoint, cluster, attribute, event.
auto attributePath = std::make_unique<AttributePathParams>();
auto eventPath = std::make_unique<EventPathParams>();
// We want to get event reports at the minInterval, not the maxInterval.
eventPath->mIsUrgentEvent = true;
ReadPrepareParams readParams(session.Value());

readParams.mMinIntervalFloorSeconds = 0;
// Select a max interval based on the device's claimed idle sleep interval.
auto idleSleepInterval = std::chrono::duration_cast<System::Clock::Seconds32>(
session.Value()->GetRemoteMRPConfig().mIdleRetransTimeout);

auto maxIntervalCeilingMin = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN);
if (idleSleepInterval < maxIntervalCeilingMin) {
idleSleepInterval = maxIntervalCeilingMin;
}

auto maxIntervalCeilingMax = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX);
if (idleSleepInterval > maxIntervalCeilingMax) {
idleSleepInterval = maxIntervalCeilingMax;
}
// Wildcard endpoint, cluster, attribute, event.
auto attributePath = std::make_unique<AttributePathParams>();
auto eventPath = std::make_unique<EventPathParams>();
// We want to get event reports at the minInterval, not the maxInterval.
eventPath->mIsUrgentEvent = true;
ReadPrepareParams readParams(session.Value());

readParams.mMinIntervalFloorSeconds = 0;
// Select a max interval based on the device's claimed idle sleep interval.
auto idleSleepInterval = std::chrono::duration_cast<System::Clock::Seconds32>(
session.Value()->GetRemoteMRPConfig().mIdleRetransTimeout);

auto maxIntervalCeilingMin = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN);
if (idleSleepInterval < maxIntervalCeilingMin) {
idleSleepInterval = maxIntervalCeilingMin;
}

auto maxIntervalCeilingMax = System::Clock::Seconds32(MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX);
if (idleSleepInterval > maxIntervalCeilingMax) {
idleSleepInterval = maxIntervalCeilingMax;
}
#ifdef DEBUG
if (maxIntervalOverride.HasValue()) {
idleSleepInterval = maxIntervalOverride.Value();
}
#endif
readParams.mMaxIntervalCeilingSeconds = static_cast<uint16_t>(idleSleepInterval.count());

readParams.mpAttributePathParamsList = attributePath.get();
readParams.mAttributePathParamsListSize = 1;
readParams.mpEventPathParamsList = eventPath.get();
readParams.mEventPathParamsListSize = 1;
readParams.mKeepSubscriptions = true;
readParams.mIsFabricFiltered = false;
size_t dataVersionFilterListSize = 0;
DataVersionFilter * dataVersionFilterList;
[self _createDataVersionFilterListFromDictionary:dataVersions dataVersionFilterList:&dataVersionFilterList count:&dataVersionFilterListSize sizeReduction:dataVersionFilterListSizeReduction];
readParams.mDataVersionFilterListSize = dataVersionFilterListSize;
readParams.mpDataVersionFilterList = dataVersionFilterList;
attributePath.release();
eventPath.release();

// TODO: Change from local filter list generation to rehydrating ClusterStateCache ot take advantage of existing filter list sorting algorithm

// SendAutoResubscribeRequest cleans up the params, even on failure.
err = readClient->SendAutoResubscribeRequest(std::move(readParams));
if (err == CHIP_NO_ERROR) {
break;
}

// If error is not a "no memory" issue, then break and go through regular resubscribe logic
if (err != CHIP_ERROR_NO_MEMORY) {
break;
}

// If "no memory" error is not caused by data version filter list, break as well
if (!dataVersionFilterListSize) {
break;
}

// Now "no memory" could mean subscribe request packet space ran out. Reduce size and try again immediately
dataVersionFilterListSizeReduction++;
if (maxIntervalOverride.HasValue()) {
idleSleepInterval = maxIntervalOverride.Value();
}
#endif
readParams.mMaxIntervalCeilingSeconds = static_cast<uint16_t>(idleSleepInterval.count());

readParams.mpAttributePathParamsList = attributePath.get();
readParams.mAttributePathParamsListSize = 1;
readParams.mpEventPathParamsList = eventPath.get();
readParams.mEventPathParamsListSize = 1;
readParams.mKeepSubscriptions = true;
readParams.mIsFabricFiltered = false;

// Subscribe with data version filter list from our cache.
size_t dataVersionFilterListSize = 0;
DataVersionFilter * dataVersionFilterList;
[self _createDataVersionFilterListFromDictionary:[self _getCachedDataVersions] dataVersionFilterList:&dataVersionFilterList count:&dataVersionFilterListSize];

readParams.mDataVersionFilterListSize = dataVersionFilterListSize;
readParams.mpDataVersionFilterList = dataVersionFilterList;
attributePath.release();
eventPath.release();

// TODO: Change from local filter list generation to rehydrating ClusterStateCache to take advantage of existing filter list sorting algorithm

// SendAutoResubscribeRequest cleans up the params, even on failure.
CHIP_ERROR err = readClient->SendAutoResubscribeRequest(std::move(readParams));
if (err != CHIP_NO_ERROR) {
NSError * error = [MTRError errorForCHIPErrorCode:err logContext:self];
MTR_LOG_ERROR("%@ SendAutoResubscribeRequest error %@", self, error);
Expand All @@ -2523,8 +2502,6 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
return;
}

MTR_LOG("%@ Subscribe with data version list size %lu, reduced by %lu", self, static_cast<unsigned long>(dataVersions.count), static_cast<unsigned long>(dataVersionFilterListSizeReduction));

// Callback and ClusterStateCache and ReadClient will be deleted
// when OnDone is called.
os_unfair_lock_lock(&self->_lock);
Expand Down

0 comments on commit 493c5f8

Please sign in to comment.