Merge branch 'master' into feature/unittest--transport-tests

Author: feasel0

examples/light-switch-app/silabs/src/ZclCallbacks.cpp

@@ -33,8 +33,8 @@ using namespace ::chip::app::Clusters;
 void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & attributePath, uint8_t type, uint16_t size,
                                        uint8_t * value)
 {
-    ClusterId clusterId     = attributePath.mClusterId;
-    AttributeId attributeId = attributePath.mAttributeId;
+    ClusterId clusterId                      = attributePath.mClusterId;
+    [[maybe_unused]] AttributeId attributeId = attributePath.mAttributeId;
     ChipLogProgress(Zcl, "Cluster callback: " ChipLogFormatMEI, ChipLogValueMEI(clusterId));
 
     if (clusterId == OnOffSwitchConfiguration::Id)

examples/lock-app/silabs/src/ZclCallbacks.cpp

@@ -48,7 +48,7 @@ void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath &
 
     if (clusterId == DoorLock::Id && attributeId == DoorLock::Attributes::LockState::Id)
     {
-        DoorLock::DlLockState lockState = *(reinterpret_cast<DoorLock::DlLockState *>(value));
+        [[maybe_unused]] DoorLock::DlLockState lockState = *(reinterpret_cast<DoorLock::DlLockState *>(value));
         ChipLogProgress(Zcl, "Door lock cluster: " ChipLogFormatMEI " state %d", ChipLogValueMEI(clusterId),
                         to_underlying(lockState));
 #ifdef DIC_ENABLE

src/darwin/Framework/CHIP/MTRAsyncWorkQueue.h

@@ -192,6 +192,17 @@ MTR_TESTABLE
 /// is lost.
 - (instancetype)initWithContext:(ContextType)context;
 
+/// Creates a work queue with the given context object and a queue width.
+///
+/// The queue will call readyHandler on up to "width" number of work items
+/// concurrently. Once "width" number of work items have started, no other
+/// work items will get a readyHandler call until one of the running work items
+/// has called its completion block with MTRAsyncWorkComplete.
+///
+/// This allows the a MTRAsyncWorkQueue object to manage a pool of
+/// resources that can be use concurrently at any given time.
+- (instancetype)initWithContext:(ContextType)context width:(NSUInteger)width;
+
 /// Enqueues the specified work item, making it eligible for execution.
 ///
 /// Once a work item is enqueued, ownership of it passes to the queue and

src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm

@@ -197,7 +197,8 @@ @implementation MTRAsyncWorkQueue {
     os_unfair_lock _lock;
     __weak id _context;
     NSMutableArray<MTRAsyncWorkItem *> * _items;
-    NSInteger _runningWorkItemCount;
+    NSUInteger _runningWorkItemCount;
+    NSUInteger _width;
 }
 
 // A helper struct that facilitates access to _context while
@@ -216,11 +217,17 @@ @implementation MTRAsyncWorkQueue {
 };
 
 - (instancetype)initWithContext:(id)context
+{
+    return [self initWithContext:context width:1];
+}
+
+- (instancetype)initWithContext:(id)context width:(NSUInteger)width
 {
     NSParameterAssert(context);
     if (self = [super init]) {
         _context = context;
         _items = [NSMutableArray array];
+        _width = width;
     }
     return self;
 }
@@ -286,35 +293,84 @@ - (void)_postProcessWorkItem:(MTRAsyncWorkItem *)workItem
 {
     os_unfair_lock_assert_owner(&_lock);
 
-    MTRAsyncWorkItem * runningWorkItem = (_runningWorkItemCount) ? _items.firstObject : nil;
-    if (workItem != runningWorkItem) {
+    BOOL foundWorkItem = NO;
+    NSUInteger indexOfWorkItem = 0;
+    for (NSUInteger i = 0; i < _width; i++) {
+        if (_items[i] == workItem) {
+            foundWorkItem = YES;
+            indexOfWorkItem = i;
+            break;
+        }
+    }
+    if (!foundWorkItem) {
         NSAssert(NO, @"work item to post-process is not running");
         return;
     }
 
+    // already part of the running work items allowed by width - retry directly
     if (retry) {
         MTR_LOG_DEFAULT("MTRAsyncWorkQueue<%@> retry needed for work item [%llu]", context.description, workItem.uniqueID);
-    } else {
-        [workItem markComplete];
-        [_items removeObjectAtIndex:0];
-        MTR_LOG_DEFAULT("MTRAsyncWorkQueue<%@, items count: %tu> completed work item [%llu]", context.description, _items.count, workItem.uniqueID);
+        [self _callWorkItem:workItem withContext:context];
+        return;
     }
 
-    // when "concurrency width" is implemented this will be decremented instead
-    _runningWorkItemCount = 0;
+    [workItem markComplete];
+    [_items removeObjectAtIndex:indexOfWorkItem];
+    MTR_LOG_DEFAULT("MTRAsyncWorkQueue<%@, items count: %tu> completed work item [%llu]", context.description, _items.count, workItem.uniqueID);
+
+    // sanity check running work item count is positive
+    if (_runningWorkItemCount == 0) {
+        NSAssert(NO, @"running work item count should be positive");
+        return;
+    }
+
+    _runningWorkItemCount--;
     [self _callNextReadyWorkItemWithContext:context];
 }
 
+- (void)_callWorkItem:(MTRAsyncWorkItem *)workItem withContext:(ContextSnapshot const &)context
+{
+    os_unfair_lock_assert_owner(&_lock);
+
+    mtr_weakify(self);
+    [workItem callReadyHandlerWithContext:context.reference completion:^(MTRAsyncWorkOutcome outcome) {
+        mtr_strongify(self);
+        BOOL handled = NO;
+        if (self) {
+            ContextSnapshot context(self); // re-acquire a new snapshot
+            std::lock_guard lock(self->_lock);
+            if (!workItem.isComplete) {
+                [self _postProcessWorkItem:workItem context:context retry:(outcome == MTRAsyncWorkNeedsRetry)];
+                handled = YES;
+            }
+        }
+        return handled;
+    }];
+}
+
 - (void)_callNextReadyWorkItemWithContext:(ContextSnapshot const &)context
 {
     os_unfair_lock_assert_owner(&_lock);
 
-    // when "concurrency width" is implemented this will be checked against the width
-    if (_runningWorkItemCount) {
-        return; // can't run next work item until the current one is done
+    // sanity check not running more than allowed
+    if (_runningWorkItemCount > _width) {
+        NSAssert(NO, @"running work item count larger than the maximum width");
+        return;
     }
 
-    if (!_items.count) {
+    // sanity check consistent counts
+    if (_items.count < _runningWorkItemCount) {
+        NSAssert(NO, @"work item count is less than running work item count");
+        return;
+    }
+
+    // can't run more work items if already running at max concurrent width
+    if (_runningWorkItemCount == _width) {
+        return;
+    }
+
+    // no more items to run
+    if (_items.count == _runningWorkItemCount) {
         return; // nothing to run
     }
 
@@ -324,16 +380,16 @@ - (void)_callNextReadyWorkItemWithContext:(ContextSnapshot const &)context
         return;
     }
 
-    // when "concurrency width" is implemented this will be incremented instead
-    _runningWorkItemCount = 1;
-
-    MTRAsyncWorkItem * workItem = _items.firstObject;
+    NSUInteger nextWorkItemToRunIndex = _runningWorkItemCount;
+    MTRAsyncWorkItem * workItem = _items[nextWorkItemToRunIndex];
+    _runningWorkItemCount++;
 
-    // Check if batching is possible or needed. Only ask work item to batch once for simplicity
+    // Check if batching is possible or needed.
     auto batchingHandler = workItem.batchingHandler;
-    if (batchingHandler && workItem.retryCount == 0) {
-        while (_items.count >= 2) {
-            MTRAsyncWorkItem * nextWorkItem = _items[1];
+    if (batchingHandler) {
+        while (_items.count > _runningWorkItemCount) {
+            NSUInteger firstNonRunningItemIndex = _runningWorkItemCount;
+            MTRAsyncWorkItem * nextWorkItem = _items[firstNonRunningItemIndex];
             if (!nextWorkItem.batchingHandler || nextWorkItem.batchingID != workItem.batchingID) {
                 goto done; // next item is not eligible to merge with this one
             }
@@ -355,20 +411,7 @@ - (void)_callNextReadyWorkItemWithContext:(ContextSnapshot const &)context
     done:;
     }
 
-    mtr_weakify(self);
-    [workItem callReadyHandlerWithContext:context.reference completion:^(MTRAsyncWorkOutcome outcome) {
-        mtr_strongify(self);
-        BOOL handled = NO;
-        if (self) {
-            ContextSnapshot context(self); // re-acquire a new snapshot
-            std::lock_guard lock(self->_lock);
-            if (!workItem.isComplete) {
-                [self _postProcessWorkItem:workItem context:context retry:(outcome == MTRAsyncWorkNeedsRetry)];
-                handled = YES;
-            }
-        }
-        return handled;
-    }];
+    [self _callWorkItem:workItem withContext:context];
 }
 
 - (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData

src/darwin/Framework/CHIP/MTRDeviceConnectivityMonitor.mm

@@ -23,18 +23,23 @@
 #import <os/lock.h>
 
 #include <lib/dnssd/ServiceNaming.h>
+#include <lib/support/CodeUtils.h>
+#include <vector>
 
 @implementation MTRDeviceConnectivityMonitor {
     NSString * _instanceName;
-    DNSServiceRef _resolver;
+    std::vector<DNSServiceRef> _resolvers;
     NSMutableDictionary<NSString *, nw_connection_t> * _connections;
 
     MTRDeviceConnectivityMonitorHandler _monitorHandler;
     dispatch_queue_t _handlerQueue;
 }
 
 namespace {
-constexpr char kLocalDot[] = "local.";
+constexpr const char * kResolveDomains[] = {
+    "default.service.arpa.", // SRP
+    "local.",
+};
 constexpr char kOperationalType[] = "_matter._tcp";
 constexpr int64_t kSharedConnectionLingerIntervalSeconds = (10);
 }
@@ -68,8 +73,8 @@ - (instancetype)initWithCompressedFabricID:(NSNumber *)compressedFabricID nodeID
 
 - (void)dealloc
 {
-    if (_resolver) {
-        DNSServiceRefDeallocate(_resolver);
+    for (auto & resolver : _resolvers) {
+        DNSServiceRefDeallocate(resolver);
     }
 }
 
@@ -188,32 +193,39 @@ - (void)startMonitoringWithHandler:(MTRDeviceConnectivityMonitorHandler)handler
     _handlerQueue = queue;
 
     // If there's already a resolver running, just return
-    if (_resolver) {
+    if (_resolvers.size() != 0) {
         MTR_LOG_INFO("%@ connectivity monitor already running", self);
         return;
     }
 
     MTR_LOG_INFO("%@ start connectivity monitoring for %@ (%lu monitoring objects)", self, _instanceName, static_cast<unsigned long>(sConnectivityMonitorCount));
 
-    _resolver = [MTRDeviceConnectivityMonitor _sharedResolverConnection];
-    if (!_resolver) {
+    auto sharedConnection = [MTRDeviceConnectivityMonitor _sharedResolverConnection];
+    if (!sharedConnection) {
         MTR_LOG_ERROR("%@ failed to get shared resolver connection", self);
         return;
     }
-    DNSServiceErrorType dnsError = DNSServiceResolve(&_resolver,
-        kDNSServiceFlagsShareConnection,
-        kDNSServiceInterfaceIndexAny,
-        _instanceName.UTF8String,
-        kOperationalType,
-        kLocalDot,
-        ResolveCallback,
-        (__bridge void *) self);
-    if (dnsError != kDNSServiceErr_NoError) {
-        MTR_LOG_ERROR("%@ failed to create resolver", self);
-        return;
+
+    for (auto domain : kResolveDomains) {
+        DNSServiceRef resolver = sharedConnection;
+        DNSServiceErrorType dnsError = DNSServiceResolve(&resolver,
+            kDNSServiceFlagsShareConnection,
+            kDNSServiceInterfaceIndexAny,
+            _instanceName.UTF8String,
+            kOperationalType,
+            domain,
+            ResolveCallback,
+            (__bridge void *) self);
+        if (dnsError == kDNSServiceErr_NoError) {
+            _resolvers.emplace_back(std::move(resolver));
+        } else {
+            MTR_LOG_ERROR("%@ failed to create resolver for \"%s\" domain: %" PRId32, self, StringOrNullMarker(domain), dnsError);
+        }
     }
 
-    sConnectivityMonitorCount++;
+    if (_resolvers.size() != 0) {
+        sConnectivityMonitorCount++;
+    }
 }
 
 - (void)_stopMonitoring
@@ -227,9 +239,11 @@ - (void)_stopMonitoring
     _monitorHandler = nil;
     _handlerQueue = nil;
 
-    if (_resolver) {
-        DNSServiceRefDeallocate(_resolver);
-        _resolver = NULL;
+    if (_resolvers.size() != 0) {
+        for (auto & resolver : _resolvers) {
+            DNSServiceRefDeallocate(resolver);
+        }
+        _resolvers.clear();
 
         // If no monitor objects exist, schedule to deallocate shared connection and queue
         sConnectivityMonitorCount--;

src/darwin/Framework/CHIP/MTROperationalBrowser.mm

@@ -21,13 +21,16 @@
 
 #include <cinttypes>
 #include <lib/dnssd/ServiceNaming.h>
+#include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <platform/LockTracker.h>
 
 namespace {
-constexpr char kLocalDot[] = "local.";
+constexpr const char * kBrowseDomains[] = {
+    "default.service.arpa.", // SRP
+    "local.",
+};
 constexpr char kOperationalType[] = "_matter._tcp";
-constexpr DNSServiceFlags kBrowseFlags = 0;
 }
 
 MTROperationalBrowser::MTROperationalBrowser(MTRDeviceControllerFactory * aFactory, dispatch_queue_t aQueue)
@@ -43,23 +46,30 @@
 {
     assertChipStackLockedByCurrentThread();
 
-    ChipLogProgress(Controller, "Trying to start operational browse");
+    ChipLogProgress(Controller, "Trying to start persistent operational browse");
 
-    auto err
-        = DNSServiceBrowse(&mBrowseRef, kBrowseFlags, kDNSServiceInterfaceIndexAny, kOperationalType, kLocalDot, OnBrowse, this);
+    auto err = DNSServiceCreateConnection(&mBrowseRef);
     if (err != kDNSServiceErr_NoError) {
-        ChipLogError(Controller, "Failed to start operational browse: %" PRId32, err);
+        ChipLogError(Controller, "Failed to create connection for persistent operational browse: %" PRId32, err);
         return;
     }
 
     err = DNSServiceSetDispatchQueue(mBrowseRef, mQueue);
     if (err != kDNSServiceErr_NoError) {
-        ChipLogError(Controller, "Failed to set up dispatch queue properly");
+        ChipLogError(Controller, "Failed to set up dispatch queue properly for persistent operational browse: %" PRId32, err);
         DNSServiceRefDeallocate(mBrowseRef);
         return;
     }
 
     mInitialized = true;
+
+    for (auto domain : kBrowseDomains) {
+        auto browseRef = mBrowseRef; // Mandatory copy because of kDNSServiceFlagsShareConnection.
+        err = DNSServiceBrowse(&browseRef, kDNSServiceFlagsShareConnection, kDNSServiceInterfaceIndexAny, kOperationalType, domain, OnBrowse, this);
+        if (err != kDNSServiceErr_NoError) {
+            ChipLogError(Controller, "Failed to start persistent operational browse for \"%s\" domain: %" PRId32, StringOrNullMarker(domain), err);
+        }
+    }
 }
 
 void MTROperationalBrowser::OnBrowse(DNSServiceRef aServiceRef, DNSServiceFlags aFlags, uint32_t aInterfaceId,