Skip to content

Commit

Permalink
[Matter.framework] Get downloadLogOfType to work over XPC (#32319)
Browse files Browse the repository at this point in the history
* [Matter.framework] Move downloadLogOfType from MTRDevice to MTRBaseDevice

* [Matter.framework] Get downloadLogOfType to work over XPC

* Add a simple test to MTRXPCProtocolTests.m to check that the XPC protocol works

* Add a simple test to MTRXPCListenerSampleTests

* [Matter.framework] Do not delete the file containing the log data once the downloadLogOfType completion is done
  • Loading branch information
vivien-apple authored and pull[bot] committed Sep 13, 2024
1 parent 1c23fe2 commit 1125776
Show file tree
Hide file tree
Showing 12 changed files with 181 additions and 37 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/darwin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ jobs:
# target versions instead?
run: |
mkdir -p /tmp/darwin/framework-tests
../../../out/debug/chip-all-clusters-app --interface-id -1 > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
echo "This is a simple log" > /tmp/darwin/framework-tests/end_user_support_log.txt
../../../out/debug/chip-all-clusters-app --interface-id -1 --end_user_support_log /tmp/darwin/framework-tests/end_user_support_log.txt > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
../../../out/debug/chip-all-clusters-app --interface-id -1 --dac_provider ../../../credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json --product-id 32768 --discriminator 3839 --secured-device-port 5539 --KVS /tmp/chip-all-clusters-app-kvs2 > >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid-err.log >&2) &
# Disable BLE (CHIP_IS_BLE=NO) because the app does not have the permission to use it and that may crash the CI.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
ChipLogProgress(chipTool, "Downloading logs from node 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId));

MTRDeviceController * commissioner = CurrentCommissioner();
auto * device = [MTRDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];

auto logType = static_cast<MTRDiagnosticLogType>(mLogType);
auto queue = dispatch_queue_create("com.chip.bdx.downloader", DISPATCH_QUEUE_SERIAL);
Expand Down
21 changes: 21 additions & 0 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#import <Matter/MTRCluster.h>
#import <Matter/MTRDefines.h>
#import <Matter/MTRDiagnosticLogsType.h>

@class MTRSetupPayload;
@class MTRDeviceController;
Expand Down Expand Up @@ -542,6 +543,26 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
reportHandler:(MTRDeviceResponseHandler)reportHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished
MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));

/**
* Download log of the desired type from the device.
*
* @param type The type of log being requested. This should correspond to a value in the enum MTRDiagnosticLogType.
* @param timeout The timeout for getting the log. If the timeout expires, completion will be called with whatever
* has been retrieved by that point (which might be none or a partial log).
* If the timeout is set to 0, the request will not expire and completion will not be called until
* the log is fully retrieved or an error occurs.
* @param queue The queue on which completion will be called.
* @param completion The completion handler that is called after attempting to retrieve the requested log.
* - In case of success, the completion handler is called with a non-nil URL and a nil error.
* - If there is an error, a non-nil error is used and the url can be non-nil too if some logs have already been downloaded.
*/
- (void)downloadLogOfType:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
queue:(dispatch_queue_t)queue
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
MTR_NEWLY_AVAILABLE;

@end

/**
Expand Down
13 changes: 13 additions & 0 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2147,6 +2147,19 @@ + (NSDictionary *)eventReportForHeader:(const chip::app::EventHeader &)header an

return buffer;
}

- (void)downloadLogOfType:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
queue:(dispatch_queue_t)queue
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
{
[_deviceController downloadLogFromNodeWithID:@(_nodeID)
type:type
timeout:timeout
queue:queue
completion:completion];
}

@end

@implementation MTRBaseDevice (Deprecated)
Expand Down
22 changes: 0 additions & 22 deletions src/darwin/Framework/CHIP/MTRDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#import <Foundation/Foundation.h>
#import <Matter/MTRBaseDevice.h>
#import <Matter/MTRDefines.h>
#import <Matter/MTRDiagnosticLogsType.h>

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -326,27 +325,6 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
*/
- (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID MTR_UNSTABLE_API;

/**
* Download log of the desired type from the device.
*
* Note: The consumer of this API should move the file that the url points to or open it for reading before the
* completion handler returns. Otherwise, the file will be deleted, and the data will be lost.
*
* @param type The type of log being requested. This should correspond to a value in the enum MTRDiagnosticLogType.
* @param timeout The timeout for getting the log. If the timeout expires, completion will be called with whatever
* has been retrieved by that point (which might be none or a partial log).
* If the timeout is set to 0, the request will not expire and completion will not be called until
* the log is fully retrieved or an error occurs.
* @param queue The queue on which completion will be called.
* @param completion The completion handler that is called after attempting to retrieve the requested log.
* - In case of success, the completion handler is called with a non-nil URL and a nil error.
* - If there is an error, a non-nil error is used and the url can be non-nil too if some logs have already been downloaded.
*/
- (void)downloadLogOfType:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
queue:(dispatch_queue_t)queue
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
MTR_NEWLY_AVAILABLE;
@end

MTR_EXTERN NSString * const MTRPreviousDataKey MTR_NEWLY_AVAILABLE;
Expand Down
12 changes: 0 additions & 12 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1713,18 +1713,6 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator
[baseDevice openCommissioningWindowWithDiscriminator:discriminator duration:duration queue:queue completion:completion];
}

- (void)downloadLogOfType:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
queue:(dispatch_queue_t)queue
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
{
[_deviceController downloadLogFromNodeWithID:_nodeID
type:type
timeout:timeout
queue:queue
completion:completion];
}

#pragma mark - Cache management

// assume lock is held
Expand Down
10 changes: 10 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController+XPC.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#import <Matter/MTRCluster.h>
#import <Matter/MTRDefines.h>
#import <Matter/MTRDeviceController.h>
#import <Matter/MTRDiagnosticLogsType.h>

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -183,6 +184,15 @@ typedef void (^MTRValuesHandler)(id _Nullable values, NSError * _Nullable error)
attributeId:(NSNumber * _Nullable)attributeId
completion:(MTRValuesHandler)completion;

/**
* Requests downloading some logs
*/
- (void)downloadLogWithController:(id _Nullable)controller
nodeId:(uint64_t)nodeId
type:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
completion:(void (^)(NSString * _Nullable url, NSError * _Nullable error))completion;

@end

/**
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController+XPC.mm
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ + (NSXPCInterface *)xpcInterfaceForServerProtocol
forSelector:@selector(readAttributeCacheWithController:nodeId:endpointId:clusterId:attributeId:completion:)
argumentIndex:0
ofReply:YES];
[xpcInterface setClasses:GetXPCAllowedClasses()
forSelector:@selector(downloadLogWithController:nodeId:type:timeout:completion:)
argumentIndex:0
ofReply:YES];
return xpcInterface;
}

Expand Down
32 changes: 32 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,38 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator
});
}

- (void)downloadLogOfType:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
queue:(dispatch_queue_t)queue
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
{
MTR_LOG_DEBUG("Downloading log ...");

__auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
if (error != nil) {
completion(nil, error);
return;
}

[handle.proxy downloadLogWithController:self.controllerID
nodeId:self.nodeID.unsignedLongLongValue
type:type
timeout:timeout
completion:^(NSString * _Nullable url, NSError * _Nullable error) {
dispatch_async(queue, ^{
MTR_LOG_DEBUG("Download log");
completion([NSURL URLWithString:url], error);
// The following captures the proxy handle in the closure so that the
// handle won't be released prior to block call.
__auto_type handleRetainer = handle;
(void) handleRetainer;
});
}];
};

[self fetchProxyHandleWithQueue:queue completion:workBlock];
}

- (void)fetchProxyHandleWithQueue:(dispatch_queue_t)queue completion:(MTRFetchProxyHandleCompletion)completion
{
if (self.controllerID != nil) {
Expand Down
1 change: 0 additions & 1 deletion src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ - (instancetype)initWithType:(MTRDiagnosticLogType)type
// data in the logs that the caller may find useful. For this reason, fileURL is passed in even
// when there is an error but fileHandle is not nil.
completion(strongSelf->_fileHandle ? fileURL : nil, bdxError);
[strongSelf deleteFile];

done(strongSelf);
}
Expand Down
46 changes: 46 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,28 @@ - (void)getAnyDeviceControllerWithCompletion:(void (^)(id _Nullable controller,
completion(MTRDeviceControllerId, nil);
}

- (void)downloadLogWithController:(id)controller
nodeId:(uint64_t)nodeId
type:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
completion:(void (^)(NSString * _Nullable url, NSError * _Nullable error))completion
{
(void) controller;
__auto_type sharedController = sController;
if (sharedController) {
__auto_type device = [MTRBaseDevice deviceWithNodeID:@(nodeId) controller:sharedController];
[device downloadLogOfType:type
timeout:timeout
queue:dispatch_get_main_queue()
completion:^(NSURL * _Nullable url, NSError * _Nullable error) {
completion([url absoluteString], error);
}];
} else {
NSLog(@"Failed to get shared controller");
completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
}
}

- (void)readAttributeWithController:(id)controller
nodeId:(uint64_t)nodeId
endpointId:(NSNumber * _Nullable)endpointId
Expand Down Expand Up @@ -1865,6 +1887,30 @@ - (void)test015_MTRDeviceInteraction
}, @(NO));
}

- (void)test016_DownloadLog
{
XCTestExpectation * expectation =
[self expectationWithDescription:@"Download EndUserSupport log"];

MTRBaseDevice * device = GetConnectedDevice();
dispatch_queue_t queue = dispatch_get_main_queue();

[device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport
timeout:10
queue:queue
completion:^(NSURL * _Nullable url, NSError * _Nullable error) {
NSLog(@"downloadLogOfType: url: %@, error: %@", url, error);
XCTAssertNil(error);

NSError * readError;
NSString * fileContent = [NSString stringWithContentsOfURL:url encoding:NSUTF8StringEncoding error:&readError];
XCTAssertNil(readError);
XCTAssertTrue([fileContent isEqualToString:@"This is a simple log\n"]);
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil];
}

- (void)test900_SubscribeClusterStateCache
{
XCTestExpectation * expectation = [self expectationWithDescription:@"subscribe attributes by cache"];
Expand Down
52 changes: 52 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ @interface MTRXPCProtocolTests<NSXPCListenerDelegate, CHIPRemoteDeviceProtocol>
@property (readwrite, strong) void (^handleReadClusterStateCache)
(id controller, NSNumber * nodeId, NSNumber * _Nullable endpointId, NSNumber * _Nullable clusterId,
NSNumber * _Nullable attributeId, void (^completion)(id _Nullable values, NSError * _Nullable error));
@property (readwrite, strong) void (^handleDownloadLog)(id controller, NSNumber * nodeId, MTRDiagnosticLogType type, NSTimeInterval timeout,
void (^completion)(NSString * _Nullable url, NSError * _Nullable error));

@end

Expand Down Expand Up @@ -274,6 +276,18 @@ - (void)readAttributeCacheWithController:(id _Nullable)controller
});
}

- (void)downloadLogWithController:(id)controller
nodeId:(uint64_t)nodeId
type:(MTRDiagnosticLogType)type
timeout:(NSTimeInterval)timeout
completion:(void (^)(NSString * _Nullable url, NSError * _Nullable error))completion
{
dispatch_async(dispatch_get_main_queue(), ^{
XCTAssertNotNil(self.handleDownloadLog);
self.handleDownloadLog(controller, @(nodeId), type, timeout, completion);
});
}

- (void)setUp
{
[self setContinueAfterFailure:NO];
Expand All @@ -300,6 +314,44 @@ - (void)tearDown
_xpcDisconnectExpectation = nil;
}

- (void)testDownloadLogSuccess
{
uint64_t myNodeId = 9876543210;
NSString * myBdxURL = @"bdx://foo";
NSTimeInterval myTimeout = 10;

XCTestExpectation * callExpectation = [self expectationWithDescription:@"XPC call received"];
XCTestExpectation * responseExpectation = [self expectationWithDescription:@"XPC response received"];

__auto_type uuid = self.controllerUUID;
_handleDownloadLog = ^(id controller, NSNumber * nodeId, MTRDiagnosticLogType type, NSTimeInterval timeout,
void (^completion)(NSString * _Nullable url, NSError * _Nullable error)) {
XCTAssertTrue([controller isEqualToString:uuid]);
XCTAssertEqual([nodeId unsignedLongLongValue], myNodeId);
[callExpectation fulfill];
completion(myBdxURL, nil);
};

__auto_type * device = [MTRBaseDevice deviceWithNodeID:@(myNodeId) controller:_remoteDeviceController];
NSLog(@"Device acquired. Downloading...");
[device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport
timeout:myTimeout
queue:dispatch_get_main_queue()
completion:^(NSURL * _Nullable url, NSError * _Nullable error) {
NSLog(@"Read url: %@", url);
XCTAssertNotNil(url);
XCTAssertNil(error);
[responseExpectation fulfill];
self.xpcDisconnectExpectation = [self expectationWithDescription:@"XPC Disconnected"];
}];

[self waitForExpectations:[NSArray arrayWithObjects:callExpectation, responseExpectation, nil] timeout:kTimeoutInSeconds];

// When download is done, connection should have been released
[self waitForExpectations:[NSArray arrayWithObject:_xpcDisconnectExpectation] timeout:kTimeoutInSeconds];
XCTAssertNil(_xpcConnection);
}

- (void)testReadAttributeSuccess
{
uint64_t myNodeId = 9876543210;
Expand Down

0 comments on commit 1125776

Please sign in to comment.