Make the error handling in readAttributesWithEndpointID more reasonable.

src/darwin/Framework/CHIP/MTRBaseDevice.h

@@ -215,6 +215,11 @@ typedef NS_ENUM(uint8_t, MTRTransportType) {
  *
  * A non-nil attributeID along with a nil clusterID will only succeed if the
  * attribute ID is for a global attribute that applies to all clusters.
+ *
+ * The completion will be called with an error if the entire read interaction fails.
+ * Otherwise it will be called with values, which may be empty (e.g. if no paths
+ * matched the wildcard) or may include per-path errors if particular paths
+ * failed.
  */
 - (void)readAttributesWithEndpointID:(NSNumber * _Nullable)endpointID
                            clusterID:(NSNumber * _Nullable)clusterID
@@ -336,6 +341,11 @@ typedef NS_ENUM(uint8_t, MTRTransportType) {
  *
  * If all of endpointID, clusterID, eventID are nil, all events on the
  * device will be read.
+ *
+ * The completion will be called with an error if the entire read interaction fails.
+ * Otherwise it will be called with values, which may be empty (e.g. if no paths
+ * matched the wildcard) or may include per-path errors if particular paths
+ * failed.
  */
 
 - (void)readEventsWithEndpointID:(NSNumber * _Nullable)endpointID

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -847,31 +847,34 @@ - (void)readAttributesWithEndpointID:(NSNumber * _Nullable)endpointID
     auto * bridge = new MTRDataValueDictionaryCallbackBridge(queue, completion,
         ^(ExchangeManager & exchangeManager, const SessionHandle & session, MTRDataValueDictionaryCallback successCb,
             MTRErrorCallback failureCb, MTRCallbackBridgeBase * bridge) {
+            // interactionStatus tracks whether the whole read interaction has failed.
+            //
+            // Make sure interactionStatus survives even if this block scope is destroyed.
+            auto interactionStatus = std::make_shared<CHIP_ERROR>(CHIP_NO_ERROR);
+
             auto resultArray = [[NSMutableArray alloc] init];
-            auto resultSuccess = [[NSMutableArray alloc] init];
-            auto resultFailure = [[NSMutableArray alloc] init];
-            auto onSuccessCb = [resultArray, resultSuccess](const app::ConcreteClusterPath & clusterPath, const uint32_t aValueId,
+            auto onSuccessCb = [resultArray](const app::ConcreteClusterPath & clusterPath, const uint32_t aValueId,
                                    const MTRDataValueDictionaryDecodableType & aData) {
                 app::ConcreteAttributePath attribPath(clusterPath.mEndpointId, clusterPath.mClusterId, aValueId);
                 [resultArray addObject:@ {
                     MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:attribPath],
                     MTRDataKey : aData.GetDecodedObject()
                 }];
-                if ([resultSuccess count] == 0) {
-                    [resultSuccess addObject:[NSNumber numberWithBool:YES]];
-                }
             };
 
-            auto onFailureCb = [resultArray, resultFailure](
+            auto onFailureCb = [resultArray, interactionStatus](
                                    const app::ConcreteClusterPath * clusterPath, const uint32_t aValueId, CHIP_ERROR aError) {
                 if (clusterPath) {
                     app::ConcreteAttributePath attribPath(clusterPath->mEndpointId, clusterPath->mClusterId, aValueId);
                     [resultArray addObject:@ {
                         MTRAttributePathKey : [[MTRAttributePath alloc] initWithPath:attribPath],
                         MTRErrorKey : [MTRError errorForCHIPErrorCode:aError]
                     }];
-                } else if ([resultFailure count] == 0) {
-                    [resultFailure addObject:[MTRError errorForCHIPErrorCode:aError]];
+                } else {
+                    // This will only happen once per read interaction, and
+                    // after that there will be no more calls to onFailureCb or
+                    // onSuccessCb.
+                    *interactionStatus = aError;
                 }
             };
 
@@ -893,24 +896,14 @@ - (void)readAttributesWithEndpointID:(NSNumber * _Nullable)endpointID
             readParams.mpAttributePathParamsList = &attributePath;
             readParams.mAttributePathParamsListSize = 1;
 
-            auto onDone = [resultArray, resultSuccess, resultFailure, bridge, successCb, failureCb](
+            auto onDone = [resultArray, interactionStatus, bridge, successCb, failureCb](
                               BufferedReadClientCallback<MTRDataValueDictionaryDecodableType> * callback) {
-                if ([resultFailure count] > 0 || [resultSuccess count] == 0) {
+                if (*interactionStatus != CHIP_NO_ERROR) {
                     // 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_READ_FAILED);
-                        }
-                    }
+                    failureCb(bridge, *interactionStatus);
                 } else {
                     // Success
-                    if (successCb) {
-                        successCb(bridge, resultArray);
-                    }
+                    successCb(bridge, resultArray);
                 }
                 chip::Platform::Delete(callback);
             };
@@ -1514,31 +1507,34 @@ - (void)readEventsWithEndpointID:(NSNumber * _Nullable)endpointID
     auto * bridge = new MTRDataValueDictionaryCallbackBridge(queue, completion,
         ^(ExchangeManager & exchangeManager, const SessionHandle & session, MTRDataValueDictionaryCallback successCb,
             MTRErrorCallback failureCb, MTRCallbackBridgeBase * bridge) {
+            // interactionStatus tracks whether the whole read interaction has failed.
+            //
+            // Make sure interactionStatus survives even if this block scope is destroyed.
+            auto interactionStatus = std::make_shared<CHIP_ERROR>(CHIP_NO_ERROR);
+
             auto resultArray = [[NSMutableArray alloc] init];
-            auto resultSuccess = [[NSMutableArray alloc] init];
-            auto resultFailure = [[NSMutableArray alloc] init];
-            auto onSuccessCb = [resultArray, resultSuccess](const app::ConcreteClusterPath & clusterPath, const uint32_t aValueId,
+            auto onSuccessCb = [resultArray](const app::ConcreteClusterPath & clusterPath, const uint32_t aValueId,
                                    const MTRDataValueDictionaryDecodableType & aData) {
                 app::ConcreteEventPath eventPath(clusterPath.mEndpointId, clusterPath.mClusterId, aValueId);
                 [resultArray addObject:@ {
                     MTREventPathKey : [[MTREventPath alloc] initWithPath:eventPath],
                     MTRDataKey : aData.GetDecodedObject()
                 }];
-                if ([resultSuccess count] == 0) {
-                    [resultSuccess addObject:[NSNumber numberWithBool:YES]];
-                }
             };
 
-            auto onFailureCb = [resultArray, resultFailure](
+            auto onFailureCb = [resultArray, interactionStatus](
                                    const app::ConcreteClusterPath * clusterPath, const uint32_t aValueId, CHIP_ERROR aError) {
                 if (clusterPath) {
                     app::ConcreteEventPath eventPath(clusterPath->mEndpointId, clusterPath->mClusterId, aValueId);
                     [resultArray addObject:@ {
                         MTREventPathKey : [[MTREventPath alloc] initWithPath:eventPath],
                         MTRErrorKey : [MTRError errorForCHIPErrorCode:aError]
                     }];
-                } else if ([resultFailure count] == 0) {
-                    [resultFailure addObject:[MTRError errorForCHIPErrorCode:aError]];
+                } else {
+                    // This will only happen once per read interaction, and
+                    // after that there will be no more calls to onFailureCb or
+                    // onSuccessCb.
+                    *interactionStatus = aError;
                 }
             };
 
@@ -1560,24 +1556,13 @@ - (void)readEventsWithEndpointID:(NSNumber * _Nullable)endpointID
             readParams.mpEventPathParamsList = &eventPath;
             readParams.mEventPathParamsListSize = 1;
 
-            auto onDone = [resultArray, resultSuccess, resultFailure, bridge, successCb, failureCb](
+            auto onDone = [resultArray, interactionStatus, bridge, successCb, failureCb](
                               BufferedReadClientCallback<MTRDataValueDictionaryDecodableType> * callback) {
-                if ([resultFailure count] > 0 || [resultSuccess count] == 0) {
+                if (*interactionStatus != CHIP_NO_ERROR) {
                     // 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_READ_FAILED);
-                        }
-                    }
+                    failureCb(bridge, *interactionStatus);
                 } else {
-                    // Success
-                    if (successCb) {
-                        successCb(bridge, resultArray);
-                    }
+                    successCb(bridge, resultArray);
                 }
                 chip::Platform::Delete(callback);
             };

src/darwin/Framework/CHIP/MTRDeviceController+XPC.h

@@ -76,6 +76,15 @@ typedef void (^MTRValuesHandler)(id _Nullable values, NSError * _Nullable error)
  */
 + (MTRSubscribeParams * _Nullable)decodeXPCSubscribeParams:(NSDictionary<NSString *, id> * _Nullable)params;
 
+/**
+ * Returns an NSXPCInterface configured for MTRDeviceControllerServerProtocol.
+ */
++ (NSXPCInterface *)xpcInterfaceForServerProtocol MTR_NEWLY_AVAILABLE;
+
+/**
+ * Returns an NSXPCInterface configured for MTRDeviceControllerClientProtocol.
+ */
++ (NSXPCInterface *)xpcInterfaceForClientProtocol MTR_NEWLY_AVAILABLE;
 @end
 
 /**

src/darwin/Framework/CHIP/MTRDeviceController+XPC.mm

@@ -29,6 +29,16 @@
 static NSString * const kMinIntervalKey = @"minInterval";
 static NSString * const kMaxIntervalKey = @"maxInterval";
 
+// Classes that are used by MTRDevice responses.  In particular, needs to
+// include NSError.
+static NSSet * GetXPCAllowedClasses()
+{
+    static NSSet * const sXPCAllowedClasses = [NSSet setWithArray:@[
+        [NSString class], [NSNumber class], [NSData class], [NSArray class], [NSDictionary class], [NSError class]
+    ]];
+    return sXPCAllowedClasses;
+}
+
 static NSArray * _Nullable encodeAttributePath(MTRAttributePath * _Nullable path)
 {
     if (!path) {
@@ -190,6 +200,41 @@ + (MTRSubscribeParams * _Nullable)decodeXPCSubscribeParams:(NSDictionary<NSStrin
     return result;
 }
 
++ (NSXPCInterface *)xpcInterfaceForServerProtocol
+{
+    auto * xpcInterface = [NSXPCInterface interfaceWithProtocol:@protocol(MTRDeviceControllerServerProtocol)];
+    [xpcInterface setClasses:GetXPCAllowedClasses()
+                 forSelector:@selector(readAttributeWithController:nodeId:endpointId:clusterId:attributeId:params:completion:)
+               argumentIndex:0
+                     ofReply:YES];
+    [xpcInterface setClasses:GetXPCAllowedClasses()
+                 forSelector:@selector
+                 (writeAttributeWithController:nodeId:endpointId:clusterId:attributeId:value:timedWriteTimeout:completion:)
+               argumentIndex:0
+                     ofReply:YES];
+    [xpcInterface setClasses:GetXPCAllowedClasses()
+                 forSelector:@selector
+                 (invokeCommandWithController:nodeId:endpointId:clusterId:commandId:fields:timedInvokeTimeout:completion:)
+               argumentIndex:0
+                     ofReply:YES];
+
+    [xpcInterface setClasses:GetXPCAllowedClasses()
+                 forSelector:@selector(readAttributeCacheWithController:nodeId:endpointId:clusterId:attributeId:completion:)
+               argumentIndex:0
+                     ofReply:YES];
+    return xpcInterface;
+}
+
++ (NSXPCInterface *)xpcInterfaceForClientProtocol
+{
+    auto * xpcInterface = [NSXPCInterface interfaceWithProtocol:@protocol(MTRDeviceControllerClientProtocol)];
+    [xpcInterface setClasses:GetXPCAllowedClasses()
+                 forSelector:@selector(handleReportWithController:nodeId:values:error:)
+               argumentIndex:2
+                     ofReply:NO];
+    return xpcInterface;
+}
+
 @end
 
 @implementation MTRDeviceController (Deprecated_XPC)

src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.mm

@@ -68,8 +68,8 @@ @implementation MTRDeviceControllerXPCConnection
 - (instancetype)initWithWorkQueue:(dispatch_queue_t)workQueue connectBlock:(NSXPCConnection * (^)(void) )connectBlock
 {
     if ([super init]) {
-        _remoteDeviceServerProtocol = [NSXPCInterface interfaceWithProtocol:@protocol(MTRDeviceControllerServerProtocol)];
-        _remoteDeviceClientProtocol = [NSXPCInterface interfaceWithProtocol:@protocol(MTRDeviceControllerClientProtocol)];
+        _remoteDeviceServerProtocol = [MTRDeviceController xpcInterfaceForServerProtocol];
+        _remoteDeviceClientProtocol = [MTRDeviceController xpcInterfaceForClientProtocol];
         _connectBlock = connectBlock;
         _workQueue = workQueue;
         _reportRegistry = [[NSMutableDictionary alloc] init];