Remove no-longer-used MTRDevice logic for truncating data version lists

After project-chip#34111, ReadClient
handles this logic itself, so the attempted truncation in MTRDevice was now dead
code.

Author: bzbarsky-apple

src/app/ReadClient.cpp

@@ -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);
@@ -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

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -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;
@@ -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
@@ -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);
@@ -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);