Darwin: convenience API Part 2 (Issue 16691) (#21757)

* Issue 16691 - Darwin: convenience API Part 2

* Restyled change

Author: jtung-apple

.restyled.yaml

@@ -73,7 +73,6 @@ exclude:
     - "examples/chef/sample_app_util/test_files/*.yaml"
     - "examples/chef/zzz_generated/**/*"
     - "src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.mm" # https://github.com/project-chip/connectedhomeip/issues/20236
-    - "src/darwin/Framework/CHIP/zap-generated/MTRClusters.mm"
 
 
 changed_paths:

src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.h

@@ -23,22 +23,37 @@ NS_ASSUME_NONNULL_BEGIN
 
 typedef void (^MTRAsyncCallbackReadyHandler)(id context, NSUInteger retryCount);
 
-// How to queue a new work item:
+// MTRAsyncCallbackQueue high level description
+//   The MTRAsyncCallbackQueue was made to call one readyHandler
+//   block at a time asynchronously, and the readyHandler is
+//   expected to start/schedule a task. When the task finishes
+//   asynchronously in the future (at any time, from any queue
+//   or thread), it is expected to ask the workItem object to
+//   either endWork or retryWork.
+
+// Sequence of steps when queuing a work item:
 //   - Create MTRAsyncCallbackQueueWorkItem object
 //   - Create ready handler block (MTRAsyncCallbackReadyHandler)
-//      - block is called when it's the work item's turn to do work
-//      - its body is to do work with the device
+//      - block is called when it's the WorkItem's turn to do work
+//      - its body is to perform a task that is expected to end asynchronously in the future
 //      - at the end of work, call on the work item object:
 //         - endWork for success or failure
 //         - retryWork for temporary failures
-//   - Set the work handler block to the Item object
-//   - Call enqueueWorkItem on the MTRDevice's work queue property
+//   - Set the readyHandler block on the WorkItem object
+//   - Call enqueueWorkItem on a MTRAsyncCallbackQueue
 
 // A serial one-at-a-time queue for performing work items
 @interface MTRAsyncCallbackWorkQueue : NSObject
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;
 
+// The context object is only held and passed back as a reference and is opaque to the work queue
+- (instancetype)initWithContext:(id _Nullable)context queue:(dispatch_queue_t)queue;
+
+// Called by the work queue owner to clean up and cancel work items
+- (void)invalidate;
+
+// Work items may be enqueued from any queue or thread
 - (void)enqueueWorkItem:(MTRAsyncCallbackQueueWorkItem *)item;
 
 // TODO: Add a "set concurrency width" method to allow for more than 1 work item at a time
@@ -49,12 +64,17 @@ typedef void (^MTRAsyncCallbackReadyHandler)(id context, NSUInteger retryCount);
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;
 
+// Both readyHandler and cancelHander will be called on the queue given to initWithQueue
 - (instancetype)initWithQueue:(dispatch_queue_t)queue;
 @property (nonatomic, strong) MTRAsyncCallbackReadyHandler readyHandler;
 @property (nonatomic, strong) dispatch_block_t cancelHandler;
 
-// Called by Cluster object's after async work is done
+// Called by the creater of the work item when async work is done and should
+// be removed from the queue. The work queue will run the next work item.
 - (void)endWork;
+
+// Called by the creater of the work item when async work should be retried.
+// The work queue will call this workItem's readyHandler again.
 - (void)retryWork;
 @end
 

src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm

@@ -18,12 +18,17 @@
 #import <dispatch/dispatch.h>
 #import <os/lock.h>
 
-#import "MTRAsyncCallbackWorkQueue_Internal.h"
+#import "MTRAsyncCallbackWorkQueue.h"
 #import "MTRLogging.h"
 
 #pragma mark - Class extensions
 
 @interface MTRAsyncCallbackWorkQueue ()
+// The lock protects the internal state of the work queue so that these may be called from any queue or thread:
+//   -enqueueWorkItem:
+//   -invalidate
+//   -endWork:
+//   -retryWork:
 @property (nonatomic, readonly) os_unfair_lock lock;
 @property (nonatomic, strong, readonly) id context;
 @property (nonatomic, strong, readonly) dispatch_queue_t queue;
@@ -81,7 +86,8 @@ - (void)invalidate
     [invalidateItems removeAllObjects];
 }
 
-- (void)endWork:(MTRAsyncCallbackQueueWorkItem *)workItem
+// called after executing a work item
+- (void)_postProcessWorkItem:(MTRAsyncCallbackQueueWorkItem *)workItem retry:(BOOL)retry
 {
     os_unfair_lock_lock(&_lock);
     // sanity check if running
@@ -102,41 +108,25 @@ - (void)endWork:(MTRAsyncCallbackQueueWorkItem *)workItem
         return;
     }
 
-    // since work is done, remove from queue and call ready on the next item
-    [self.items removeObjectAtIndex:0];
+    // if work item is done (no need to retry), remove from queue and call ready on the next item
+    if (!retry) {
+        [self.items removeObjectAtIndex:0];
+    }
 
     // when "concurrency width" is implemented this will be decremented instead
     self.runningWorkItemCount = 0;
     [self _callNextReadyWorkItem];
     os_unfair_lock_unlock(&_lock);
 }
 
-- (void)retryWork:(MTRAsyncCallbackQueueWorkItem *)workItem
+- (void)endWork:(MTRAsyncCallbackQueueWorkItem *)workItem
 {
-    // reset BOOL and call again
-    os_unfair_lock_lock(&_lock);
-    // sanity check if running
-    if (!self.runningWorkItemCount) {
-        // something is wrong with state - nothing is currently running
-        os_unfair_lock_unlock(&_lock);
-        MTR_LOG_ERROR("retryWork: no work is running on work queue");
-        return;
-    }
-
-    // sanity check the same work item is running
-    // when "concurrency width" is implemented need to check first N items
-    MTRAsyncCallbackQueueWorkItem * firstWorkItem = self.items.firstObject;
-    if (firstWorkItem != workItem) {
-        // something is wrong with this work item - should not be currently running
-        os_unfair_lock_unlock(&_lock);
-        MTR_LOG_ERROR("retryWork: work item is not first on work queue");
-        return;
-    }
+    [self _postProcessWorkItem:workItem retry:NO];
+}
 
-    // when "concurrency width" is implemented this will be decremented instead
-    self.runningWorkItemCount = 0;
-    [self _callNextReadyWorkItem];
-    os_unfair_lock_unlock(&_lock);
+- (void)retryWork:(MTRAsyncCallbackQueueWorkItem *)workItem
+{
+    [self _postProcessWorkItem:workItem retry:YES];
 }
 
 // assume lock is held while calling this
@@ -166,13 +156,11 @@ - (instancetype)initWithQueue:(dispatch_queue_t)queue
     return self;
 }
 
-// Called by Cluster object's after async work is done
 - (void)endWork
 {
     [self.workQueue endWork:self];
 }
 
-// Called by Cluster object's after async work is done
 - (void)retryWork
 {
     [self.workQueue retryWork:self];

src/darwin/Framework/CHIP/MTRBaseDevice.h

@@ -30,6 +30,7 @@ NS_ASSUME_NONNULL_BEGIN
  *
  *                MTRAttributePathKey : MTRAttributePath object. Included for attribute value.
  *                MTRCommandPathKey : MTRCommandPath object. Included for command response.
+ *                MTREventPathKey : MTREventPath object. Included for event value.
  *                MTRErrorKey : NSError object. Included to indicate an error.
  *                MTRDataKey: Data-value NSDictionary object.
  *                              Included when there is data and when there is no error.
@@ -67,11 +68,18 @@ NS_ASSUME_NONNULL_BEGIN
  *                MTRDataKey : Data-value NSDictionary object.
  */
 typedef void (^MTRDeviceResponseHandler)(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error);
+
+/**
+ * Handler for -subscribeWithQueue: attribute and event reports
+ *
+ * @param values This array contains MTRAttributeReport objects for attribute reports, and MTREventReport objects for event reports
+ */
 typedef void (^MTRDeviceReportHandler)(NSArray * values);
 typedef void (^MTRDeviceErrorHandler)(NSError * error);
 
 extern NSString * const MTRAttributePathKey;
 extern NSString * const MTRCommandPathKey;
+extern NSString * const MTREventPathKey;
 extern NSString * const MTRDataKey;
 extern NSString * const MTRErrorKey;
 extern NSString * const MTRTypeKey;

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -50,6 +50,7 @@
 
 NSString * const MTRAttributePathKey = @"attributePath";
 NSString * const MTRCommandPathKey = @"commandPath";
+NSString * const MTREventPathKey = @"eventPath";
 NSString * const MTRDataKey = @"data";
 NSString * const MTRErrorKey = @"error";
 NSString * const MTRTypeKey = @"type";

src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h

@@ -68,6 +68,10 @@ NS_ASSUME_NONNULL_BEGIN
 - (instancetype)initWithPath:(const chip::app::ConcreteDataAttributePath &)path;
 @end
 
+@interface MTREventPath ()
+- (instancetype)initWithPath:(const chip::app::ConcreteEventPath &)path;
+@end
+
 @interface MTRCommandPath ()
 - (instancetype)initWithPath:(const chip::app::ConcreteCommandPath &)path;
 @end