Skip to content

Commit

Permalink
Add a continuous browse for Matter operational advertisements on Darwin.
Browse files Browse the repository at this point in the history
The information is propagated to MTRDevice, but we're not making use of it there
yet.

The fabricIndex changes were due to a pre-existing issue: we are in fact getting
it from the wrong thread/queue in various places (MTRDevice init, MTRDevice
command invocation), such that an assertChipStackLockedByCurrentThread() in the
getter failed.
  • Loading branch information
bzbarsky-apple committed Feb 24, 2023
1 parent 53142c7 commit 91874dd
Show file tree
Hide file tree
Showing 9 changed files with 268 additions and 4 deletions.
11 changes: 11 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,17 @@ - (void)invalidate
os_unfair_lock_unlock(&self->_lock);
}

- (void)nodeMayBeAdvertisingOperational
{
// TODO: Figure out what to do with that information. If we're not waiting
// to subscribe/resubscribe, do nothing, otherwise perhaps trigger the
// subscribe/resubscribe immediately? We need to have much better tracking
// of our internal state for that, and may need to add something on
// ReadClient to cancel its outstanding timer and try to resubscribe
// immediately....
MTR_LOG_DEFAULT("%@ saw new operational advertisement", self);
}

// assume lock is held
- (void)_changeState:(MTRDeviceState)state
{
Expand Down
43 changes: 40 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@
#include <credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.h>
#include <credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h>
#include <lib/core/CHIPVendorIdentifiers.hpp>
#include <platform/LockTracker.h>
#include <platform/PlatformManager.h>
#include <setup_payload/ManualSetupPayloadGenerator.h>
#include <system/SystemClock.h>

#include <atomic>

#import <os/lock.h>

static NSString * const kErrorCommissionerInit = @"Init failure while initializing a commissioner";
Expand Down Expand Up @@ -82,7 +85,10 @@
typedef id (^SyncWorkQueueBlockWithReturnValue)(void);
typedef BOOL (^SyncWorkQueueBlockWithBoolReturnValue)(void);

@interface MTRDeviceController ()
@interface MTRDeviceController () {
// Atomic because it can be touched from multiple threads.
std::atomic<chip::FabricIndex> _storedFabricIndex;
}

// queue used to serialize all work performed by the MTRDeviceController
@property (atomic, readonly) dispatch_queue_t chipWorkQueue;
Expand Down Expand Up @@ -123,6 +129,8 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory queue:(dis
if ([self checkForInitError:(_operationalCredentialsDelegate != nullptr) logMsg:kErrorOperationalCredentialsInit]) {
return nil;
}

_storedFabricIndex = chip::kUndefinedFabricIndex;
}
return self;
}
Expand Down Expand Up @@ -152,12 +160,15 @@ - (void)cleanupAfterStartup
// in a very specific way that only MTRDeviceControllerFactory knows about.
- (void)shutDownCppController
{
assertChipStackLockedByCurrentThread();

if (_cppCommissioner) {
auto * commissionerToShutDown = _cppCommissioner;
// Flag ourselves as not running before we start shutting down
// _cppCommissioner, so we're not in a state where we claim to be
// running but are actually partially shut down.
_cppCommissioner = nullptr;
_storedFabricIndex = chip::kUndefinedFabricIndex;
commissionerToShutDown->Shutdown();
delete commissionerToShutDown;
if (_operationalCredentialsDelegate != nil) {
Expand Down Expand Up @@ -345,6 +356,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
return;
}

self->_storedFabricIndex = fabricIdx;
commissionerInitialized = YES;
});

Expand Down Expand Up @@ -813,17 +825,26 @@ - (BOOL)syncRunOnWorkQueueWithBoolReturnValue:(SyncWorkQueueBlockWithBoolReturnV

- (chip::FabricIndex)fabricIndex
{
return _storedFabricIndex;
}

- (nullable NSNumber *)compressedFabricID
{
assertChipStackLockedByCurrentThread();

if (!_cppCommissioner) {
return chip::kUndefinedFabricIndex;
return nil;
}

return _cppCommissioner->GetFabricIndex();
return @(_cppCommissioner->GetCompressedFabricId());
}

- (CHIP_ERROR)isRunningOnFabric:(chip::FabricTable *)fabricTable
fabricIndex:(chip::FabricIndex)fabricIndex
isRunning:(BOOL *)isRunning
{
assertChipStackLockedByCurrentThread();

if (![self isRunning]) {
*isRunning = NO;
return CHIP_NO_ERROR;
Expand Down Expand Up @@ -861,6 +882,22 @@ - (void)invalidateCASESessionForNode:(chip::NodeId)nodeID;
[self syncRunOnWorkQueue:block error:nil];
}

- (void)operationalInstanceAdded:(chip::NodeId)nodeID
{
// Don't use deviceForNodeID here, because we don't want to create the
// device if it does not already exist.
os_unfair_lock_lock(&_deviceMapLock);
MTRDevice * device = self.nodeIDToDeviceMap[@(nodeID)];
os_unfair_lock_unlock(&_deviceMapLock);

if (device == nil) {
return;
}

ChipLogProgress(Controller, "Notifying device about node 0x" ChipLogFormatX64 " advertising", ChipLogValueX64(nodeID));
[device nodeMayBeAdvertisingOperational];
}

@end

/**
Expand Down
24 changes: 24 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#import "MTRFramework.h"
#import "MTRLogging_Internal.h"
#import "MTROTAProviderDelegateBridge.h"
#import "MTROperationalBrowser.h"
#import "MTRP256KeypairBridge.h"
#import "MTRPersistentStorageDelegateBridge.h"
#import "NSDataSpanConversion.h"
Expand Down Expand Up @@ -81,6 +82,7 @@ @interface MTRDeviceControllerFactory ()
@property (readonly) NSMutableArray<MTRDeviceController *> * controllers;
@property (readonly) PersistentStorageOperationalKeystore * keystore;
@property (readonly) Credentials::PersistentStorageOpCertStore * opCertStore;
@property (readonly) MTROperationalBrowser * operationalBrowser;
@property () chip::Credentials::DeviceAttestationVerifier * deviceAttestationVerifier;

- (BOOL)findMatchingFabric:(FabricTable &)fabricTable
Expand Down Expand Up @@ -643,6 +645,9 @@ - (MTRDeviceController * _Nullable)createController
// Bringing up the first controller. Start the event loop now. If we
// fail to bring it up, its cleanup will stop the event loop again.
chip::DeviceLayer::PlatformMgrImpl().StartEventLoopTask();
dispatch_sync(_chipWorkQueue, ^{
self->_operationalBrowser = new MTROperationalBrowser(self, self->_chipWorkQueue);
});
}

// Add the controller to _controllers now, so if we fail partway through its
Expand Down Expand Up @@ -742,6 +747,10 @@ - (void)controllerShuttingDown:(MTRDeviceController *)controller
[_controllers removeObject:controller];

if ([_controllers count] == 0) {
dispatch_sync(_chipWorkQueue, ^{
delete self->_operationalBrowser;
self->_operationalBrowser = nullptr;
});
// That was our last controller. Stop the event loop before it
// shuts down, because shutdown of the last controller will tear
// down most of the world.
Expand Down Expand Up @@ -777,6 +786,21 @@ - (nullable MTRDeviceController *)runningControllerForFabricIndex:(chip::FabricI
return nil;
}

- (void)operationalInstanceAdded:(chip::PeerId &)operationalID
{
for (MTRDeviceController * controller in _controllers) {
auto * compressedFabricId = controller.compressedFabricID;
if (compressedFabricId != nil && compressedFabricId.unsignedLongLongValue == operationalID.GetCompressedFabricId()) {
ChipLogProgress(Controller, "Notifying controller at fabric index %u about new operational node 0x" ChipLogFormatX64,
controller.fabricIndex, ChipLogValueX64(operationalID.GetNodeId()));
[controller operationalInstanceAdded:operationalID.GetNodeId()];
}

// Keep going: more than one controller might match a given compressed
// fabric id, though the chances are low.
}
}

- (MTRPersistentStorageDelegateBridge *)storageDelegateBridge
{
return _persistentStorageDelegateBridge;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#import "MTRDeviceControllerFactory.h"

#include <lib/core/DataModelTypes.h>
#include <lib/core/PeerId.h>

class MTRPersistentStorageDelegateBridge;

Expand All @@ -47,6 +48,12 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (nullable MTRDeviceController *)runningControllerForFabricIndex:(chip::FabricIndex)fabricIndex;

/**
* Notify the controller factory that a new operational instance with the given
* compressed fabric id and node id has been observed.
*/
- (void)operationalInstanceAdded:(chip::PeerId &)operationalID;

@property (readonly) MTRPersistentStorageDelegateBridge * storageDelegateBridge;
@property (readonly) chip::Credentials::GroupDataProvider * groupData;
@property (readonly) chip::Credentials::DeviceAttestationVerifier * deviceAttestationVerifier;
Expand Down
15 changes: 14 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,17 @@ NS_ASSUME_NONNULL_BEGIN

/**
* Will return chip::kUndefinedFabricIndex if we do not have a fabric index.
* This property MUST be gotten from the Matter work queue.
*/
@property (readonly) chip::FabricIndex fabricIndex;

/**
* Will return the compressed fabric id of the fabric if the controller is
* running, else nil.
*
* This property MUST be gotten from the Matter work queue.
*/
@property (readonly, nullable) NSNumber * compressedFabricID;

/**
* Init a newly created controller.
*
Expand Down Expand Up @@ -185,6 +192,12 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID;

/**
* Notify the controller that a new operational instance with the given node id
* and a compressed fabric id that matches this controller has been observed.
*/
- (void)operationalInstanceAdded:(chip::NodeId)nodeID;

#pragma mark - Device-specific data and SDK access
// DeviceController will act as a central repository for this opaque dictionary that MTRDevice manages
- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID;
Expand Down
5 changes: 5 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ typedef void (^MTRDevicePerformAsyncBlock)(MTRBaseDevice * baseDevice);
// called by controller to clean up and shutdown
- (void)invalidate;

// Called by controller when a new operational advertisement for what we think
// is this device's identity has been observed. This could have
// false-positives, for example due to compressed fabric id collisions.
- (void)nodeMayBeAdvertisingOperational;

@property (nonatomic, readonly) MTRDeviceController * deviceController;
@property (nonatomic, readonly, copy) NSNumber * nodeID;
// Queue used for various internal bookkeeping work. In general endWork calls
Expand Down
47 changes: 47 additions & 0 deletions src/darwin/Framework/CHIP/MTROperationalBrowser.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright (c) 2023 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#import <Foundation/Foundation.h>
#import <Matter/MTRDeviceControllerFactory.h>
#import <dns_sd.h>

class MTROperationalBrowser
{
public:
// Should be created at a point when the factory starts up the event loop,
// and destroyed when the event loop is stopped.
MTROperationalBrowser(MTRDeviceControllerFactory * aFactory, dispatch_queue_t aQueue);

~MTROperationalBrowser();

private:
static void OnBrowse(DNSServiceRef aServiceRef, DNSServiceFlags aFlags, uint32_t aInterfaceId, DNSServiceErrorType aError,
const char * aName, const char * aType, const char * aDomain, void * aContext);

void TryToStartBrowse();

MTRDeviceControllerFactory * const mDeviceControllerFactory;
dispatch_queue_t mQueue;
DNSServiceRef mBrowseRef;

// If mInitialized is true, mBrowseRef is valid.
bool mInitialized = false;

// If mIsDestroying is true, we're in our destructor, shutting things down.
bool mIsDestroying = false;
};
Loading

0 comments on commit 91874dd

Please sign in to comment.