From 9b0413924d120256edcbb61911ebc54e3e4a7aee Mon Sep 17 00:00:00 2001 From: Vivien Nicolas Date: Tue, 30 Jan 2024 17:50:58 +0100 Subject: [PATCH] [darwin-framework-tool] Address post-landing comments of #31638 [CI] Add some tests for bdx download with darwin-framework-tool --- .../matter_chip_tool_adapter/encoder.py | 11 + .../commands/bdx/DownloadLogCommand.h | 11 +- .../commands/bdx/DownloadLogCommand.mm | 41 +- .../commands/common/RemoteDataModelLogger.h | 1 + .../commands/common/RemoteDataModelLogger.mm | 29 + .../suites/TestDiagnosticLogs-darwin.yaml | 515 ++++++++++++++++++ src/darwin/Framework/CHIP/MTRDevice.h | 5 +- .../Framework/CHIP/MTRDeviceController.mm | 15 +- .../CHIP/MTRDeviceControllerFactory.mm | 28 +- .../CHIP/MTRDiagnosticLogsDownloader.mm | 36 +- 10 files changed, 638 insertions(+), 54 deletions(-) create mode 100644 src/app/tests/suites/TestDiagnosticLogs-darwin.yaml diff --git a/examples/chip-tool/py_matter_chip_tool_adapter/matter_chip_tool_adapter/encoder.py b/examples/chip-tool/py_matter_chip_tool_adapter/matter_chip_tool_adapter/encoder.py index 355a2b18433e31..f4e8e6711d7105 100644 --- a/examples/chip-tool/py_matter_chip_tool_adapter/matter_chip_tool_adapter/encoder.py +++ b/examples/chip-tool/py_matter_chip_tool_adapter/matter_chip_tool_adapter/encoder.py @@ -148,6 +148,17 @@ } }, + 'Bdx': { + 'commands': { + 'Download': { + 'arguments': { + 'LogType': 'log-type', + }, + 'has_endpoint': False, + } + } + }, + 'DelayCommands': { 'alias': 'delay', 'commands': { diff --git a/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.h b/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.h index ce48270d28b6ac..8fa366464ec2a1 100644 --- a/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.h +++ b/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.h @@ -32,14 +32,23 @@ class DownloadLogCommand : public CHIPCommandBridge "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."); + AddArgument("async", 0, 1, &mIsAsyncCommand, + "By default the command waits for the download to finish before returning. If async is true the command will " + "not wait and the download will proceed in the background"); + AddArgument("filepath", &mFilePath, "An optional filepath to save the download log content to."); } /////////// CHIPCommandBridge Interface ///////// CHIP_ERROR RunCommand() override; - chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(10); } + chip::System::Clock::Timeout GetWaitDuration() const override + { + return chip::System::Clock::Seconds16(mTimeout > 0 ? mTimeout + 1 : 300); + } private: chip::NodeId mNodeId; uint8_t mLogType; uint16_t mTimeout; + chip::Optional mFilePath; + chip::Optional mIsAsyncCommand; }; diff --git a/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.mm b/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.mm index 494ec964f14b63..aff22f35524ca1 100644 --- a/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.mm +++ b/examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.mm @@ -21,6 +21,7 @@ #import "MTRError_Utils.h" #include "DownloadLogCommand.h" +#include "RemoteDataModelLogger.h" CHIP_ERROR DownloadLogCommand::RunCommand() { @@ -32,27 +33,49 @@ auto logType = static_cast(mLogType); auto queue = dispatch_queue_create("com.chip.bdx.downloader", DISPATCH_QUEUE_SERIAL); + bool isAsyncCommand = mIsAsyncCommand.ValueOr(false); + + bool dumpToFile = mFilePath.HasValue(); + auto * dumpFilePath = dumpToFile ? [NSString stringWithUTF8String:mFilePath.Value()] : nil; + auto * self = this; auto completion = ^(NSURL * url, NSError * error) { // A non-nil url indicates the presence of content, which can occur even in error scenarios like timeouts. + NSString * logContent = nil; if (nil != url) { NSError * readError = nil; auto * data = [NSData dataWithContentsOfURL:url options:NSDataReadingUncached error:&readError]; - VerifyOrReturn(nil == readError, self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(readError))); + if (nil != readError) { + VerifyOrReturn(isAsyncCommand, self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(readError))); + return; + } - auto * content = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; - NSLog(@"Content: %@", content); - } + logContent = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; + NSLog(@"Content: %@", logContent); - VerifyOrReturn(nil == error, self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(error))); + if (dumpToFile) { + NSError * writeError = nil; + auto * fileManager = [NSFileManager defaultManager]; + [fileManager copyItemAtPath:[url path] toPath:dumpFilePath error:&writeError]; + if (nil != writeError) { + VerifyOrReturn(isAsyncCommand, self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(readError))); + return; + } + } + } - // The url is nil when there are no logs on the target device. - if (nil == url) { - NSLog(@"No logs has been found onto node 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId)); + ChipLogProgress(chipTool, "Diagnostic logs transfer: %s", error ? "Error" : "Success"); + auto err = RemoteDataModelLogger::LogBdxDownload(logContent, error); + if (CHIP_NO_ERROR != err) { + VerifyOrDo(isAsyncCommand, self->SetCommandExitStatus(err)); + return; } - self->SetCommandExitStatus(CHIP_NO_ERROR); + + VerifyOrDo(isAsyncCommand, self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(error))); }; [device downloadLogOfType:logType timeout:mTimeout queue:queue completion:completion]; + + VerifyOrDo(!isAsyncCommand, SetCommandExitStatus(CHIP_NO_ERROR)); return CHIP_NO_ERROR; } diff --git a/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.h b/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.h index 39605e6fb56390..b2cd92df859deb 100644 --- a/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.h +++ b/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.h @@ -32,5 +32,6 @@ CHIP_ERROR LogCommandAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNumbe CHIP_ERROR LogAttributeErrorAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNumber * attributeId, NSError * error); CHIP_ERROR LogCommandErrorAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNumber * commandId, NSError * error); CHIP_ERROR LogGetCommissionerNodeId(NSNumber * nodeId); +CHIP_ERROR LogBdxDownload(NSString * content, NSError * error); void SetDelegate(RemoteDataModelLoggerDelegate * delegate); }; // namespace RemoteDataModelLogger diff --git a/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.mm b/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.mm index 760fe0bc998ed5..ff12cdc022e515 100644 --- a/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.mm +++ b/examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.mm @@ -35,6 +35,7 @@ constexpr char kClusterErrorIdKey[] = "clusterError"; constexpr char kValueKey[] = "value"; constexpr char kNodeIdKey[] = "nodeId"; +constexpr char kLogContentIdKey[] = "logContent"; constexpr char kBase64Header[] = "base64:"; @@ -204,5 +205,33 @@ CHIP_ERROR LogGetCommissionerNodeId(NSNumber * value) return gDelegate->LogJSON(valueStr.c_str()); } +CHIP_ERROR LogBdxDownload(NSString * content, NSError * error) +{ + VerifyOrReturnError(gDelegate != nullptr, CHIP_NO_ERROR); + + Json::Value rootValue; + rootValue[kValueKey] = Json::Value(); + + Json::Value jsonValue; + VerifyOrDie(CHIP_NO_ERROR == AsJsonValue(content, jsonValue)); + rootValue[kValueKey][kLogContentIdKey] = jsonValue; + + if (error) { + auto err = MTRErrorToCHIPErrorCode(error); + auto status = chip::app::StatusIB(err); + +#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT + auto statusName = chip::Protocols::InteractionModel::StatusName(status.mStatus); + rootValue[kValueKey][kErrorIdKey] = statusName; +#else + auto statusName = status.mStatus; + rootValue[kValueKey][kErrorIdKey] = chip::to_underlying(statusName); +#endif // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT + } + + auto valueStr = JsonToString(rootValue); + return gDelegate->LogJSON(valueStr.c_str()); +} + void SetDelegate(RemoteDataModelLoggerDelegate * delegate) { gDelegate = delegate; } }; // namespace RemoteDataModelLogger diff --git a/src/app/tests/suites/TestDiagnosticLogs-darwin.yaml b/src/app/tests/suites/TestDiagnosticLogs-darwin.yaml new file mode 100644 index 00000000000000..355e9142290cfb --- /dev/null +++ b/src/app/tests/suites/TestDiagnosticLogs-darwin.yaml @@ -0,0 +1,515 @@ +# Copyright (c) 2024 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. + +name: Diagnostic Logs Tests for Darwin + +config: + nodeId: 0x12344321 + cluster: "Bdx" + timeout: 180 + end_user_support_log_file_path: "/tmp/end_user_support_log.txt" + end_user_support_log_file_content: "End User Support Log Content" + network_diagnostics_log_file_path: "/tmp/network_diagnostics_log.txt" + + bdx_transfer_file_path_1: "/tmp/bdx_log_output_1.txt" + bdx_transfer_file_name_1: "bdx_log_output_1.txt" + bdx_transfer_file_path_2: "/tmp/bdx_log_output_2.txt" + bdx_transfer_file_name_2: "bdx_log_output_2.txt" + + long_log_file_content: + "Network Diagnostic Log Content is more than 1024 bytes + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ......................................................." + +tests: + # + # Set up the test by adding some destination log files for the target accessory: + # 1. End User Support + # 2. Network Diagnostics + # 3. Crash + # + # The first thing to do is to delete them if they exist. It could be some + # left over from a previous test run. + # + + - label: "Delete EndUserSupport logs" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: end_user_support_log_file_path + + - label: "Delete NetworkDiag logs" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: network_diagnostics_log_file_path + + - label: "Stop the accessory" + cluster: "SystemCommands" + command: "Stop" + + - label: "Create End User Support logs" + cluster: "SystemCommands" + command: "CreateFile" + arguments: + values: + - name: "filePath" + value: end_user_support_log_file_path + - name: "fileContent" + value: end_user_support_log_file_content + + - label: "Create NetworkDiag logs" + cluster: "SystemCommands" + command: "CreateFile" + arguments: + values: + - name: "filePath" + value: network_diagnostics_log_file_path + - name: "fileContent" + value: long_log_file_content + + - label: "Start the accessory with the destination logs files" + cluster: "SystemCommands" + command: "Start" + arguments: + values: + - name: "endUserSupportLogPath" + value: end_user_support_log_file_path + - name: "networkDiagnosticsLogPath" + value: network_diagnostics_log_file_path + + - label: "Wait for the commissioned device to be retrieved" + cluster: "DelayCommands" + command: "WaitForCommissionee" + arguments: + values: + - name: "nodeId" + value: nodeId + + - label: "Read End User Support log intent" + command: "Download" + arguments: + values: + - name: "LogType" + value: 0 # EndUserSupport + - name: "Timeout" + value: 0 + response: + values: + - name: "logContent" + value: end_user_support_log_file_content + constraints: + minLength: 28 + maxLength: 28 + + - label: "Read Network Diagnostic log intent" + command: "Download" + arguments: + values: + - name: "LogType" + value: 1 # Network Diagnostic + - name: "Timeout" + value: 0 + response: + values: + - name: "logContent" + value: long_log_file_content + constraints: + minLength: 9238 + maxLength: 9238 + + - label: "Read Crash log intent" + command: "Download" + arguments: + values: + - name: "LogType" + value: 2 # Crash + - name: "Timeout" + value: 0 + response: + values: + - name: "logContent" + value: "" + + - label: + "Read Network Diagnostic log intent with a very short timeout and a + very long log" + command: "Download" + arguments: + values: + - name: "LogType" + value: 1 # Network Diagnostic + - name: "Timeout" + value: 1 + response: + values: + - name: "logContent" + value: null + - name: "error" + value: "FAILURE" + + - label: + "Read End User Support log intent after a failure to make sure that + everything still works" + command: "Download" + arguments: + values: + - name: "LogType" + value: 0 # EndUserSupport + - name: "Timeout" + value: 0 + response: + values: + - name: "logContent" + value: end_user_support_log_file_content + constraints: + minLength: 28 + maxLength: 28 + # + # Validate that multiple BDX transfers can run in parallels. + # + - label: "Delete possible leftover from previous run" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: bdx_transfer_file_path_1 + + - label: "Delete possible leftover from previous run" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: bdx_transfer_file_path_2 + + - label: "Update End User Support logs with a long log" + cluster: "SystemCommands" + command: "CreateFile" + arguments: + values: + - name: "filePath" + value: end_user_support_log_file_path + - name: "fileContent" + value: long_log_file_content + + - label: "Update Network Diagnostic logs with a long log" + cluster: "SystemCommands" + command: "CreateFile" + arguments: + values: + - name: "filePath" + value: network_diagnostics_log_file_path + - name: "fileContent" + value: long_log_file_content + + - label: "Start a second accessory" + cluster: "SystemCommands" + command: "Start" + arguments: + values: + - name: "discriminator" + value: 50 + - name: "port" + value: 5601 + - name: "kvs" + value: "/tmp/chip_kvs_second" + - name: "endUserSupportLogPath" + value: end_user_support_log_file_path + - name: "networkDiagnosticsLogPath" + value: network_diagnostics_log_file_path + - name: "registerKey" + value: "default#2" + + - label: "Commission second accessory from beta" + identity: "beta" + cluster: "CommissionerCommands" + command: "PairWithCode" + arguments: + values: + - name: "nodeId" + value: nodeId + - name: "payload" + value: "MT:-24J0IX4122-.548G00" + + - label: "Wait for the second commissioned device to be retrieved for beta" + identity: "beta" + cluster: "DelayCommands" + command: "WaitForCommissionee" + arguments: + values: + - name: "nodeId" + value: nodeId + + - label: "BDX: Request End User Support from the first accessory" + command: "Download" + arguments: + values: + - name: "LogType" + value: 1 # Network Diagnostic + - name: "Timeout" + value: 0 + - name: "async" + value: true + - name: "filepath" + value: bdx_transfer_file_path_1 + + - label: "BDX: Request End User Support from the second accessory" + identity: "beta" + command: "Download" + arguments: + values: + - name: "LogType" + value: 1 # Network Diagnostic + - name: "Timeout" + value: 0 + - name: "async" + value: true + - name: "filepath" + value: bdx_transfer_file_path_2 + + - label: + "BDX: Wait for 'Diagnostic logs transfer: Success' message from the + first accessory" + cluster: "DelayCommands" + command: "WaitForMessage" + arguments: + values: + - name: "registerKey" + value: "default" + - name: "message" + value: "Diagnostic logs transfer: Success" + - name: "timeoutInSeconds" + value: 60 + + - label: + "BDX: Wait for 'Diagnostic logs transfer: Success' message from the + second accessory" + cluster: "DelayCommands" + command: "WaitForMessage" + arguments: + values: + - name: "registerKey" + value: "default#2" + - name: "message" + value: "Diagnostic logs transfer: Success" + - name: "timeoutInSeconds" + value: 60 + + - label: + "Compare the content the original log and the file that has been + created as the result of the BDX transfer" + cluster: "SystemCommands" + command: "CompareFiles" + arguments: + values: + - name: "file1" + value: end_user_support_log_file_path + - name: "file2" + value: bdx_transfer_file_path_1 + + - label: + "Compare the content the original log and the file that has been + created as the result of the BDX transfer" + cluster: "SystemCommands" + command: "CompareFiles" + arguments: + values: + - name: "file1" + value: network_diagnostics_log_file_path + - name: "file2" + value: bdx_transfer_file_path_2 + + - label: "Delete the result of the previous run" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: bdx_transfer_file_path_1 + + - label: "Delete the result of the previous run" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: bdx_transfer_file_path_2 diff --git a/src/darwin/Framework/CHIP/MTRDevice.h b/src/darwin/Framework/CHIP/MTRDevice.h index 9d3b0768ff4061..4f23880a19532b 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.h +++ b/src/darwin/Framework/CHIP/MTRDevice.h @@ -338,8 +338,9 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) * 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 that will be called to return the URL of the requested log if successful. Otherwise - * returns an error. + * @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 has already been downloaded. */ - (void)downloadLogOfType:(MTRDiagnosticLogType)type timeout:(NSTimeInterval)timeout diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index decd879a657ef3..c23eb463b16fc1 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -1222,12 +1222,15 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID queue:(dispatch_queue_t)queue completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion { - [_factory downloadLogFromNodeWithID:nodeID - controller:self - type:type - timeout:timeout - queue:queue - completion:completion]; + [self asyncDispatchToMatterQueue:^() { + [_factory downloadLogFromNodeWithID:nodeID + controller:self + type:type + timeout:timeout + queue:queue + completion:completion]; + } + errorHandler:nil]; } @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 319c33fdac999e..6f2c769fff2ad5 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1078,24 +1078,20 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID queue:(dispatch_queue_t)queue completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion { - dispatch_sync(_chipWorkQueue, ^{ - if (![self isRunning]) { - return; - } + assertChipStackLockedByCurrentThread(); - if (_diagnosticLogsDownloader == nil) { - _diagnosticLogsDownloader = [[MTRDiagnosticLogsDownloader alloc] init]; - auto systemState = _controllerFactory->GetSystemState(); - systemState->BDXTransferServer()->SetDelegate([_diagnosticLogsDownloader getBridge]); - } + if (_diagnosticLogsDownloader == nil) { + _diagnosticLogsDownloader = [[MTRDiagnosticLogsDownloader alloc] init]; + auto systemState = _controllerFactory->GetSystemState(); + systemState->BDXTransferServer()->SetDelegate([_diagnosticLogsDownloader getBridge]); + } - [_diagnosticLogsDownloader downloadLogFromNodeWithID:nodeID - controller:controller - type:type - timeout:timeout - queue:queue - completion:completion]; - }); + [_diagnosticLogsDownloader downloadLogFromNodeWithID:nodeID + controller:controller + type:type + timeout:timeout + queue:queue + completion:completion]; } - (void)operationalInstanceAdded:(chip::PeerId &)operationalID diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm index 4814b37d7a82f4..46226d69357058 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm @@ -59,7 +59,7 @@ - (instancetype)initWithType:(MTRDiagnosticLogType)type - (void)writeToFile:(NSData *)data error:(out NSError **)error; -- (BOOL)compare:(NSString *)fileDesignator +- (BOOL)matches:(NSString *)fileDesignator fabricIndex:(NSNumber *)fabricIndex nodeID:(NSNumber *)nodeID; @@ -139,8 +139,7 @@ - (void)handleBDXTransferSessionEndForFileDesignator:(NSString *)fileDesignator private: static void OnTransferTimeout(chip::System::Layer * layer, void * context); - MTRDiagnosticLogsDownloader * mDelegate; - AbortHandler mAbortHandler; + MTRDiagnosticLogsDownloader * __weak mDelegate; }; @implementation Download @@ -162,7 +161,7 @@ - (instancetype)initWithType:(MTRDiagnosticLogType)type Download * strongSelf = weakSelf; if (strongSelf) { // If a fileHandle exists, it means that the BDX session has been initiated and a file has - // been created to host the data of the session. So even if there is an error it may be some + // been created to host the data of the session. So even if there is an error there may be some // 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); @@ -192,15 +191,12 @@ - (void)checkInteractionModelResponse:(MTRDiagnosticLogsClusterRetrieveLogsRespo VerifyOrReturn(![status isEqual:@(MTRDiagnosticLogsStatusBusy)], [self failure:[MTRError errorForCHIPErrorCode:CHIP_ERROR_BUSY]]); VerifyOrReturn(![status isEqual:@(MTRDiagnosticLogsStatusDenied)], [self failure:[MTRError errorForCHIPErrorCode:CHIP_ERROR_ACCESS_DENIED]]); - // If there is not logs for the given type, forward it to the caller with a nil url and stop here. - VerifyOrReturn(![status isEqual:@(MTRDiagnosticLogsStatusNoLogs)], [self success]); - - // If the whole log content fits into the response LogContent field, forward it to the caller + // If the whole log content fits into the response LogContent field or if there is no log, forward it to the caller // and stop here. - if ([status isEqual:@(MTRDiagnosticLogsStatusExhausted)]) { + if ([status isEqual:@(MTRDiagnosticLogsStatusExhausted)] || [status isEqual:@(MTRDiagnosticLogsStatusNoLogs)]) { NSError * writeError = nil; [self writeToFile:response.logContent error:&writeError]; - VerifyOrReturn(nil == writeError, [self failure:[MTRError errorForCHIPErrorCode:CHIP_ERROR_INTERNAL]]); + VerifyOrReturn(nil == writeError, [self failure:writeError]); [self success]; return; @@ -238,7 +234,7 @@ - (void)deleteFile [[NSFileManager defaultManager] removeItemAtPath:[_fileURL path] error:&error]; if (nil != error) { // There is an error but there is really not much we can do at that point besides logging it. - MTR_LOG_ERROR("Error: %@", error); + MTR_LOG_ERROR("Error trying to delete the log file: %@. Error: %@", _fileURL, error); } } @@ -249,11 +245,11 @@ - (void)writeToFile:(NSData *)data error:(out NSError **)error [_fileHandle writeData:data error:error]; } -- (BOOL)compare:(NSString *)fileDesignator +- (BOOL)matches:(NSString *)fileDesignator fabricIndex:(NSNumber *)fabricIndex nodeID:(NSNumber *)nodeID { - return [_fileDesignator isEqualToString:fileDesignator] && _fabricIndex == fabricIndex && _nodeID == nodeID; + return [_fileDesignator isEqualToString:fileDesignator] && [_fabricIndex isEqualToNumber:fabricIndex] && [_nodeID isEqualToNumber:nodeID]; } - (void)failure:(NSError * _Nullable)error @@ -329,7 +325,7 @@ - (void)dealloc - (Download * _Nullable)get:(NSString *)fileDesignator fabricIndex:(NSNumber *)fabricIndex nodeID:(NSNumber *)nodeID { for (Download * download in _downloads) { - if ([download compare:fileDesignator fabricIndex:fabricIndex nodeID:nodeID]) { + if ([download matches:fileDesignator fabricIndex:fabricIndex nodeID:nodeID]) { return download; } } @@ -344,6 +340,8 @@ - (Download * _Nullable)add:(MTRDiagnosticLogType)type completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion done:(void (^)(Download * finishedDownload))done { + assertChipStackLockedByCurrentThread(); + auto download = [[Download alloc] initWithType:type fabricIndex:fabricIndex nodeID:nodeID queue:queue completion:completion done:done]; VerifyOrReturnValue(nil != download, nil); @@ -353,6 +351,8 @@ - (Download * _Nullable)add:(MTRDiagnosticLogType)type - (void)remove:(Download *)download { + assertChipStackLockedByCurrentThread(); + [_downloads removeObject:download]; } @end @@ -525,9 +525,7 @@ - (void)handleBDXTransferSessionEndForFileDesignator:(NSString *)fileDesignator } }; - // Ideally we would like to handle aborts a bit differently since this only works - // because our BDX stack supports one transfer at a time. - mAbortHandler = ^(NSError * error) { + auto abortHandler = ^(NSError * error) { assertChipStackLockedByCurrentThread(); auto err = [MTRError errorToCHIPErrorCode:error]; transfer->Reject(err); @@ -537,7 +535,7 @@ - (void)handleBDXTransferSessionEndForFileDesignator:(NSString *)fileDesignator fabricIndex:fabricIndex nodeID:nodeId completion:completionHandler - abortHandler:mAbortHandler]; + abortHandler:abortHandler]; return CHIP_NO_ERROR; } @@ -588,8 +586,6 @@ - (void)handleBDXTransferSessionEndForFileDesignator:(NSString *)fileDesignator } }; - mAbortHandler = nil; - [mDelegate handleBDXTransferSessionDataForFileDesignator:fileDesignator fabricIndex:fabricIndex nodeID:nodeId