Skip to content

Commit

Permalink
[Darwin] Fix OTAProviderDelegate init after all controllers has been …
Browse files Browse the repository at this point in the history
…turned off and a new controller is started (#21412)

* Add a test named testControllerWithOTAProviderDelegate to ensure that starting a new controller once all the controllers has been shut down does not fail

* Fix OTA delegate bridge init on Darwin
  • Loading branch information
vivien-apple authored and web-flow committed Jul 29, 2022
1 parent 079a65d commit abd196a
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 10 deletions.
32 changes: 25 additions & 7 deletions src/darwin/Framework/CHIP/MTRControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ @interface MTRControllerFactory ()
- (BOOL)findMatchingFabric:(FabricTable &)fabricTable
params:(MTRDeviceControllerStartupParams *)params
fabric:(const FabricInfo * _Nullable * _Nonnull)fabric;

- (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceController * _Nonnull)controller;
@end

@implementation MTRControllerFactory
Expand Down Expand Up @@ -387,7 +389,7 @@ - (MTRDeviceController * _Nullable)startControllerOnExistingFabric:(MTRDeviceCon
return nil;
}

return controller;
return [self maybeInitializeOTAProvider:controller];
}

- (MTRDeviceController * _Nullable)startControllerOnNewFabric:(MTRDeviceControllerStartupParams *)startupParams
Expand Down Expand Up @@ -447,7 +449,7 @@ - (MTRDeviceController * _Nullable)startControllerOnNewFabric:(MTRDeviceControll
return nil;
}

return controller;
return [self maybeInitializeOTAProvider:controller];
}

- (MTRDeviceController * _Nullable)createController
Expand All @@ -462,11 +464,6 @@ - (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();

if (_otaProviderDelegateBridge) {
auto systemState = _controllerFactory->GetSystemState();
_otaProviderDelegateBridge->Init(systemState->SystemLayer(), systemState->ExchangeMgr());
}
}

// Add the controller to _controllers now, so if we fail partway through its
Expand Down Expand Up @@ -516,6 +513,27 @@ - (BOOL)findMatchingFabric:(FabricTable &)fabricTable
return YES;
}

// Initialize the MTROTAProviderDelegateBridge if it has not been initialized already
//
// Returns nil on failure, the input controller on success.
// If the provider has been initialized already, it is not considered as a failure.
//
- (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceController * _Nonnull)controller
{
VerifyOrReturnValue(_otaProviderDelegateBridge != nil, controller);
VerifyOrReturnValue([_controllers count] == 1, controller);

auto systemState = _controllerFactory->GetSystemState();
CHIP_ERROR err = _otaProviderDelegateBridge->Init(systemState->SystemLayer(), systemState->ExchangeMgr());
if (CHIP_NO_ERROR != err) {
MTR_LOG_ERROR("Failed to init provider delegate bridge: %" CHIP_ERROR_FORMAT, err.Format());
[controller shutdown];
return nil;
}

return controller;
}

@end

@implementation MTRControllerFactory (InternalMethods)
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
MTROTAProviderDelegateBridge(id<MTROTAProviderDelegate> delegate);
~MTROTAProviderDelegateBridge();

void Init(chip::System::Layer * systemLayer, chip::Messaging::ExchangeManager * exchangeManager);
CHIP_ERROR Init(chip::System::Layer * systemLayer, chip::Messaging::ExchangeManager * exchangeManager);
void Shutdown();

void HandleQueryImage(
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,9 @@ void ResetState()
Clusters::OTAProvider::SetDelegate(kOtaProviderEndpoint, nullptr);
}

void MTROTAProviderDelegateBridge::Init(System::Layer * systemLayer, Messaging::ExchangeManager * exchangeManager)
CHIP_ERROR MTROTAProviderDelegateBridge::Init(System::Layer * systemLayer, Messaging::ExchangeManager * exchangeManager)
{
gOtaSender.Init(systemLayer, exchangeManager);
return gOtaSender.Init(systemLayer, exchangeManager);
}

void MTROTAProviderDelegateBridge::Shutdown() { gOtaSender.Shutdown(); }
Expand Down
57 changes: 57 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRControllerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#import <XCTest/XCTest.h>

#import "MTRTestKeys.h"
#import "MTRTestOTAProvider.h"
#import "MTRTestStorage.h"

static uint16_t kTestVendorId = 0xFFF1u;
Expand Down Expand Up @@ -158,6 +159,62 @@ - (void)testControllerMultipleShutdown
XCTAssertFalse([factory isRunning]);
}

- (void)testControllerWithOTAProviderDelegate
{
__auto_type * factory = [MTRControllerFactory sharedInstance];
XCTAssertNotNil(factory);

__auto_type * otaProvider = [[MTRTestOTAProvider alloc] init];
__auto_type * storage = [[MTRTestStorage alloc] init];
__auto_type * factoryParams = [[MTRControllerFactoryParams alloc] initWithStorage:storage];
factoryParams.otaProviderDelegate = otaProvider;
XCTAssertTrue([factory startup:factoryParams]);
XCTAssertTrue([factory isRunning]);

__auto_type * testKeys = [[MTRTestKeys alloc] init];
XCTAssertNotNil(testKeys);

__auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:testKeys fabricId:1 ipk:testKeys.ipk];
XCTAssertNotNil(params);

params.vendorId = @(kTestVendorId);
MTRDeviceController * controller = [factory startControllerOnNewFabric:params];
XCTAssertTrue([controller isRunning]);
[controller shutdown];

// OTA Provider depends on the system state maintained by CHIPDeviceControllerFactory that is destroyed when
// the controller count goes down to 0. Make sure that a new controller can still be started successfully onto the
// same fabric.
MTRDeviceController * controller2 = [factory startControllerOnExistingFabric:params];
XCTAssertTrue([controller2 isRunning]);
[controller2 shutdown];

// Check that a new controller can be started on a different fabric too.
__auto_type * params2 = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:testKeys fabricId:2 ipk:testKeys.ipk];
XCTAssertNotNil(params2);

params2.vendorId = @(kTestVendorId);

MTRDeviceController * controller3 = [factory startControllerOnNewFabric:params2];
XCTAssertTrue([controller3 isRunning]);
[controller3 shutdown];

// Stop the factory, start it up again and create a controller to ensure that no dead state from the previous
// ota provider delegate is staying around.
[factory shutdown];
XCTAssertFalse([factory isRunning]);

XCTAssertTrue([factory startup:factoryParams]);
XCTAssertTrue([factory isRunning]);

MTRDeviceController * controller4 = [factory startControllerOnExistingFabric:params2];
XCTAssertTrue([controller4 isRunning]);
[controller4 shutdown];

[factory shutdown];
XCTAssertFalse([factory isRunning]);
}

- (void)testControllerInvalidAccess
{
__auto_type * factory = [MTRControllerFactory sharedInstance];
Expand Down
25 changes: 25 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRTestOTAProvider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Copyright (c) 2022 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.
*/

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

NS_ASSUME_NONNULL_BEGIN

@interface MTRTestOTAProvider : NSObject <MTROTAProviderDelegate>
@end

NS_ASSUME_NONNULL_END
57 changes: 57 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRTestOTAProvider.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* Copyright (c) 2022 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.
*/

#import "MTRTestOTAProvider.h"

@interface MTRTestOTAProvider ()
@end

@implementation MTRTestOTAProvider
- (void)handleQueryImage:(MTROtaSoftwareUpdateProviderClusterQueryImageParams *)params
completionHandler:(void (^)(MTROtaSoftwareUpdateProviderClusterQueryImageResponseParams * _Nullable data,
NSError * _Nullable error))completionHandler
{
}

- (void)handleApplyUpdateRequest:(MTROtaSoftwareUpdateProviderClusterApplyUpdateRequestParams *)params
completionHandler:(void (^)(MTROtaSoftwareUpdateProviderClusterApplyUpdateResponseParams * _Nullable data,
NSError * _Nullable error))completionHandler
{
}

- (void)handleNotifyUpdateApplied:(MTROtaSoftwareUpdateProviderClusterNotifyUpdateAppliedParams *)params
completionHandler:(StatusCompletion)completionHandler
{
}

- (void)handleBDXTransferSessionBegin:(NSString * _Nonnull)fileDesignator
offset:(NSNumber * _Nonnull)offset
completionHandler:(void (^)(NSError * error))completionHandler
{
}

- (void)handleBDXTransferSessionEnd:(NSError * _Nullable)error
{
}

- (void)handleBDXQuery:(NSNumber * _Nonnull)blockSize
blockIndex:(NSNumber * _Nonnull)blockIndex
bytesToSkip:(NSNumber * _Nonnull)bytesToSkip
completionHandler:(void (^)(NSData * _Nullable data, BOOL isEOF))completionHandler
{
}

@end
6 changes: 6 additions & 0 deletions src/darwin/Framework/Matter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
objects = {

/* Begin PBXBuildFile section */
1E5801C328941C050033A199 /* MTRTestOTAProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E748B3828941A44008A1BE8 /* MTRTestOTAProvider.m */; };
1E85730C265519AE0050A4D9 /* callback-stub.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1E857307265519AE0050A4D9 /* callback-stub.cpp */; };
1EB41B7B263C4CC60048E4C1 /* MTRClustersTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 1EB41B7A263C4CC60048E4C1 /* MTRClustersTests.m */; };
1EC3238D271999E2002A8BF0 /* cluster-objects.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1EC3238C271999E2002A8BF0 /* cluster-objects.cpp */; };
Expand Down Expand Up @@ -127,6 +128,8 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
1E748B3828941A44008A1BE8 /* MTRTestOTAProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRTestOTAProvider.m; sourceTree = "<group>"; };
1E748B3928941A45008A1BE8 /* MTRTestOTAProvider.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRTestOTAProvider.h; sourceTree = "<group>"; };
1E857307265519AE0050A4D9 /* callback-stub.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "callback-stub.cpp"; path = "../../../../zzz_generated/controller-clusters/zap-generated/callback-stub.cpp"; sourceTree = "<group>"; };
1EB41B7A263C4CC60048E4C1 /* MTRClustersTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRClustersTests.m; sourceTree = "<group>"; };
1EC3238C271999E2002A8BF0 /* cluster-objects.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "cluster-objects.cpp"; path = "../../../../zzz_generated/app-common/app-common/zap-generated/cluster-objects.cpp"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -404,6 +407,8 @@
B202529A2459E34F00F97062 /* CHIPTests */ = {
isa = PBXGroup;
children = (
1E748B3928941A45008A1BE8 /* MTRTestOTAProvider.h */,
1E748B3828941A44008A1BE8 /* MTRTestOTAProvider.m */,
D437613E285BDC0D0051FEA2 /* MTRErrorTestUtils.h */,
D437613F285BDC0D0051FEA2 /* MTRTestKeys.h */,
D4376140285BDC0D0051FEA2 /* MTRTestStorage.h */,
Expand Down Expand Up @@ -667,6 +672,7 @@
997DED1A26955D0200975E97 /* MTRThreadOperationalDatasetTests.mm in Sources */,
51C8E3F82825CDB600D47D00 /* MTRTestKeys.m in Sources */,
99C65E10267282F1003402F6 /* MTRControllerTests.m in Sources */,
1E5801C328941C050033A199 /* MTRTestOTAProvider.m in Sources */,
5A6FEC9D27B5E48900F25F42 /* MTRXPCProtocolTests.m in Sources */,
5AE6D4E427A99041001F2493 /* MTRDeviceTests.m in Sources */,
5A7947DE27BEC3F500434CF2 /* MTRXPCListenerSampleTests.m in Sources */,
Expand Down

0 comments on commit abd196a

Please sign in to comment.