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

* Issue 16691 - Darwin: convenience API Part 2

* Restyled change

.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