Simplify the logic in writeAttributeWithEndpointID in MTRBaseDevice. (p…

…roject-chip#25230) * Simplify the logic in writeAttributeWithEndpointID in MTRBaseDevice. * Address review comments. Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> --------- Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com>
lecndav · Mar 22, 2023 · d2db8dc · d2db8dc
1 parent d545fc6
commit d2db8dc
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 45 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.h b/src/darwin/Framework/CHIP/MTRBaseDevice.h
@@ -234,8 +234,13 @@ typedef NS_ENUM(uint8_t, MTRTransportType) {
  *
  * @param completion  response handler will receive either values or error.
  *
- *                    Received values are documented in the definition of
- *                    MTRDeviceResponseHandler.
+ *                    A path-specific error status will get turned into an error
+ *                    passed to the completion, so values will only be passed in
+ *                    when the write succeeds.  In that case, values will have
+ *                    the format documented in the definition of
+ *                    MTRDeviceResponseHandler and will be an array with a single element
+ *                    which is a dictionary that has a MTRAttributePathKey entry in it, whose value
+ *                    is the attribute path that was successfully written to.
  */
 - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
                            clusterID:(NSNumber *)clusterID

diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -952,56 +952,21 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
     auto * bridge = new MTRDataValueDictionaryCallbackBridge(queue, completion,
         ^(ExchangeManager & exchangeManager, const SessionHandle & session, MTRDataValueDictionaryCallback successCb,
             MTRErrorCallback failureCb, MTRCallbackBridgeBase * bridge) {
-            auto resultArray = [[NSMutableArray alloc] init];
-            auto resultSuccess = [[NSMutableArray alloc] init];
-            auto resultFailure = [[NSMutableArray alloc] init];
-            auto onSuccessCb = [resultArray, resultSuccess](const app::ConcreteAttributePath & attribPath) {
-                [resultArray addObject:@ { MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:attribPath] }];
-                if ([resultSuccess count] == 0) {
-                    [resultSuccess addObject:[NSNumber numberWithBool:YES]];
-                }
-            };
-
-            auto onFailureCb = [resultArray, resultFailure](const app::ConcreteAttributePath * attribPath, CHIP_ERROR aError) {
-                if (attribPath) {
-                    [resultArray addObject:@ {
-                        MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:*attribPath],
-                        MTRErrorKey : [MTRError errorForCHIPErrorCode:aError],
-                    }];
-                } else {
-                    if ([resultFailure count] == 0) {
-                        [resultFailure addObject:[MTRError errorForCHIPErrorCode:aError]];
-                    }
-                }
+            // Controller::WriteAttribute guarantees that there will be exactly one call to either the success callback or the
+            // failure callback, for a non-group session.
+            auto onSuccessCb = [successCb, bridge](const app::ConcreteAttributePath & attribPath) {
+                auto resultArray = @[ @ { MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:attribPath] } ];
+                successCb(bridge, resultArray);
             };
 
-            auto onDoneCb
-                = [bridge, successCb, failureCb, resultArray, resultSuccess, resultFailure](app::WriteClient * pWriteClient) {
-                      if ([resultFailure count] > 0 || [resultSuccess count] == 0) {
-                          // Failure
-                          if (failureCb) {
-                              if ([resultFailure count] > 0) {
-                                  failureCb(bridge, [MTRError errorToCHIPErrorCode:resultFailure[0]]);
-                              } else if ([resultArray count] > 0) {
-                                  failureCb(bridge, [MTRError errorToCHIPErrorCode:resultArray[0][MTRErrorKey]]);
-                              } else {
-                                  failureCb(bridge, CHIP_ERROR_WRITE_FAILED);
-                              }
-                          }
-                      } else {
-                          // Success
-                          if (successCb) {
-                              successCb(bridge, resultArray);
-                          }
-                      }
-                  };
+            auto onFailureCb = [failureCb, bridge](
+                                   const app::ConcreteAttributePath * attribPath, CHIP_ERROR aError) { failureCb(bridge, aError); };
 
             return chip::Controller::WriteAttribute<MTRDataValueDictionaryDecodableType>(session,
                 static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
                 static_cast<chip::ClusterId>([clusterID unsignedLongValue]),
                 static_cast<chip::AttributeId>([attributeID unsignedLongValue]), MTRDataValueDictionaryDecodableType(value),
-                onSuccessCb, onFailureCb, (timeoutMs == nil) ? NullOptional : Optional<uint16_t>([timeoutMs unsignedShortValue]),
-                onDoneCb, NullOptional);
+                onSuccessCb, onFailureCb, (timeoutMs == nil) ? NullOptional : Optional<uint16_t>([timeoutMs unsignedShortValue]));
         });
     std::move(*bridge).DispatchAction(self);
 }