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..27bbdd676d4971 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 + 10 : 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..b105b6cb3098c8 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,62 @@ auto logType = static_cast(mLogType); auto queue = dispatch_queue_create("com.chip.bdx.downloader", DISPATCH_QUEUE_SERIAL); + bool shouldWaitForDownload = !mIsAsyncCommand.ValueOr(false); + mIsAsyncCommand.ClearValue(); + + bool dumpToFile = mFilePath.HasValue(); + auto * dumpFilePath = dumpToFile ? [NSString stringWithUTF8String:mFilePath.Value()] : nil; + mFilePath.ClearValue(); + 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) { + if (shouldWaitForDownload) { + self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(readError)); + } + return; + } + + logContent = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; + NSLog(@"Content: %@", logContent); - auto * content = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; - NSLog(@"Content: %@", content); + if (dumpToFile) { + NSError * writeError = nil; + auto * fileManager = [NSFileManager defaultManager]; + [fileManager copyItemAtPath:[url path] toPath:dumpFilePath error:&writeError]; + if (nil != writeError) { + if (shouldWaitForDownload) { + self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(readError)); + } + return; + } + } } - VerifyOrReturn(nil == error, self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(error))); + ChipLogProgress(chipTool, "Diagnostic logs transfer: %s", error ? "Error" : "Success"); + auto err = RemoteDataModelLogger::LogBdxDownload(logContent, error); - // 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)); + if (CHIP_NO_ERROR != err) { + if (shouldWaitForDownload) { + self->SetCommandExitStatus(err); + } + return; + } + + if (shouldWaitForDownload) { + self->SetCommandExitStatus(MTRErrorToCHIPErrorCode(error)); } - self->SetCommandExitStatus(CHIP_NO_ERROR); }; [device downloadLogOfType:logType timeout:mTimeout queue:queue completion:completion]; + + if (!shouldWaitForDownload) { + 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/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/accessory_server_bridge.py b/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/accessory_server_bridge.py index d3e3403fef4954..a8991deb9b7870 100644 --- a/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/accessory_server_bridge.py +++ b/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/accessory_server_bridge.py @@ -17,6 +17,7 @@ import xmlrpc.client _DEFAULT_KEY = 'default' +_DEFAULT_WAIT_FOR_MESSAGE_TIMEOUT_SECONDS = 10 _IP = '127.0.0.1' _PORT = 9000 @@ -113,9 +114,11 @@ def factoryReset(request): def waitForMessage(request): register_key = _get_option(request, 'registerKey', _DEFAULT_KEY) message = _get_option(request, 'message') + timeout_in_seconds = _get_option( + request, 'timeoutInSeconds', _DEFAULT_WAIT_FOR_MESSAGE_TIMEOUT_SECONDS) with xmlrpc.client.ServerProxy(_make_url(), allow_none=True) as proxy: - proxy.waitForMessage(register_key, [message]) + proxy.waitForMessage(register_key, [message], timeout_in_seconds) def createOtaImage(request): otaImageFilePath = _get_option(request, 'otaImageFilePath') diff --git a/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/delay_commands.py b/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/delay_commands.py index fdb949da9b1449..7586be6de2239a 100644 --- a/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/delay_commands.py +++ b/scripts/py_matter_yamltests/matter_yamltests/pseudo_clusters/clusters/delay_commands.py @@ -40,6 +40,7 @@ + diff --git a/scripts/tests/chiptest/__init__.py b/scripts/tests/chiptest/__init__.py index dba11f15d44c1e..f70c22a610222c 100644 --- a/scripts/tests/chiptest/__init__.py +++ b/scripts/tests/chiptest/__init__.py @@ -160,6 +160,13 @@ def _GetInDevelopmentTests() -> Set[str]: } +def _GetChipToolUnsupportedTests() -> Set[str]: + """Tests that fail in chip-tool for some reason""" + return { + "TestDiagnosticLogsDownloadCommand", # chip-tool does not implement a bdx download command. + } + + def _GetDarwinFrameworkToolUnsupportedTests() -> Set[str]: """Tests that fail in darwin-framework-tool for some reason""" return { @@ -258,6 +265,7 @@ def _GetChipReplUnsupportedTests() -> Set[str]: "Test_TC_RVCCLEANM_3_3.yaml", # chip-repl does not support EqualityCommands pseudo-cluster "Test_TC_BINFO_2_1.yaml", # chip-repl does not support EqualityCommands pseudo-cluster "TestDiagnosticLogs.yaml", # chip-repl does not implement a BDXTransferServerDelegate + "TestDiagnosticLogsDownloadCommand.yaml", # chip-repl does not implement the bdx download command } @@ -338,7 +346,7 @@ def tests_with_command(chip_tool: str, is_manual: bool): ) -def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft_unsupported_as_in_development: bool, use_short_run_name: bool): +def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft_unsupported_as_in_development: bool, treat_chip_tool_unsupported_as_in_development: bool, use_short_run_name: bool): """ use_short_run_name should be true if we want the run_name to be "Test_ABC" instead of "some/path/Test_ABC.yaml" """ @@ -348,7 +356,8 @@ def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft extra_slow_tests = _GetExtraSlowTests() in_development_tests = _GetInDevelopmentTests() chip_repl_unsupported_tests = _GetChipReplUnsupportedTests() - treat_dft_unsupported_as_in_development_tests = _GetDarwinFrameworkToolUnsupportedTests() + dft_unsupported_as_in_development_tests = _GetDarwinFrameworkToolUnsupportedTests() + chip_tool_unsupported_as_in_development_tests = _GetChipToolUnsupportedTests() purposeful_failure_tests = _GetPurposefulFailureTests() for path in _AllYamlTests(): @@ -382,7 +391,10 @@ def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft else: run_name = str(path) - if treat_dft_unsupported_as_in_development and run_name in treat_dft_unsupported_as_in_development_tests: + if treat_dft_unsupported_as_in_development and run_name in dft_unsupported_as_in_development_tests: + tags.add(TestTag.IN_DEVELOPMENT) + + if treat_chip_tool_unsupported_as_in_development and run_name in chip_tool_unsupported_as_in_development_tests: tags.add(TestTag.IN_DEVELOPMENT) yield TestDefinition( @@ -394,17 +406,17 @@ def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft def AllReplYamlTests(): - for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=True, treat_dft_unsupported_as_in_development=False, use_short_run_name=False): + for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=True, treat_dft_unsupported_as_in_development=False, treat_chip_tool_unsupported_as_in_development=False, use_short_run_name=False): yield test def AllChipToolYamlTests(): - for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, treat_dft_unsupported_as_in_development=False, use_short_run_name=True): + for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, treat_dft_unsupported_as_in_development=False, treat_chip_tool_unsupported_as_in_development=True, use_short_run_name=True): yield test def AllDarwinFrameworkToolYamlTests(): - for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, treat_dft_unsupported_as_in_development=True, use_short_run_name=True): + for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, treat_dft_unsupported_as_in_development=True, treat_chip_tool_unsupported_as_in_development=False, use_short_run_name=True): yield test diff --git a/scripts/tests/chiptest/accessories.py b/scripts/tests/chiptest/accessories.py index 60e2136209ec19..99433d7ed88660 100644 --- a/scripts/tests/chiptest/accessories.py +++ b/scripts/tests/chiptest/accessories.py @@ -98,12 +98,12 @@ def factoryReset(self, name): return accessory.factoryReset() return False - def waitForMessage(self, name, message): + def waitForMessage(self, name, message, timeoutInSeconds=10): accessory = self.__accessories[name] if accessory: # The message param comes directly from the sys.argv[2:] of WaitForMessage.py and should contain a list of strings that # comprise the entire message to wait for - return accessory.waitForMessage(' '.join(message)) + return accessory.waitForMessage(' '.join(message), timeoutInSeconds) return False def createOtaImage(self, otaImageFilePath, rawImageFilePath, rawImageContent, vid='0xDEAD', pid='0xBEEF'): diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index 19f0a7dcb74b0b..df5e8385097e95 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/scripts/tests/chiptest/test_definition.py @@ -83,8 +83,8 @@ def factoryReset(self): def waitForAnyAdvertisement(self): self.__waitFor("mDNS service published:", self.process, self.outpipe) - def waitForMessage(self, message): - self.__waitFor(message, self.process, self.outpipe) + def waitForMessage(self, message, timeoutInSeconds=10): + self.__waitFor(message, self.process, self.outpipe, timeoutInSeconds) return True def kill(self): @@ -124,7 +124,7 @@ def __startServer(self, runner, command): self.kvsPathSet.add(value) return runner.RunSubprocess(app_cmd, name='APP ', wait=False) - def __waitFor(self, waitForString, server_process, outpipe): + def __waitFor(self, waitForString, server_process, outpipe, timeoutInSeconds=10): logging.debug('Waiting for %s' % waitForString) start_time = time.monotonic() @@ -139,7 +139,7 @@ def __waitFor(self, waitForString, server_process, outpipe): (waitForString, server_process.returncode)) logging.error(died_str) raise Exception(died_str) - if time.monotonic() - start_time > 10: + if time.monotonic() - start_time > timeoutInSeconds: raise Exception('Timeout while waiting for %s' % waitForString) time.sleep(0.1) ready, self.lastLogIndex = outpipe.CapturedLogContains( @@ -332,6 +332,13 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str, # so it will be commissionable again. app.factoryReset() + # It may sometimes be useful to run the same app multiple times depending + # on the implementation. So this code creates a duplicate entry but with a different + # key. + app = App(runner, path) + apps_register.add(f'{key}#2', app) + app.factoryReset() + if dry_run: tool_storage_dir = None tool_storage_args = [] diff --git a/src/app/tests/suites/TestDiagnosticLogs.yaml b/src/app/tests/suites/TestDiagnosticLogs.yaml index cf31a96b8ddf58..57ab8cb0fa04f8 100644 --- a/src/app/tests/suites/TestDiagnosticLogs.yaml +++ b/src/app/tests/suites/TestDiagnosticLogs.yaml @@ -18,6 +18,7 @@ config: nodeId: 0x12344321 cluster: "Diagnostic Logs" endpoint: 0 + timeout: 120 end_user_support_log_file_path: "/tmp/end_user_support_log.txt" end_user_support_log_file_content: "End User Support Log Content" end_user_support_log_file_content_long: @@ -31,6 +32,113 @@ config: bdx_transfer_file_path: "/tmp/end_user_support_bdx_output.txt" bdx_transfer_file_name: "end_user_support_bdx_output.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: + "This is a long log content... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ....................................................... + ......................................................." + tests: # # Set up the test by adding some destination log files for the target accessory: @@ -658,3 +766,180 @@ tests: values: - name: "filePath" value: bdx_transfer_file_path + + # + # 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: "RetrieveLogsRequest" + identity: "alpha" + arguments: + values: + - name: "Intent" + value: 1 # NetworkDiagnostics + - name: "RequestedProtocol" + value: 1 # BDX + - name: "TransferFileDesignator" + value: bdx_transfer_file_name_1 + response: + values: + - name: "Status" + value: 0 # Success + - name: "LogContent" + value: "" + + - label: "BDX: Request End User Support from the second accessory" + command: "RetrieveLogsRequest" + identity: "beta" + arguments: + values: + - name: "Intent" + value: 1 # NetworkDiagnostics + - name: "RequestedProtocol" + value: 1 # BDX + - name: "TransferFileDesignator" + value: bdx_transfer_file_name_2 + response: + values: + - name: "Status" + value: 0 # Success + - name: "LogContent" + value: "" + + - 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" + + - 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" + + - 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/app/tests/suites/TestDiagnosticLogsDownloadCommand.yaml b/src/app/tests/suites/TestDiagnosticLogsDownloadCommand.yaml new file mode 100644 index 00000000000000..a67acdd837b54f --- /dev/null +++ b/src/app/tests/suites/TestDiagnosticLogsDownloadCommand.yaml @@ -0,0 +1,598 @@ +# 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 server: + # 1. End User Support + # 2. Network Diagnostics + # 3. Crash + # + # The first thing to do is to delete them if they exist. There 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 server" + 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 server 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: "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 we handle Busy properly + # + - label: + "BDX: Wait for 'Diagnostic logs transfer: Success' message from the + previous steps" + cluster: "DelayCommands" + command: "WaitForMessage" + arguments: + values: + - name: "registerKey" + value: "default" + - name: "message" + value: "Diagnostic logs transfer: Success" + + - label: "Delete possible leftover from previous run" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: bdx_transfer_file_path_1 + + - 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: "BDX: Request End User Support from the server" + command: "Download" + arguments: + values: + - name: "LogType" + value: 0 # EndUserSupport + - name: "Timeout" + value: 0 + - name: "async" + value: true + - name: "filepath" + value: bdx_transfer_file_path_1 + + - label: + "BDX: Request End User Support from the server again while it is busy" + command: "Download" + arguments: + values: + - name: "LogType" + value: 0 # EndUserSupport + - name: "Timeout" + value: 0 + - name: "filepath" + value: bdx_transfer_file_path_2 + response: + values: + - name: "logContent" + value: null + - name: "error" + value: "FAILURE" + + - label: "BDX: Wait for 'Diagnostic logs transfer: Success' message" + cluster: "DelayCommands" + command: "WaitForMessage" + arguments: + values: + - name: "registerKey" + value: "default" + - name: "message" + value: "Diagnostic logs transfer: Success" + - name: "timeoutInSeconds" + value: 60 + + - label: "Delete the result of the previous run" + cluster: "SystemCommands" + command: "DeleteFile" + arguments: + values: + - name: "filePath" + value: bdx_transfer_file_path_1 + + # + # Validate that multiple BDX transfers can run in parallel. + # + - 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 server" + 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 server 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 server" + 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 server" + 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 server" + 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 server" + 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/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index 8268b07b7f3dcc..ebc728ca206875 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -226,7 +226,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params) ReturnErrorOnFailure(stateParams.exchangeMgr->Init(stateParams.sessionMgr)); ReturnErrorOnFailure(stateParams.messageCounterManager->Init(stateParams.exchangeMgr)); ReturnErrorOnFailure(stateParams.unsolicitedStatusHandler->Init(stateParams.exchangeMgr)); - ReturnErrorOnFailure(stateParams.bdxTransferServer->ListenForSendInit(stateParams.systemLayer, stateParams.exchangeMgr)); + ReturnErrorOnFailure(stateParams.bdxTransferServer->Init(stateParams.systemLayer, stateParams.exchangeMgr)); InitDataModelHandler(); diff --git a/src/darwin/Framework/CHIP/MTRDevice.h b/src/darwin/Framework/CHIP/MTRDevice.h index 9d3b0768ff4061..96526a2ecacf67 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 have 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 9621b447fb49b1..78c27f3ae46b20 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -1300,12 +1300,17 @@ - (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:^() { + [self->_factory downloadLogFromNodeWithID:nodeID + controller:self + type:type + timeout:timeout + queue:queue + completion:completion]; + } + errorHandler:^(NSError * error) { + completion(nil, error); + }]; } - (NSArray *)accessGrantsForClusterPath:(MTRClusterPath *)clusterPath diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 457abeaed55a5f..0430558e8a4b32 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1220,24 +1220,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 diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 4fa06b2548b513..a0f7378016dc01 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -1731,6 +1731,17 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; #define CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER 0 #endif +/** + * @def CHIP_CONFIG_MAX_BDX_LOG_TRANSFERS + * + * @brief + * Maximum number of simultaneously active bdx log transfers. + * + */ +#ifndef CHIP_CONFIG_MAX_BDX_LOG_TRANSFERS +#define CHIP_CONFIG_MAX_BDX_LOG_TRANSFERS 5 +#endif // CHIP_CONFIG_MAX_BDX_LOG_TRANSFERS + /** * @} */ diff --git a/src/protocols/bdx/BUILD.gn b/src/protocols/bdx/BUILD.gn index 17f18826ebe474..a95fd02202f626 100644 --- a/src/protocols/bdx/BUILD.gn +++ b/src/protocols/bdx/BUILD.gn @@ -20,6 +20,9 @@ static_library("bdx") { sources = [ "BdxMessages.cpp", "BdxMessages.h", + "BdxTransferDiagnosticLog.cpp", + "BdxTransferDiagnosticLog.h", + "BdxTransferDiagnosticLogPool.h", "BdxTransferProxy.h", "BdxTransferProxyDiagnosticLog.cpp", "BdxTransferProxyDiagnosticLog.h", diff --git a/src/protocols/bdx/BdxTransferDiagnosticLog.cpp b/src/protocols/bdx/BdxTransferDiagnosticLog.cpp new file mode 100644 index 00000000000000..8b349fed31e8f5 --- /dev/null +++ b/src/protocols/bdx/BdxTransferDiagnosticLog.cpp @@ -0,0 +1,198 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * 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. + */ + +#include "BdxTransferDiagnosticLog.h" + +namespace chip { +namespace bdx { + +namespace { +// Max block size for the BDX transfer. +constexpr uint32_t kMaxBdxBlockSize = 1024; + +// Timeout for the BDX transfer session.. +constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); +constexpr TransferRole kBdxRole = TransferRole::kReceiver; +} // namespace + +void BdxTransferDiagnosticLog::HandleTransferSessionOutput(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + + ChipLogDetail(BDX, "Got an event %s", event.ToString(event.EventType)); + + switch (event.EventType) + { + case TransferSession::OutputEventType::kInitReceived: + AbortTransferOnFailure(OnTransferSessionBegin(event)); + break; + case TransferSession::OutputEventType::kStatusReceived: + ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); + LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_INTERNAL)); + break; + case TransferSession::OutputEventType::kInternalError: + LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_INTERNAL)); + break; + case TransferSession::OutputEventType::kTransferTimeout: + LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_TIMEOUT)); + break; + case TransferSession::OutputEventType::kBlockReceived: + AbortTransferOnFailure(OnBlockReceived(event)); + break; + case TransferSession::OutputEventType::kMsgToSend: + LogErrorOnFailure(OnMessageToSend(event)); + + if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) + { + LogErrorOnFailure(OnTransferSessionEnd(CHIP_NO_ERROR)); + } + break; + case TransferSession::OutputEventType::kAckEOFReceived: + case TransferSession::OutputEventType::kNone: + case TransferSession::OutputEventType::kQueryWithSkipReceived: + case TransferSession::OutputEventType::kQueryReceived: + case TransferSession::OutputEventType::kAckReceived: + case TransferSession::OutputEventType::kAcceptReceived: + // Nothing to do. + break; + default: + // Should never happen. + chipDie(); + break; + } +} + +CHIP_ERROR BdxTransferDiagnosticLog::OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader, + System::PacketBufferHandle && payload) +{ + assertChipStackLockedByCurrentThread(); + + VerifyOrReturnError(ec != nullptr, CHIP_ERROR_INCORRECT_STATE); + + // If we receive a SendInit message, then we prepare for transfer + if (payloadHeader.HasMessageType(MessageType::SendInit)) + { + FabricIndex fabricIndex = ec->GetSessionHandle()->GetFabricIndex(); + NodeId peerNodeId = ec->GetSessionHandle()->GetPeer().GetNodeId(); + VerifyOrReturnError(fabricIndex != kUndefinedFabricIndex, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(peerNodeId != kUndefinedNodeId, CHIP_ERROR_INVALID_ARGUMENT); + + mTransferProxy.SetFabricIndex(fabricIndex); + mTransferProxy.SetPeerNodeId(peerNodeId); + auto flags(TransferControlFlags::kSenderDrive); + ReturnLogErrorOnFailure(Responder::PrepareForTransfer(mSystemLayer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout)); + } + + return TransferFacilitator::OnMessageReceived(ec, payloadHeader, std::move(payload)); +} + +CHIP_ERROR BdxTransferDiagnosticLog::OnMessageToSend(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + + VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); + + auto & msgTypeData = event.msgTypeData; + bool isStatusReport = msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport); + + // All messages sent from the Sender expect a response, except for a StatusReport which would indicate an error and + // the end of the transfer. + Messaging::SendFlags sendFlags; + VerifyOrDo(isStatusReport, sendFlags.Set(Messaging::SendMessageFlags::kExpectResponse)); + + // If there's an error sending the message, close the exchange by calling Reset. + auto err = mExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags); + VerifyOrDo(CHIP_NO_ERROR == err, OnTransferSessionEnd(err)); + + return err; +} + +CHIP_ERROR BdxTransferDiagnosticLog::OnTransferSessionBegin(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + VerifyOrReturnError(nullptr != mDelegate, CHIP_ERROR_INCORRECT_STATE); + + ReturnErrorOnFailure(mTransferProxy.Init(&mTransfer)); + return mDelegate->OnTransferBegin(&mTransferProxy); +} + +CHIP_ERROR BdxTransferDiagnosticLog::OnTransferSessionEnd(CHIP_ERROR error) +{ + assertChipStackLockedByCurrentThread(); + VerifyOrReturnError(nullptr != mDelegate, CHIP_ERROR_INCORRECT_STATE); + + LogErrorOnFailure(mDelegate->OnTransferEnd(&mTransferProxy, error)); + Reset(); + return CHIP_NO_ERROR; +} + +CHIP_ERROR BdxTransferDiagnosticLog::OnBlockReceived(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + VerifyOrReturnError(nullptr != mDelegate, CHIP_ERROR_INCORRECT_STATE); + + ByteSpan blockData(event.blockdata.Data, event.blockdata.Length); + return mDelegate->OnTransferData(&mTransferProxy, blockData); +} + +void BdxTransferDiagnosticLog::AbortTransferOnFailure(CHIP_ERROR error) +{ + VerifyOrReturn(CHIP_NO_ERROR != error); + LogErrorOnFailure(error); + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(error))); +} + +void BdxTransferDiagnosticLog::Reset() +{ + assertChipStackLockedByCurrentThread(); + + Responder::ResetTransfer(); + + if (mExchangeCtx) + { + mIsExchangeClosing = true; + mExchangeCtx->Close(); + mIsExchangeClosing = false; + mExchangeCtx = nullptr; + } + + mTransferProxy.Reset(); +} + +void BdxTransferDiagnosticLog::OnExchangeClosing(Messaging::ExchangeContext * ec) +{ + // The exchange can be closing while TransferFacilitator is still accessing us, so + // the BdxTransferDiagnosticLog can not be released "right now". + mSystemLayer->ScheduleWork( + [](auto * systemLayer, auto * appState) -> void { + auto * _this = static_cast(appState); + _this->mPoolDelegate->Release(_this); + }, + this); + + // This block checks and handles the scenario where the exchange is closed externally (e.g., receiving a StatusReport). + // Continuing to use it could lead to a use-after-free error and such an error might occur when the poll timer triggers and + // OnTransferSessionEnd is called. + // We know it's not just us normally closing the exchange if mIsExchangeClosing is false. + VerifyOrReturn(!mIsExchangeClosing); + mExchangeCtx = nullptr; + LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_INTERNAL)); +} + +} // namespace bdx +} // namespace chip diff --git a/src/protocols/bdx/BdxTransferDiagnosticLog.h b/src/protocols/bdx/BdxTransferDiagnosticLog.h new file mode 100644 index 00000000000000..bc7a8773cd2dff --- /dev/null +++ b/src/protocols/bdx/BdxTransferDiagnosticLog.h @@ -0,0 +1,111 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * 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 + +#include +#include +#include +#include + +namespace chip { +namespace bdx { + +class BdxTransferDiagnosticLog : public Responder +{ +public: + BdxTransferDiagnosticLog(BDXTransferServerDelegate * delegate, BdxTransferDiagnosticLogPoolDelegate * poolDelegate, + System::Layer * systemLayer) : + mSystemLayer(systemLayer), + mDelegate(delegate), mPoolDelegate(poolDelegate){}; + + ~BdxTransferDiagnosticLog() { Reset(); }; + + /** + * This method handles BDX messages and other TransferSession events. + * + * @param[in] event An OutputEvent that contains output from the TransferSession object. + */ + void HandleTransferSessionOutput(TransferSession::OutputEvent & event) override; + + void OnExchangeClosing(Messaging::ExchangeContext * ec) override; + +protected: + /** + * Called when a BDX message is received over the exchange context + * + * @param[in] ec The exchange context + * + * @param[in] payloadHeader The payload header of the message + * + * @param[in] payload The payload of the message + */ + CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader, + System::PacketBufferHandle && payload) override; + +private: + /** + * Called to send a BDX MsgToSend message over the exchange + * + * + * @param[in] event The output event to be send + */ + CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event); + + /** + * Called to begin the transfer session when an init message has been received + * + * @param[in] event The output event received + */ + CHIP_ERROR OnTransferSessionBegin(TransferSession::OutputEvent & event); + + /** + * Called to end the transfer session when a BlockAckEOF message has been sent over the exchange + * or an error has occurred during the BDX session + * + * @param[in] error The error type + */ + CHIP_ERROR OnTransferSessionEnd(CHIP_ERROR error); + + /** + * Called when a block has been received from the Sender. The block is processed + * and written to a file and a block ack is sent back to the sender. + * + * @param[in] event The output event received + */ + CHIP_ERROR OnBlockReceived(TransferSession::OutputEvent & event); + + /** + * This method is called to reset state. It resets the transfer and cleans up the + * exchange and the fabric index and peer node id. + */ + void Reset(); + + void AbortTransferOnFailure(CHIP_ERROR error); + + BDXTransferProxyDiagnosticLog mTransferProxy; + bool mIsExchangeClosing = false; + + System::Layer * mSystemLayer; + + BDXTransferServerDelegate * mDelegate; + BdxTransferDiagnosticLogPoolDelegate * mPoolDelegate; +}; + +} // namespace bdx +} // namespace chip diff --git a/src/protocols/bdx/BdxTransferDiagnosticLogPool.h b/src/protocols/bdx/BdxTransferDiagnosticLogPool.h new file mode 100644 index 00000000000000..a2d7d3f5d11105 --- /dev/null +++ b/src/protocols/bdx/BdxTransferDiagnosticLogPool.h @@ -0,0 +1,58 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * 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 + +#include +#include +#include + +namespace chip { +namespace bdx { + +class BdxTransferDiagnosticLog; + +class BdxTransferDiagnosticLogPoolDelegate +{ +public: + virtual ~BdxTransferDiagnosticLogPoolDelegate() {} + + virtual BdxTransferDiagnosticLog * Allocate(BDXTransferServerDelegate * delegate, System::Layer * systemLayer) = 0; + + virtual void Release(BdxTransferDiagnosticLog * client) = 0; +}; + +template +class BdxTransferDiagnosticLogPool : public BdxTransferDiagnosticLogPoolDelegate +{ +public: + ~BdxTransferDiagnosticLogPool() override { mTransferPool.ReleaseAll(); } + + BdxTransferDiagnosticLog * Allocate(BDXTransferServerDelegate * delegate, System::Layer * systemLayer) override + { + return mTransferPool.CreateObject(delegate, this, systemLayer); + } + + void Release(BdxTransferDiagnosticLog * transfer) override { mTransferPool.ReleaseObject(transfer); } + +private: + ObjectPool mTransferPool; +}; + +} // namespace bdx +} // namespace chip diff --git a/src/protocols/bdx/BdxTransferServer.cpp b/src/protocols/bdx/BdxTransferServer.cpp index b276dba51ab521..709d21b1f3b835 100644 --- a/src/protocols/bdx/BdxTransferServer.cpp +++ b/src/protocols/bdx/BdxTransferServer.cpp @@ -21,16 +21,7 @@ namespace chip { namespace bdx { -namespace { -// Max block size for the BDX transfer. -constexpr uint32_t kMaxBdxBlockSize = 1024; - -// Timeout for the BDX transfer session.. -constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); -constexpr TransferRole kBdxRole = TransferRole::kReceiver; -} // namespace - -CHIP_ERROR BDXTransferServer::ListenForSendInit(System::Layer * systemLayer, Messaging::ExchangeManager * exchangeMgr) +CHIP_ERROR BDXTransferServer::Init(System::Layer * systemLayer, Messaging::ExchangeManager * exchangeMgr) { VerifyOrReturnError(nullptr != systemLayer, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(nullptr != exchangeMgr, CHIP_ERROR_INVALID_ARGUMENT); @@ -40,170 +31,31 @@ CHIP_ERROR BDXTransferServer::ListenForSendInit(System::Layer * systemLayer, Mes return mExchangeMgr->RegisterUnsolicitedMessageHandlerForType(MessageType::SendInit, this); } -void BDXTransferServer::Shutdown() -{ - VerifyOrReturn(nullptr != mSystemLayer); - VerifyOrReturn(nullptr != mExchangeMgr); - - LogErrorOnFailure(mExchangeMgr->UnregisterUnsolicitedMessageHandlerForType(MessageType::SendInit)); - - mSystemLayer = nullptr; - mExchangeMgr = nullptr; -} - -void BDXTransferServer::HandleTransferSessionOutput(TransferSession::OutputEvent & event) -{ - assertChipStackLockedByCurrentThread(); - - ChipLogDetail(BDX, "Got an event %s", event.ToString(event.EventType)); - - switch (event.EventType) - { - case TransferSession::OutputEventType::kInitReceived: - AbortTransferOnFailure(OnTransferSessionBegin(event)); - break; - case TransferSession::OutputEventType::kStatusReceived: - ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); - LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_INTERNAL)); - break; - case TransferSession::OutputEventType::kInternalError: - LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_INTERNAL)); - break; - case TransferSession::OutputEventType::kTransferTimeout: - LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_TIMEOUT)); - break; - case TransferSession::OutputEventType::kBlockReceived: - AbortTransferOnFailure(OnBlockReceived(event)); - break; - case TransferSession::OutputEventType::kMsgToSend: - LogErrorOnFailure(OnMessageToSend(event)); - - if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) - { - LogErrorOnFailure(OnTransferSessionEnd(CHIP_NO_ERROR)); - } - break; - case TransferSession::OutputEventType::kAckEOFReceived: - case TransferSession::OutputEventType::kNone: - case TransferSession::OutputEventType::kQueryWithSkipReceived: - case TransferSession::OutputEventType::kQueryReceived: - case TransferSession::OutputEventType::kAckReceived: - case TransferSession::OutputEventType::kAcceptReceived: - // Nothing to do. - break; - default: - // Should never happen. - chipDie(); - break; - } -} - -CHIP_ERROR BDXTransferServer::OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader, - System::PacketBufferHandle && payload) -{ - assertChipStackLockedByCurrentThread(); - - VerifyOrReturnError(ec != nullptr, CHIP_ERROR_INCORRECT_STATE); - - // If we receive a SendInit message, then we prepare for transfer - if (payloadHeader.HasMessageType(MessageType::SendInit)) - { - FabricIndex fabricIndex = ec->GetSessionHandle()->GetFabricIndex(); - NodeId peerNodeId = ec->GetSessionHandle()->GetPeer().GetNodeId(); - VerifyOrReturnError(fabricIndex != kUndefinedFabricIndex, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(peerNodeId != kUndefinedNodeId, CHIP_ERROR_INVALID_ARGUMENT); - - mTransferProxy.SetFabricIndex(fabricIndex); - mTransferProxy.SetPeerNodeId(peerNodeId); - auto flags(TransferControlFlags::kSenderDrive); - ReturnLogErrorOnFailure(Responder::PrepareForTransfer(mSystemLayer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout)); - } - - return TransferFacilitator::OnMessageReceived(ec, payloadHeader, std::move(payload)); -} - -CHIP_ERROR BDXTransferServer::OnMessageToSend(TransferSession::OutputEvent & event) +CHIP_ERROR BDXTransferServer::OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, + Messaging::ExchangeDelegate *& newDelegate) { - assertChipStackLockedByCurrentThread(); + auto * logTransfer = mPoolDelegate.Allocate(mDelegate, mSystemLayer); + VerifyOrReturnError(nullptr != logTransfer, CHIP_ERROR_NO_MEMORY); - VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); - - auto & msgTypeData = event.msgTypeData; - bool isStatusReport = msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport); - - // All messages sent from the Sender expect a response, except for a StatusReport which would indicate an error and - // the end of the transfer. - Messaging::SendFlags sendFlags; - VerifyOrDo(isStatusReport, sendFlags.Set(Messaging::SendMessageFlags::kExpectResponse)); - - // If there's an error sending the message, close the exchange by calling Reset. - auto err = mExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags); - VerifyOrDo(CHIP_NO_ERROR == err, Reset()); - - return err; -} - -CHIP_ERROR BDXTransferServer::OnTransferSessionBegin(TransferSession::OutputEvent & event) -{ - assertChipStackLockedByCurrentThread(); - VerifyOrReturnError(nullptr != mDelegate, CHIP_ERROR_INCORRECT_STATE); - - ReturnErrorOnFailure(mTransferProxy.Init(&mTransfer)); - return mDelegate->OnTransferBegin(&mTransferProxy); -} - -CHIP_ERROR BDXTransferServer::OnTransferSessionEnd(CHIP_ERROR error) -{ - assertChipStackLockedByCurrentThread(); - VerifyOrReturnError(nullptr != mDelegate, CHIP_ERROR_INCORRECT_STATE); - - LogErrorOnFailure(mDelegate->OnTransferEnd(&mTransferProxy, error)); - Reset(); + newDelegate = logTransfer; return CHIP_NO_ERROR; } -CHIP_ERROR BDXTransferServer::OnBlockReceived(TransferSession::OutputEvent & event) -{ - assertChipStackLockedByCurrentThread(); - VerifyOrReturnError(nullptr != mDelegate, CHIP_ERROR_INCORRECT_STATE); - - ByteSpan blockData(event.blockdata.Data, event.blockdata.Length); - return mDelegate->OnTransferData(&mTransferProxy, blockData); -} - -void BDXTransferServer::AbortTransferOnFailure(CHIP_ERROR error) +void BDXTransferServer::OnExchangeCreationFailed(Messaging::ExchangeDelegate * delegate) { - VerifyOrReturn(CHIP_NO_ERROR != error); - LogErrorOnFailure(error); - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(error))); + auto * logTransfer = static_cast(delegate); + mPoolDelegate.Release(logTransfer); } -void BDXTransferServer::Reset() +void BDXTransferServer::Shutdown() { - assertChipStackLockedByCurrentThread(); - - Responder::ResetTransfer(); - - if (mExchangeCtx) - { - mIsExchangeClosing = true; - mExchangeCtx->Close(); - mIsExchangeClosing = false; - mExchangeCtx = nullptr; - } + VerifyOrReturn(nullptr != mSystemLayer); + VerifyOrReturn(nullptr != mExchangeMgr); - mTransferProxy.Reset(); -} + LogErrorOnFailure(mExchangeMgr->UnregisterUnsolicitedMessageHandlerForType(MessageType::SendInit)); -void BDXTransferServer::OnExchangeClosing(Messaging::ExchangeContext * ec) -{ - // This block checks and handles the scenario where the exchange is closed externally (e.g., receiving a StatusReport). - // Continuing to use it could lead to a use-after-free error and such an error might occur when the poll timer triggers and - // OnTransferSessionEnd is called. - // We know it's not just us normally closing the exchange if mIsExchangeClosing is false. - VerifyOrReturn(!mIsExchangeClosing); - mExchangeCtx = nullptr; - LogErrorOnFailure(OnTransferSessionEnd(CHIP_ERROR_INTERNAL)); + mSystemLayer = nullptr; + mExchangeMgr = nullptr; } } // namespace bdx diff --git a/src/protocols/bdx/BdxTransferServer.h b/src/protocols/bdx/BdxTransferServer.h index 83da8b97ecb6a8..0fac0e88b672de 100644 --- a/src/protocols/bdx/BdxTransferServer.h +++ b/src/protocols/bdx/BdxTransferServer.h @@ -18,97 +18,41 @@ #pragma once -#include "BdxTransferServerDelegate.h" +#include +#include #include -#include -#include -#include +#include namespace chip { namespace bdx { -class BDXTransferServer : public Responder +class BdxTransferDiagnosticLog; + +class BDXTransferServer : public Messaging::UnsolicitedMessageHandler { public: BDXTransferServer(){}; ~BDXTransferServer() { Shutdown(); }; - CHIP_ERROR ListenForSendInit(System::Layer * systemLayer, Messaging::ExchangeManager * exchangeMgr); + CHIP_ERROR Init(System::Layer * systemLayer, Messaging::ExchangeManager * exchangeMgr); void Shutdown(); void SetDelegate(BDXTransferServerDelegate * delegate) { mDelegate = delegate; } - /** - * This method handles BDX messages and other TransferSession events. - * - * @param[in] event An OutputEvent that contains output from the TransferSession object. - */ - void HandleTransferSessionOutput(TransferSession::OutputEvent & event) override; - - void OnExchangeClosing(Messaging::ExchangeContext * ec) override; - protected: - /** - * Called when a BDX message is received over the exchange context - * - * @param[in] ec The exchange context - * - * @param[in] payloadHeader The payload header of the message - * - * @param[in] payload The payload of the message - */ - CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader, - System::PacketBufferHandle && payload) override; + CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, + Messaging::ExchangeDelegate *& newDelegate) override; + void OnExchangeCreationFailed(Messaging::ExchangeDelegate * delegate) override; private: - /** - * Called to send a BDX MsgToSend message over the exchange - * - * - * @param[in] event The output event to be send - */ - CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event); - - /** - * Called to begin the transfer session when an init message has been received - * - * @param[in] event The output event received - */ - CHIP_ERROR OnTransferSessionBegin(TransferSession::OutputEvent & event); - - /** - * Called to end the transfer session when a BlockAckEOF message has been sent over the exchange - * or an error has occurred during the BDX session - * - * @param[in] error The error type - */ - CHIP_ERROR OnTransferSessionEnd(CHIP_ERROR error); - - /** - * Called when a block has been received from the Sender. The block is processed - * and written to a file and a block ack is sent back to the sender. - * - * @param[in] event The output event received - */ - CHIP_ERROR OnBlockReceived(TransferSession::OutputEvent & event); - - /** - * This method is called to reset state. It resets the transfer and cleans up the - * exchange and the fabric index and peer node id. - */ - void Reset(); - - void AbortTransferOnFailure(CHIP_ERROR error); - System::Layer * mSystemLayer; Messaging::ExchangeManager * mExchangeMgr; - BDXTransferServerDelegate * mDelegate; - BDXTransferProxyDiagnosticLog mTransferProxy; - bool mIsExchangeClosing = false; + BDXTransferServerDelegate * mDelegate; + BdxTransferDiagnosticLogPool mPoolDelegate; }; } // namespace bdx