Switch Darwin to the new single-attribute subscription API (#12797)

src/app/zap-templates/common/ClusterTestGeneration.js

@@ -319,16 +319,18 @@ function parse(filename)
         command : "waitForReport",
         attribute : test.attribute,
         response : test.response,
-        async : true
+        async : true,
+        allocateSubscribeDataCallback : true,
       };
       delete test.response;
 
       // insert the new report test into the tests list
       yaml.tests.splice(index, 0, reportTest);
 
       // Associate the "subscribeAttribute" test with the synthesized report test
-      test.hasWaitForReport = true;
-      test.waitForReport    = reportTest;
+      test.hasWaitForReport              = true;
+      test.waitForReport                 = reportTest;
+      test.allocateSubscribeDataCallback = !test.hasWaitForReport;
     }
   });
 

src/darwin/Framework/CHIP/CHIPCallbackBridgeBase_internal.h

@@ -42,7 +42,7 @@ template <class T> class CHIPCallbackBridge {
 
         if (CHIP_NO_ERROR != err) {
             dispatch_async(queue, ^{
-                handler([CHIPError errorForCHIPErrorCode:err], nil);
+                handler(nil, [CHIPError errorForCHIPErrorCode:err]);
             });
 
             NSString * errorStr = [NSString stringWithFormat:@"%s: %s", typeid(T).name(), chip::ErrorStr(err)];
@@ -58,6 +58,9 @@ template <class T> class CHIPCallbackBridge {
 
     static void DispatchFailure(void * context, NSError * error) { DispatchCallbackResult(context, error, nil); }
 
+protected:
+    dispatch_queue_t mQueue;
+
 private:
     static void DispatchCallbackResult(void * context, NSError * error, id value)
     {
@@ -71,16 +74,20 @@ template <class T> class CHIPCallbackBridge {
             return;
         }
 
+        if (error) {
+            // We should delete ourselves; there will be no more callbacks.
+            callbackBridge->mKeepAlive = false;
+        }
+
         dispatch_async(callbackBridge->mQueue, ^{
-            callbackBridge->mHandler(error, value);
+            callbackBridge->mHandler(value, error);
 
             if (!callbackBridge->mKeepAlive) {
                 delete callbackBridge;
             }
         });
     }
 
-    dispatch_queue_t mQueue;
     ResponseHandler mHandler;
     bool mKeepAlive;
 

src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt

@@ -56,15 +56,15 @@ using namespace chip::app::Clusters;
     {{/chip_cluster_command_arguments}}
 
     new CHIP{{>callbackName}}CallbackBridge(self.callbackQueue,
-      {{! For now, don't change the bridge API; instead just use an adapter
-          to invoke our completion handler. This is not great from a
-          type-safety perspective, of course. }}
       {{#if hasSpecificResponse}}
-        ^(NSError * _Nullable error, id _Nullable value) {
-          completionHandler(value, error);
-        },
+        {{! This treats completionHandler as taking an id for the data.  This is
+            not great from a type-safety perspective, of course. }}
+        completionHandler,
       {{else}}
-        ^(NSError * _Nullable error, id _Nullable value) {
+        {{! For now, don't change the bridge API; instead just use an adapter
+            to invoke our completion handler. This is not great from a
+            type-safety perspective, of course. }}
+        ^(id _Nullable value, NSError * _Nullable error) {
           completionHandler(error);
         },
       {{/if}}
@@ -82,7 +82,8 @@ using namespace chip::app::Clusters;
 {{/chip_cluster_commands}}
 
 {{#chip_server_cluster_attributes}}
-{{! TODO: Need to add support for struct-type attibutes here, and enum-typed ones }}
+{{! TODO: Need to add support for struct-type attibutes here.  And remove the callbackName
+          inline duplication with subscribe. }}
 {{#unless (isStrEqual chipCallback.name "Unsupported")}}
 {{#*inline "attribute"}}Attribute{{asUpperCamelCase name}}{{/inline}}
 {{#*inline "callbackName"}}
@@ -103,12 +104,9 @@ using namespace chip::app::Clusters;
 - (void)read{{>attribute}}WithCompletionHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error))completionHandler
 {
     new CHIP{{>callbackName}}CallbackBridge(self.callbackQueue,
-      {{! For now, don't change the bridge API; instead just use an adapter
-          to invoke our completion handler. This is not great from a
-          type-safety perspective, of course. }}
-      ^(NSError * _Nullable error, id _Nullable value) {
-        completionHandler(value, error);
-      },
+      {{! This treats completionHandler as taking an id for the data.  This is
+          not great from a type-safety perspective, of course. }}
+      completionHandler,
       ^(Cancelable * success, Cancelable * failure) {
           using TypeInfo = {{asUpperCamelCase parent.name}}::Attributes::{{asUpperCamelCase name}}::TypeInfo;
           auto successFn = Callback<{{>callbackName}}Callback>::FromCancelable(success);
@@ -125,7 +123,7 @@ using namespace chip::app::Clusters;
       {{! For now, don't change the bridge API; instead just use an adapter
           to invoke our completion handler. This is not great from a
           type-safety perspective, of course. }}
-      ^(NSError * _Nullable error, id _Nullable ignored) {
+      ^(id _Nullable ignored, NSError * _Nullable error) {
         completionHandler(error);
       },
       ^(Cancelable * success, Cancelable * failure) {
@@ -146,31 +144,36 @@ using namespace chip::app::Clusters;
 
 {{/if}}
 {{#if isReportableAttribute}}
-{{#unless isList}}
-{{#*inline "callbackName"}}DefaultSuccess{{/inline}}
-{{#*inline "callbackParams"}}, minInterval, maxInterval{{#if isAnalog}}, change{{/if}}{{/inline}}
-- (void) subscribe{{>attribute}}WithMinInterval:(uint16_t)minInterval  maxInterval:(uint16_t)maxInterval responseHandler:(ResponseHandler)responseHandler
+{{! This callbackName bit is duplicated with the readAttribute code above. }}
+{{#*inline "callbackName"}}
+  {{~#if isList~}}
+    {{asUpperCamelCase ../name}}{{asUpperCamelCase name}}List
+  {{~else~}}
+    {{~#if isNullable}}Nullable{{/if~}}
+    {{~#if (isStrEqual (asUpperCamelCase type) (asUpperCamelCase "vendor_id"))~}}
+      VendorId
+    {{~else if isEnum~}}
+      {{asUpperCamelCase ../name}}Cluster{{asUpperCamelCase type}}
+    {{~else~}}
+      {{chipCallback.name}}
+    {{~/if~}}
+  {{~/if~}}
+  Attribute
+{{~/inline}}
+- (void) subscribe{{>attribute}}WithMinInterval:(uint16_t)minInterval  maxInterval:(uint16_t)maxInterval subscriptionEstablished:(SubscriptionEstablishedHandler _Nullable)subscriptionEstablishedHandler reportHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error))reportHandler
 {
-    new CHIP{{>callbackName}}CallbackBridge(self.callbackQueue, responseHandler, ^(Cancelable * success, Cancelable * failure) {
-        return self.cppCluster.Subscribe{{>attribute}}(success, failure{{>callbackParams}});
-    });
-}
-
-{{#*inline "callbackName"}}{{#if isNullable}}Nullable{{/if}}{{chipCallback.name}}Attribute{{/inline}}
-- (void) report{{>attribute}}WithResponseHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error))responseHandler {
-    new CHIP{{>callbackName}}CallbackBridge(self.callbackQueue,
-      {{! For now, don't change the bridge API; instead just use an adapter
-          to invoke our response handler. This is not great from a
-          type-safety perspective, of course. }}
-      ^(NSError * _Nullable error, id _Nullable value) {
-        responseHandler(value, error);
-      },
+    new CHIP{{>callbackName}}CallbackSubscriptionBridge(self.callbackQueue,
+      {{! This treats reportHandler as taking an id for the data.  This is
+          not great from a type-safety perspective, of course. }}
+      reportHandler,
       ^(Cancelable * success, Cancelable * failure) {
-        return self.cppCluster.Report{{>attribute}}(success);
-      }, true);
+          using TypeInfo = {{asUpperCamelCase parent.name}}::Attributes::{{asUpperCamelCase name}}::TypeInfo;
+          auto successFn = Callback<{{>callbackName}}Callback>::FromCancelable(success);
+          auto failureFn = Callback<CHIPDefaultFailureCallbackType>::FromCancelable(failure);
+          return self.cppCluster.SubscribeAttribute<TypeInfo>(successFn->mContext, successFn->mCall, failureFn->mCall, minInterval, maxInterval, CHIP{{>callbackName}}CallbackSubscriptionBridge::OnSubscriptionEstablished);
+      }, subscriptionEstablishedHandler);
 }
 
-{{/unless}}
 {{/if}}
 {{/unless}}
 {{/chip_server_cluster_attributes}}

src/darwin/Framework/CHIP/templates/CHIPClustersObjc.zapt

@@ -9,8 +9,9 @@
 #include <CHIP/CHIPCluster.h>
 #include <CHIP/CHIPCommandPayloadsObjc.h>
 
-typedef void (^ResponseHandler)(NSError * _Nullable error, id _Nullable values);
+typedef void (^ResponseHandler)(id _Nullable value, NSError * _Nullable error);
 typedef void (^StatusCompletion)(NSError * _Nullable error);
+typedef void (^SubscriptionEstablishedHandler)(void);
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -44,10 +45,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)write{{>attribute}}WithValue:({{asObjectiveCType type parent.name}})value completionHandler:(StatusCompletion)completionHandler;
 {{/if}}
 {{#if isReportableAttribute}}
-{{#unless isList}}
-- (void) subscribe{{>attribute}}WithMinInterval:(uint16_t)minInterval  maxInterval:(uint16_t)maxInterval responseHandler:(ResponseHandler)responseHandler;
-- (void) report{{>attribute}}WithResponseHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error))responseHandler;
-{{/unless}}
+- (void) subscribe{{>attribute}}WithMinInterval:(uint16_t)minInterval  maxInterval:(uint16_t)maxInterval subscriptionEstablished:(SubscriptionEstablishedHandler _Nullable)subscriptionEstablishedHandler reportHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error))reportHandler;
 {{/if}}
 {{/unless}}
 {{/chip_server_cluster_attributes}}

src/darwin/Framework/CHIP/templates/CHIPTestClustersObjc-src.zapt

@@ -40,7 +40,7 @@ using namespace chip::app::Clusters;
       {{! For now, don't change the bridge API; instead just use an adapter
           to invoke our completion handler. This is not great from a
           type-safety perspective, of course. }}
-      ^(NSError * _Nullable error, id _Nullable ignored) {
+      ^(id _Nullable ignored, NSError * _Nullable error) {
         completionHandler(error);
       },
       ^(Cancelable * success, Cancelable * failure) {

src/darwin/Framework/CHIP/templates/partials/CHIPCallbackBridge.zapt

@@ -33,6 +33,23 @@ public:
       {{/if}}
     );
 };
+
+{{#unless partial-type}}
+class CHIP{{> @partial-block}}SubscriptionBridge : public CHIP{{> @partial-block}}Bridge
+{
+public:
+    CHIP{{> @partial-block}}SubscriptionBridge(dispatch_queue_t queue, ResponseHandler handler, CHIPActionBlock action, SubscriptionEstablishedHandler establishedHandler)
+      : CHIP{{> @partial-block}}Bridge(queue, handler, action, true),
+        mEstablishedHandler(establishedHandler)
+      {}
+
+    static void OnSubscriptionEstablished(void * context);
+
+private:
+    SubscriptionEstablishedHandler mEstablishedHandler;
+};
+{{/unless}}
+
 {{else}}
 void CHIP{{> @partial-block}}Bridge::OnSuccessFn(void * context
   {{#if partial-type}}
@@ -74,4 +91,22 @@ void CHIP{{> @partial-block}}Bridge::OnSuccessFn(void * context
     DispatchSuccess(context, objCValue);
     {{/if}}
 };
+
+{{#unless partial-type}}
+void CHIP{{> @partial-block}}SubscriptionBridge::OnSubscriptionEstablished(void * context)
+{
+     auto * self = static_cast<CHIP{{> @partial-block}}SubscriptionBridge *>(context);
+     if (!self->mQueue) {
+         return;
+     }
+
+     if (self->mEstablishedHandler != nil) {
+         dispatch_async(self->mQueue, self->mEstablishedHandler);
+         // On failure, mEstablishedHandler will be cleaned up by our destructor,
+         // but we can clean it up earlier on successful subscription
+         // establishment.
+         self->mEstablishedHandler = nil;
+     }
+}
+{{/unless}}
 {{/if}}

src/darwin/Framework/CHIP/templates/partials/test_cluster.zapt

@@ -8,6 +8,14 @@ bool testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase
 {{asBasicType chipType}} {{saveAs}};
 {{/if}}
 {{/chip_tests_item_response_parameters}}
+
+{{~#*inline "subscribeDataCallback"}}
+test_{{parent.filename}}_{{attribute}}_Reported
+{{/inline}}
+{{#if allocateSubscribeDataCallback}}
+ResponseHandler {{> subscribeDataCallback}} = nil;
+{{/if~}}
+
 - (void)testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase command}}
 {
     {{#unless async}}
@@ -43,21 +51,17 @@ bool testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase
       {{/if}}
     {{else if isSubscribeAttribute}}
     {{#chip_tests_item_parameters}}
-    {{#if (isString type)}}
-    {{#if (isOctetString type)}}
-    NSData * {{asLowerCamelCase name}}Argument = [[NSData alloc] initWithBytes:"{{octetStringEscapedForCLiteral definedValue}}" length:{{definedValue.length}}];
-    {{else}}
-    NSString * {{asLowerCamelCase name}}Argument= @"{{definedValue}}";
-    {{/if}}
-    {{else}}
     {{asObjectiveCBasicType type}} {{asLowerCamelCase name}}Argument = {{asTypedLiteral definedValue type}};
-    {{/if}}
     {{/chip_tests_item_parameters}}
     [cluster subscribeAttribute{{asUpperCamelCase attribute}}WithMinInterval:minIntervalArgument
                                                                  maxInterval:maxIntervalArgument
-                                                             responseHandler:^(NSError * err, NSDictionary * values) {
+                                                     subscriptionEstablished:^{
+        XCTAssertEqual(testSendCluster{{parent.filename}}_{{asTestIndex waitForReport.index}}_{{asUpperCamelCase waitForReport.command}}_Fulfilled, true);
+        [expectation fulfill];
+      }
+                                                               reportHandler:^({{asObjectiveCClass attributeObject.type cluster forceList=attributeObject.isList}} * _Nullable value, NSError * _Nullable err) {
     {{else if isWaitForReport}}
-    [cluster reportAttribute{{asUpperCamelCase attribute}}WithResponseHandler:^({{asObjectiveCClass attributeObject.type cluster forceList=attributeObject.isList}} * _Nullable value, NSError * _Nullable err) {
+    {{> subscribeDataCallback }} = ^({{asObjectiveCClass attributeObject.type cluster forceList=attributeObject.isList}} * _Nullable value, NSError * _Nullable err) {
     {{else if isReadAttribute}}
     [cluster readAttribute{{asUpperCamelCase attribute}}WithCompletionHandler:^({{asObjectiveCClass attributeObject.type cluster forceList=attributeObject.isList}} * _Nullable value, NSError * _Nullable err) {
     {{else if isWriteAttribute}}
@@ -149,16 +153,21 @@ bool testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase
         {{/if}}
         {{/chip_tests_item_response_parameters}}
 
-        {{else}}
-        XCTAssertEqual(testSendCluster{{parent.filename}}_{{asTestIndex waitForReport.index}}_{{asUpperCamelCase waitForReport.command}}_Fulfilled, true);
-        {{/unless}}
         {{#unless async}}
         [expectation fulfill];
         {{else}}
         testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase command}}_Fulfilled = true;
         {{/unless}}
+        {{else}}
+          {{! We're a subscription }}
+          if ({{> subscribeDataCallback}} != nil) {
+            ResponseHandler callback = {{> subscribeDataCallback}};
+            {{> subscribeDataCallback}} = nil;
+            callback(value, err);
+          }
+        {{/unless}}
         {{/if}}
-    }];
+    }{{#unless isWaitForReport}}]{{/unless}};
 
 {{/if}}
     {{#unless async}}