From aa2ade5d34ea912552f7af12fb760990e6660167 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Tue, 24 Oct 2023 19:17:31 -0700 Subject: [PATCH 01/16] Add support for diagnostic logs server cluster and add the log provider delegate to all-clusters-app - Add darwin code to download logs of differnt types and get the logs transferred via BDX --- .../all-clusters-app.matter | 58 +++ .../all-clusters-common/all-clusters-app.zap | 150 +++++++- .../diagnostic-logs-provider-delegate-impl.h | 78 ++++ ...diagnostic-logs-provider-delegate-impl.cpp | 141 ++++++++ .../all-clusters-app/linux/AppOptions.cpp | 32 +- examples/all-clusters-app/linux/BUILD.gn | 1 + .../linux/include/CHIPProjectAppConfig.h | 2 + .../all-clusters-app/linux/main-common.cpp | 9 + .../bdx/DiagnosticLogsBDXTransferHandler.cpp | 305 ++++++++++++++++ .../bdx/DiagnosticLogsBDXTransferHandler.h | 77 ++++ src/app/chip_data_model.gni | 8 + .../diagnostic-logs-provider-delegate.h | 86 +++++ .../diagnostic-logs-server.cpp | 337 +++++++++++++----- .../diagnostic-logs-server.h | 79 +++- src/darwin/Framework/CHIP/MTRDevice.h | 30 ++ src/darwin/Framework/CHIP/MTRDevice.mm | 237 ++++++++++++ .../CHIP/MTRDiagnosticLogsTransferHandler.h | 93 +++++ .../CHIP/MTRDiagnosticLogsTransferHandler.mm | 296 +++++++++++++++ .../zap-generated/MTRCommandPayloadsObjc.h | 2 +- .../Framework/CHIPTests/MTRDeviceTests.m | 29 +- .../Matter.xcodeproj/project.pbxproj | 8 + src/lib/core/CHIPConfig.h | 12 +- src/protocols/bdx/BdxTransferSession.cpp | 44 ++- src/protocols/bdx/TransferFacilitator.cpp | 7 + src/protocols/bdx/TransferFacilitator.h | 8 +- 25 files changed, 2009 insertions(+), 120 deletions(-) create mode 100644 examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h create mode 100644 examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp create mode 100644 src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp create mode 100644 src/app/bdx/DiagnosticLogsBDXTransferHandler.h create mode 100644 src/app/clusters/diagnostic-logs-server/diagnostic-logs-provider-delegate.h create mode 100644 src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h create mode 100644 src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter index 367388c7b2f015..4491adf35b3d88 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -1522,6 +1522,51 @@ server cluster NetworkCommissioning = 49 { command access(invoke: administer) ReorderNetwork(ReorderNetworkRequest): NetworkConfigResponse = 8; } +/** The cluster provides commands for retrieving unstructured diagnostic logs from a Node that may be used to aid in diagnostics. */ +client cluster DiagnosticLogs = 50 { + enum IntentEnum : ENUM8 { + kEndUserSupport = 0; + kNetworkDiag = 1; + kCrashLogs = 2; + } + + enum StatusEnum : ENUM8 { + kSuccess = 0; + kExhausted = 1; + kNoLogs = 2; + kBusy = 3; + kDenied = 4; + } + + enum TransferProtocolEnum : ENUM8 { + kResponsePayload = 0; + kBDX = 1; + } + + readonly attribute command_id generatedCommandList[] = 65528; + readonly attribute command_id acceptedCommandList[] = 65529; + readonly attribute event_id eventList[] = 65530; + readonly attribute attrib_id attributeList[] = 65531; + readonly attribute bitmap32 featureMap = 65532; + readonly attribute int16u clusterRevision = 65533; + + request struct RetrieveLogsRequestRequest { + IntentEnum intent = 0; + TransferProtocolEnum requestedProtocol = 1; + optional char_string<32> transferFileDesignator = 2; + } + + response struct RetrieveLogsResponse = 1 { + StatusEnum status = 0; + LONG_OCTET_STRING logContent = 1; + optional epoch_us UTCTimeStamp = 2; + optional systime_us timeSinceBoot = 3; + } + + /** Retrieving diagnostic logs from a Node */ + command RetrieveLogsRequest(RetrieveLogsRequestRequest): RetrieveLogsResponse = 0; +} + /** The cluster provides commands for retrieving unstructured diagnostic logs from a Node that may be used to aid in diagnostics. */ server cluster DiagnosticLogs = 50 { enum IntentEnum : ENUM8 { @@ -1556,6 +1601,13 @@ server cluster DiagnosticLogs = 50 { optional char_string<32> transferFileDesignator = 2; } + response struct RetrieveLogsResponse = 1 { + StatusEnum status = 0; + LONG_OCTET_STRING logContent = 1; + optional epoch_us UTCTimeStamp = 2; + optional systime_us timeSinceBoot = 3; + } + command RetrieveLogsRequest(RetrieveLogsRequestRequest): RetrieveLogsResponse = 0; } @@ -5671,6 +5723,7 @@ endpoint 0 { device type ma_powersource = 17, version 1; binding cluster OtaSoftwareUpdateProvider; + binding cluster DiagnosticLogs; server cluster Identify { ram attribute identifyTime default = 0x0000; @@ -5885,10 +5938,15 @@ endpoint 0 { } server cluster DiagnosticLogs { + callback attribute generatedCommandList; + callback attribute acceptedCommandList; + callback attribute eventList; + callback attribute attributeList; ram attribute featureMap default = 0; ram attribute clusterRevision default = 1; handle command RetrieveLogsRequest; + handle command RetrieveLogsResponse; } server cluster GeneralDiagnostics { diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap index b05a00644b10fc..011fd91f522250 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap @@ -17,12 +17,6 @@ } ], "package": [ - { - "pathRelativity": "relativeToZap", - "path": "../../../src/app/zap-templates/app-templates.json", - "type": "gen-templates-json", - "version": "chip-v1" - }, { "pathRelativity": "relativeToZap", "path": "../../../src/app/zap-templates/zcl/zcl-with-test-extensions.json", @@ -30,6 +24,12 @@ "category": "matter", "version": 1, "description": "Matter SDK ZCL data with some extensions" + }, + { + "pathRelativity": "relativeToZap", + "path": "../../../src/app/zap-templates/app-templates.json", + "type": "gen-templates-json", + "version": "chip-v1" } ], "endpointTypes": [ @@ -2628,6 +2628,66 @@ } ] }, + { + "name": "Diagnostic Logs", + "code": 50, + "mfgCode": null, + "define": "DIAGNOSTIC_LOGS_CLUSTER", + "side": "client", + "enabled": 1, + "commands": [ + { + "name": "RetrieveLogsRequest", + "code": 0, + "mfgCode": null, + "source": "client", + "isIncoming": 0, + "isEnabled": 1 + }, + { + "name": "RetrieveLogsResponse", + "code": 1, + "mfgCode": null, + "source": "server", + "isIncoming": 1, + "isEnabled": 1 + } + ], + "attributes": [ + { + "name": "FeatureMap", + "code": 65532, + "mfgCode": null, + "side": "client", + "type": "bitmap32", + "included": 1, + "storageOption": "RAM", + "singleton": 0, + "bounded": 0, + "defaultValue": "0", + "reportable": 1, + "minInterval": 1, + "maxInterval": 65534, + "reportableChange": 0 + }, + { + "name": "ClusterRevision", + "code": 65533, + "mfgCode": null, + "side": "client", + "type": "int16u", + "included": 1, + "storageOption": "RAM", + "singleton": 0, + "bounded": 0, + "defaultValue": "1", + "reportable": 1, + "minInterval": 1, + "maxInterval": 65534, + "reportableChange": 0 + } + ] + }, { "name": "Diagnostic Logs", "code": 50, @@ -2643,9 +2703,81 @@ "source": "client", "isIncoming": 1, "isEnabled": 1 + }, + { + "name": "RetrieveLogsResponse", + "code": 1, + "mfgCode": null, + "source": "server", + "isIncoming": 0, + "isEnabled": 1 } ], "attributes": [ + { + "name": "GeneratedCommandList", + "code": 65528, + "mfgCode": null, + "side": "server", + "type": "array", + "included": 1, + "storageOption": "External", + "singleton": 0, + "bounded": 0, + "defaultValue": "", + "reportable": 1, + "minInterval": 1, + "maxInterval": 65534, + "reportableChange": 0 + }, + { + "name": "AcceptedCommandList", + "code": 65529, + "mfgCode": null, + "side": "server", + "type": "array", + "included": 1, + "storageOption": "External", + "singleton": 0, + "bounded": 0, + "defaultValue": "", + "reportable": 1, + "minInterval": 1, + "maxInterval": 65534, + "reportableChange": 0 + }, + { + "name": "EventList", + "code": 65530, + "mfgCode": null, + "side": "server", + "type": "array", + "included": 1, + "storageOption": "External", + "singleton": 0, + "bounded": 0, + "defaultValue": "", + "reportable": 1, + "minInterval": 1, + "maxInterval": 65534, + "reportableChange": 0 + }, + { + "name": "AttributeList", + "code": 65531, + "mfgCode": null, + "side": "server", + "type": "array", + "included": 1, + "storageOption": "External", + "singleton": 0, + "bounded": 0, + "defaultValue": "", + "reportable": 1, + "minInterval": 1, + "maxInterval": 65534, + "reportableChange": 0 + }, { "name": "FeatureMap", "code": 65532, @@ -6136,6 +6268,7 @@ "define": "FAULT_INJECTION_CLUSTER", "side": "server", "enabled": 1, + "apiMaturity": "internal", "commands": [ { "name": "FailAtFault", @@ -6641,6 +6774,7 @@ "define": "SCENES_CLUSTER", "side": "server", "enabled": 1, + "apiMaturity": "provisional", "commands": [ { "name": "AddScene", @@ -13698,6 +13832,7 @@ "define": "FAN_CONTROL_CLUSTER", "side": "server", "enabled": 1, + "apiMaturity": "provisional", "commands": [ { "name": "Step", @@ -15060,6 +15195,7 @@ "define": "BALLAST_CONFIGURATION_CLUSTER", "side": "server", "enabled": 1, + "apiMaturity": "provisional", "attributes": [ { "name": "PhysicalMinLevel", @@ -18982,6 +19118,7 @@ "define": "UNIT_TESTING_CLUSTER", "side": "server", "enabled": 1, + "apiMaturity": "internal", "commands": [ { "name": "Test", @@ -20943,6 +21080,7 @@ "define": "SCENES_CLUSTER", "side": "server", "enabled": 1, + "apiMaturity": "provisional", "commands": [ { "name": "AddScene", diff --git a/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h b/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h new file mode 100644 index 00000000000000..7abeefcff96d01 --- /dev/null +++ b/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h @@ -0,0 +1,78 @@ +/* + * + * Copyright (c) 2023 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 app { +namespace Clusters { +namespace DiagnosticLogs { + +/** + * The application delegate to statically define the options. + */ + +class LogProvider : public LogProviderDelegate +{ + static LogSessionHandle sLogSessionHandle; + static LogProvider sInstance; + +public: + LogSessionHandle StartLogCollection(IntentEnum logType); + + uint64_t GetNextChunk(LogSessionHandle logSessionHandle, chip::MutableByteSpan & outBuffer, bool & outIsEOF); + + void EndLogCollection(LogSessionHandle logSessionHandle); + + uint64_t GetTotalNumberOfBytesConsumed(LogSessionHandle logSessionHandle); + + void SetEndUserSupportLogFileDesignator(const char * logFileName); + + void SetNetworkDiagnosticsLogFileDesignator(const char * logFileName); + + void SetCrashLogFileDesignator(const char * logFileName); + + LogProvider() = default; + + ~LogProvider() = default; + + static inline LogProvider & getLogProvider() { return sInstance; } + +private: + const char * GetLogFilePath(IntentEnum logType); + + char mEndUserSupportLogFileDesignator[kLogFileDesignatorMaxLen]; + char mNetworkDiagnosticsLogFileDesignator[kLogFileDesignatorMaxLen]; + char mCrashLogFileDesignator[kLogFileDesignatorMaxLen]; + + // std::ifstream mFileStream; + + LogSessionHandle mLogSessionHandle; + + uint64_t mTotalNumberOfBytesConsumed; +}; + +} // namespace DiagnosticLogs +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp new file mode 100644 index 00000000000000..ab03b73d193ae2 --- /dev/null +++ b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp @@ -0,0 +1,141 @@ +/* + * + * Copyright (c) 2023 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 "diagnostic-logs-provider-delegate-impl.h" +#include +#include + +using namespace chip; +using namespace chip::app::Clusters::DiagnosticLogs; + +constexpr uint16_t kChunkSizeZero = 0; + +LogProvider LogProvider::sInstance; + +LogSessionHandle LogProvider::sLogSessionHandle; + +LogSessionHandle LogProvider::StartLogCollection(IntentEnum logType) +{ + + mTotalNumberOfBytesConsumed = 0; + + // Open the file of type + const char * fileName = GetLogFilePath(logType); + ChipLogError(BDX, "get file name %s", fileName); + if (fileName != nullptr) + { + // + /*std::ifstream mFileStream(fileName, std::ifstream::in); + //mFileStream.open(fileName, std::ifstream::in); + if (!mFileStream.good()) + { + ChipLogError(BDX, "Failed to open the log file"); + return kInvalidLogSessionHandle; + }*/ + sLogSessionHandle++; + mLogSessionHandle = sLogSessionHandle; + return mLogSessionHandle; + } + else + { + return kInvalidLogSessionHandle; + } +} + +uint64_t LogProvider::GetNextChunk(LogSessionHandle logSessionHandle, chip::MutableByteSpan & outBuffer, bool & outIsEOF) +{ + if (logSessionHandle != mLogSessionHandle && outBuffer.size() == 0) + { + return kChunkSizeZero; + } + + const char * fd = "/tmp/bdxlogs.txt"; + + std::ifstream logFile(fd, std::ifstream::in); + if (!logFile.good()) + { + ChipLogError(BDX, "Failed to open the log file"); + return kChunkSizeZero; + } + + logFile.seekg(static_cast(mTotalNumberOfBytesConsumed)); + logFile.read(reinterpret_cast(outBuffer.data()), kLogContentMaxSize); + + if (!(logFile.good() || logFile.eof())) + { + ChipLogError(BDX, "Failed to read the log file"); + logFile.close(); + return kChunkSizeZero; + } + + outIsEOF = (logFile.peek() == EOF); + uint64_t bytesRead = static_cast(logFile.gcount()); + + ChipLogError(BDX, "GetNextChunk bytesRead %llu outIsEOF %d", bytesRead, outIsEOF); + mTotalNumberOfBytesConsumed += bytesRead; + logFile.close(); + return bytesRead; +} + +void LogProvider::EndLogCollection(LogSessionHandle logSessionHandle) +{ + if (logSessionHandle == mLogSessionHandle) + { + // logFile.close(); + } +} + +uint64_t LogProvider::GetTotalNumberOfBytesConsumed(LogSessionHandle logSessionHandle) +{ + if (logSessionHandle == mLogSessionHandle) + { + return mTotalNumberOfBytesConsumed; + } + return kChunkSizeZero; +} + +const char * LogProvider::GetLogFilePath(IntentEnum logType) +{ + ChipLogError(BDX, "GetLogFilePath %hu", logType); + switch (logType) + { + case IntentEnum::kEndUserSupport: + return mEndUserSupportLogFileDesignator; + case IntentEnum::kNetworkDiag: + return mNetworkDiagnosticsLogFileDesignator; + case IntentEnum::kCrashLogs: + return mCrashLogFileDesignator; + default: + return nullptr; + } +} + +void LogProvider::SetEndUserSupportLogFileDesignator(const char * logFileName) +{ + ChipLogError(BDX, "SetEndUserSupportLogFileDesignator %s", logFileName); + strncpy(mEndUserSupportLogFileDesignator, logFileName, strlen(logFileName)); +} + +void LogProvider::SetNetworkDiagnosticsLogFileDesignator(const char * logFileName) +{ + strncpy(mNetworkDiagnosticsLogFileDesignator, logFileName, strlen(logFileName)); +} + +void LogProvider::SetCrashLogFileDesignator(const char * logFileName) +{ + strncpy(mCrashLogFileDesignator, logFileName, strlen(logFileName)); +} diff --git a/examples/all-clusters-app/linux/AppOptions.cpp b/examples/all-clusters-app/linux/AppOptions.cpp index ef42660972f8d9..93ec06f4d1359d 100644 --- a/examples/all-clusters-app/linux/AppOptions.cpp +++ b/examples/all-clusters-app/linux/AppOptions.cpp @@ -18,17 +18,22 @@ #include "AppOptions.h" +#include "diagnostic-logs-provider-delegate-impl.h" #include #include using namespace chip::ArgParser; +using namespace chip::app::Clusters::DiagnosticLogs; using chip::ArgParser::OptionDef; using chip::ArgParser::OptionSet; using chip::ArgParser::PrintArgError; -constexpr uint16_t kOptionDacProviderFilePath = 0xFF01; -constexpr uint16_t kOptionMinCommissioningTimeout = 0xFF02; +constexpr uint16_t kOptionDacProviderFilePath = 0xFF01; +constexpr uint16_t kOptionMinCommissioningTimeout = 0xFF02; +constexpr uint16_t kOptionEndUserSupportFilePath = 0xFF03; +constexpr uint16_t kOptionNetworkDiagnosticsFilePath = 0xFF04; +constexpr uint16_t kOptionCrashFilePath = 0xFF05; static chip::Credentials::Examples::TestHarnessDACProvider mDacProvider; @@ -38,6 +43,7 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id switch (identifier) { case kOptionDacProviderFilePath: + ChipLogError(BDX, "dac provider path"); mDacProvider.Init(value); break; case kOptionMinCommissioningTimeout: { @@ -45,6 +51,19 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id commissionMgr.OverrideMinCommissioningTimeout(chip::System::Clock::Seconds16(static_cast(atoi(value)))); break; } + case kOptionEndUserSupportFilePath: { + ChipLogError(BDX, "kOptionEndUserSupportFilePath setting end user fd"); + LogProvider::getLogProvider().SetEndUserSupportLogFileDesignator(value); + break; + } + case kOptionNetworkDiagnosticsFilePath: { + LogProvider::getLogProvider().SetNetworkDiagnosticsLogFileDesignator(value); + break; + } + case kOptionCrashFilePath: { + LogProvider::getLogProvider().SetCrashLogFileDesignator(value); + break; + } default: PrintArgError("%s: INTERNAL ERROR: Unhandled option: %s\n", program, name); retval = false; @@ -59,6 +78,9 @@ OptionSet * AppOptions::GetOptions() static OptionDef optionsDef[] = { { "dac_provider", kArgumentRequired, kOptionDacProviderFilePath }, { "min_commissioning_timeout", kArgumentRequired, kOptionMinCommissioningTimeout }, + { "end_user_support_log", kArgumentRequired, kOptionEndUserSupportFilePath }, + { "network_diagnostics_log", kArgumentRequired, kOptionNetworkDiagnosticsFilePath }, + { "crash_log", kArgumentRequired, kOptionCrashFilePath }, {}, }; @@ -68,6 +90,12 @@ OptionSet * AppOptions::GetOptions() " A json file with data used by the example dac provider to validate device attestation procedure.\n" " --min_commissioning_timeout \n" " The minimum time in seconds during which commissioning session establishment is allowed by the Node.\n" + " --end_user_support_log \n" + " The end user support log file to be used for diagnostic logs transfer.\n" + " --network_diagnostics_log \n" + " The network diagnostics log file to be used for diagnostic logs transfer.\n" + " --crash_log \n" + " The crash log file to be used for diagnostic logs transfer\n" }; return &options; diff --git a/examples/all-clusters-app/linux/BUILD.gn b/examples/all-clusters-app/linux/BUILD.gn index 3a50001c1ce3fb..6e6bb5bcd41378 100644 --- a/examples/all-clusters-app/linux/BUILD.gn +++ b/examples/all-clusters-app/linux/BUILD.gn @@ -25,6 +25,7 @@ source_set("chip-all-clusters-common") { "${chip_root}/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/concentration-measurement-instances.cpp", + "${chip_root}/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/dishwasher-alarm-stub.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/dishwasher-mode.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/fan-stub.cpp", diff --git a/examples/all-clusters-app/linux/include/CHIPProjectAppConfig.h b/examples/all-clusters-app/linux/include/CHIPProjectAppConfig.h index 7346ebd2807bfb..efe6b19c26bb89 100644 --- a/examples/all-clusters-app/linux/include/CHIPProjectAppConfig.h +++ b/examples/all-clusters-app/linux/include/CHIPProjectAppConfig.h @@ -42,3 +42,5 @@ // Marks that a ModeBase Derived cluster is being used. #define EMBER_AF_PLUGIN_MODE_BASE + +#define CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER 1 diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index a6deafde128983..22552531de684d 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -19,6 +19,7 @@ #include "AllClustersCommandDelegate.h" #include "WindowCoveringManager.h" #include "air-quality-instance.h" +#include "diagnostic-logs-provider-delegate-impl.h" #include "dishwasher-mode.h" #include "include/tv-callbacks.h" #include "laundry-washer-controls-delegate-impl.h" @@ -30,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -266,3 +268,10 @@ void emberAfWindowCoveringClusterInitCallback(chip::EndpointId endpoint) Clusters::WindowCovering::SetDefaultDelegate(endpoint, &sWindowCoveringManager); Clusters::WindowCovering::ConfigStatusUpdateFeatures(endpoint); } + +using namespace chip::app::Clusters::DiagnosticLogs; +void emberAfDiagnosticLogsClusterInitCallback(chip::EndpointId endpoint) +{ + ChipLogProgress(NotSpecified, "SetDefaultLogProviderDelegate"); + DiagnosticLogsServer::Instance().SetDefaultLogProviderDelegate(endpoint, &LogProvider::getLogProvider()); +} diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp new file mode 100644 index 00000000000000..24182173416273 --- /dev/null +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp @@ -0,0 +1,305 @@ +/* + * + * Copyright (c) 2021 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 + +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER +#include "DiagnosticLogsBDXTransferHandler.h" +#include +#include +#include + +using chip::BitFlags; +using chip::ByteSpan; +using chip::CharSpan; +using chip::FabricIndex; +using chip::FabricInfo; +using chip::MutableCharSpan; +using chip::NodeId; +using chip::Span; +using chip::bdx::TransferControlFlags; +using chip::Protocols::InteractionModel::Status; + +using namespace chip; +using namespace chip::app; +using namespace chip::bdx; +using namespace chip::app::Clusters::DiagnosticLogs; + +// BDX Transfer Params +constexpr System::Clock::Timeout kBdxPollIntervalMs = System::Clock::Milliseconds32(50); + +// Timeout for the BDX transfer session. The OTA Spec mandates this should be >= 5 minutes. +constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); + +constexpr uint16_t kBdxMaxBlockSize = 1024; + +CHIP_ERROR DiagnosticLogsBDXTransferHandler::InitializeTransfer(chip::Messaging::ExchangeContext * exchangeCtx, + FabricIndex fabricIndex, NodeId nodeId, + LogProviderDelegate * delegate, IntentEnum intent, + CharSpan fileDesignator) +{ + if (mInitialized) + { + // Prevent a new node connection since another is active. + VerifyOrReturnError(mFabricIndex.Value() == fabricIndex && mNodeId.Value() == nodeId, CHIP_ERROR_BUSY); + + // Reset stale connection from the same Node if exists. + Reset(); + } + + VerifyOrReturnError(exchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE); + + mExchangeCtx = exchangeCtx->GetExchangeMgr()->NewContext(exchangeCtx->GetSessionHandle(), this); + VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY); + mIntent = intent; + mDelegate = delegate; + + mFabricIndex.SetValue(fabricIndex); + mNodeId.SetValue(nodeId); + + mNumBytesSent = 0; + + TransferSession::TransferInitData initOptions; + initOptions.TransferCtlFlags = bdx::TransferControlFlags::kSenderDrive; + initOptions.MaxBlockSize = kBdxMaxBlockSize; + initOptions.FileDesLength = static_cast(fileDesignator.size()); + initOptions.FileDesignator = reinterpret_cast(fileDesignator.data()); + + CHIP_ERROR err = Initiator::InitiateTransfer(&DeviceLayer::SystemLayer(), bdx::TransferRole::kSender, initOptions, kBdxTimeout, + kBdxPollIntervalMs); + if (err != CHIP_NO_ERROR) + { + LogErrorOnFailure(err); + return err; + } + + mInitialized = true; + return CHIP_NO_ERROR; +} + +void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSession::OutputEvent & event) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + + ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(event.EventType)); + switch (event.EventType) + { + case TransferSession::OutputEventType::kAckEOFReceived: + mStopPolling = true; // Stop polling the TransferSession only after receiving BlockAckEOF + Reset(); + break; + case TransferSession::OutputEventType::kStatusReceived: + ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); + Reset(); + break; + case TransferSession::OutputEventType::kInternalError: + ChipLogError(BDX, "InternalError"); + Reset(); + break; + case TransferSession::OutputEventType::kTransferTimeout: + ChipLogError(BDX, "Transfer timed out"); + Reset(); + break; + case TransferSession::OutputEventType::kMsgToSend: { + Messaging::SendFlags sendFlags; + if (!event.msgTypeData.HasMessageType(chip::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. + sendFlags.Set(chip::Messaging::SendMessageFlags::kExpectResponse); + } + VerifyOrReturn(mExchangeCtx != nullptr); + err = mExchangeCtx->SendMessage(event.msgTypeData.ProtocolId, event.msgTypeData.MessageType, std::move(event.MsgData), + sendFlags); + + if (err == CHIP_NO_ERROR) + { + if (!sendFlags.Has(chip::Messaging::SendMessageFlags::kExpectResponse)) + { + // After sending the StatusReport, exchange context gets closed so, set mExchangeCtx to null + mExchangeCtx = nullptr; + } + } + else + { + ChipLogError(BDX, "SendMessage failed: %" CHIP_ERROR_FORMAT, err.Format()); + Reset(); + } + + break; + } + case TransferSession::OutputEventType::kAcceptReceived: { + uint16_t blockSize = mTransfer.GetTransferBlockSize(); + + uint16_t bytesToRead = blockSize; + + if (mTransfer.GetTransferLength() > 0 && mNumBytesSent + blockSize > mTransfer.GetTransferLength()) + { + // cast should be safe because of condition above + bytesToRead = static_cast(mTransfer.GetTransferLength() - mNumBytesSent); + } + + chip::System::PacketBufferHandle blockBuf = chip::System::PacketBufferHandle::New(bytesToRead); + if (blockBuf.IsNull()) + { + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_NO_MEMORY); + return; + } + + mLogSessionHandle = mDelegate->StartLogCollection(mIntent); + + if (mLogSessionHandle == kInvalidLogSessionHandle) + { + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_INCORRECT_STATE); + return; + } + + // Send a response to the RetreiveLogRequest since we got a SendAccept message from the requestor. + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_NO_ERROR); + + MutableByteSpan buffer; + + buffer = MutableByteSpan(blockBuf->Start(), bytesToRead); + + bool isEOF = false; + + // Get the log next chunk + uint64_t bytesRead = mDelegate->GetNextChunk(mLogSessionHandle, buffer, isEOF); + + if (bytesRead == 0) + { + mTransfer.AbortTransfer(StatusCode::kUnknown); + return; + } + + if (isEOF) + { + mDelegate->EndLogCollection(mLogSessionHandle); + mLogSessionHandle = kInvalidLogSessionHandle; + } + + TransferSession::BlockData blockData; + blockData.Data = blockBuf->Start(); + blockData.Length = static_cast(bytesRead); + blockData.IsEof = isEOF; + mNumBytesSent += static_cast(blockData.Length); + + err = mTransfer.PrepareBlock(blockData); + if (err != CHIP_NO_ERROR) + { + ChipLogError(BDX, "PrepareBlock failed: %" CHIP_ERROR_FORMAT, err.Format()); + mTransfer.AbortTransfer(StatusCode::kUnknown); + } + break; + } + case TransferSession::OutputEventType::kAckReceived: { + uint16_t blockSize = mTransfer.GetTransferBlockSize(); + uint16_t bytesToRead = blockSize; + + if (mTransfer.GetTransferLength() > 0 && mNumBytesSent + blockSize > mTransfer.GetTransferLength()) + { + // cast should be safe because of condition above + bytesToRead = static_cast(mTransfer.GetTransferLength() - mNumBytesSent); + } + + chip::System::PacketBufferHandle blockBuf = chip::System::PacketBufferHandle::New(bytesToRead); + if (blockBuf.IsNull()) + { + mTransfer.AbortTransfer(StatusCode::kUnknown); + return; + } + + MutableByteSpan buffer; + + buffer = MutableByteSpan(blockBuf->Start(), bytesToRead); + + bool isEOF = false; + + // Get the log next chunk and see if it fits i.e. if EOF is reported + uint64_t bytesRead = mDelegate->GetNextChunk(mLogSessionHandle, buffer, isEOF); + + if (bytesRead == 0) + { + mTransfer.AbortTransfer(StatusCode::kUnknown); + return; + } + + if (isEOF) + { + mDelegate->EndLogCollection(mLogSessionHandle); + mLogSessionHandle = kInvalidLogSessionHandle; + } + + TransferSession::BlockData blockData; + blockData.Data = blockBuf->Start(); + blockData.Length = static_cast(bytesRead); + blockData.IsEof = isEOF; + mNumBytesSent += static_cast(blockData.Length); + + err = mTransfer.PrepareBlock(blockData); + if (err != CHIP_NO_ERROR) + { + ChipLogError(BDX, "PrepareBlock failed: %" CHIP_ERROR_FORMAT, err.Format()); + mTransfer.AbortTransfer(StatusCode::kUnknown); + } + break; + } + case TransferSession::OutputEventType::kNone: + case TransferSession::OutputEventType::kInitReceived: + case TransferSession::OutputEventType::kQueryReceived: + break; + default: + // TransferSession should prevent this case from happening. + ChipLogError(BDX, "Unsupported event type"); + break; + } +} + +void DiagnosticLogsBDXTransferHandler::Reset() +{ + assertChipStackLockedByCurrentThread(); + if (mNodeId.HasValue() && mFabricIndex.HasValue()) + { + ChipLogProgress(Controller, + "Resetting state for Diagnostic Logs Provider; no longer sending logs to node id 0x" ChipLogFormatX64 + ", fabric index %u", + ChipLogValueX64(mNodeId.Value()), mFabricIndex.Value()); + } + else + { + ChipLogProgress(Controller, "Resetting state for Diagnostic Logs Provider"); + } + + mFabricIndex.ClearValue(); + mNodeId.ClearValue(); + + Initiator::ResetTransfer(); + + if (mExchangeCtx != nullptr) + { + mExchangeCtx->Close(); + mExchangeCtx = nullptr; + } + + mDelegate = nullptr; + mInitialized = false; + mNumBytesSent = 0; +} + +#endif diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h new file mode 100644 index 00000000000000..8f7bda5edf0e60 --- /dev/null +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h @@ -0,0 +1,77 @@ +/* + * + * Copyright (c) 2023 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 + +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER + +#pragma once + +#include + +#include + +namespace chip { +namespace app { +namespace Clusters { +namespace DiagnosticLogs { +/** + * An implementation of the handler than initiates a BDX transfer as a Sender using the synchronous Sender Drive + * transfer mode. It gets the chunks of the log from the accessory and sends the block accross to the receiver until + * all the blocks have been transferred and the accessory reports that end of file is reached. + */ +class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator +{ +public: + DiagnosticLogsBDXTransferHandler() {} + ~DiagnosticLogsBDXTransferHandler() {} + + CHIP_ERROR Init(); + + CHIP_ERROR InitializeTransfer(chip::Messaging::ExchangeContext * exchangeCtx, chip::FabricIndex fabricIndex, + chip::NodeId nodeId, LogProviderDelegate * delegate, IntentEnum intent, + chip::CharSpan fileDesignator); + + void HandleTransferSessionOutput(chip::bdx::TransferSession::OutputEvent & event) override; + + void Reset(); + +private: + chip::Optional mFabricIndex; + chip::Optional mNodeId; + + chip::Messaging::ExchangeContext * mExchangeCtx; + + bool mInitialized; + + uint64_t mNumBytesSent; + + LogSessionHandle mLogSessionHandle; + + LogProviderDelegate * mDelegate; + + IntentEnum mIntent; + + char mFileDesignator[chip::bdx::kMaxFileDesignatorLen]; +}; + +} // namespace DiagnosticLogs +} // namespace Clusters +} // namespace app +} // namespace chip +#endif diff --git a/src/app/chip_data_model.gni b/src/app/chip_data_model.gni index 7490e395eec930..fe83039c50566d 100644 --- a/src/app/chip_data_model.gni +++ b/src/app/chip_data_model.gni @@ -314,6 +314,14 @@ template("chip_data_model") { "${_app_root}/clusters/${cluster}/${cluster}.cpp", "${_app_root}/clusters/${cluster}/${cluster}.h", ] + } else if (cluster == "diagnostic-logs-server") { + sources += [ + "${_app_root}/bdx/DiagnosticLogsBDXTransferHandler.cpp", + "${_app_root}/bdx/DiagnosticLogsBDXTransferHandler.h", + "${_app_root}/clusters/${cluster}/${cluster}.cpp", + "${_app_root}/clusters/${cluster}/${cluster}.h", + "${_app_root}/clusters/${cluster}/diagnostic-logs-provider-delegate.h", + ] } else { sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ] } diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-provider-delegate.h b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-provider-delegate.h new file mode 100644 index 00000000000000..99905874a379a8 --- /dev/null +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-provider-delegate.h @@ -0,0 +1,86 @@ +/* + * + * Copyright (c) 2023 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 app { +namespace Clusters { +namespace DiagnosticLogs { + +typedef uint16_t LogSessionHandle; + +// The value 0xFF will be used as an invalid log session handle and must not be used as a valid value for LogSessionHandle +constexpr uint8_t kInvalidLogSessionHandle = 0xFF; + +/** @brief + * Defines methods for implementing application-specific logic for getting the log data from the diagnostic logs provider + * LogProviderDelegate. + */ +class LogProviderDelegate +{ +public: + LogProviderDelegate() = default; + + virtual ~LogProviderDelegate() = default; + + /** + * Called to start log collection for the log type passed in. + * + * @param[in] logType The type of log for which the start of log collection is requested. + * + * @return LogSessionHandle The unique log session handle that identifies the log collection session that has been started. + */ + virtual LogSessionHandle StartLogCollection(IntentEnum logType) = 0; + + /** + * Called to get the next chunk for the log session identified by logSessionHandle. + * Should return the number of bytes read and indicate if EOF has been reached. + * + * @param[in] logSessionHandle The unique handle for this log session returned from a call to StartLogCollection. + * @param[out] outBuffer The buffer thats passed in by the caller to write to. + * @param[in] bufferLen The size of the buffer passed in. + * @param[out] outIsEOF Set to true if EOF is reached otherwise set to false. + */ + virtual uint64_t GetNextChunk(LogSessionHandle logSessionHandle, chip::MutableByteSpan & outBuffer, bool & outIsEOF) = 0; + + /** + * Called to end log collection for the log session identified by logSessionHandle + * + * @param[in] logSessionHandle The unique handle for this log session returned from a call to StartLogCollection. + * + */ + virtual void EndLogCollection(LogSessionHandle logSessionHandle) = 0; + + /** + * Called to get the total number of bytes consumed from the log session identified by logSessionHandle + * + * @param[in] logSessionHandle The unique handle for this log session returned from a call to StartLogCollection. + * + */ + virtual uint64_t GetTotalNumberOfBytesConsumed(LogSessionHandle logSessionHandle) = 0; +}; + +} // namespace DiagnosticLogs +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index 2965beff147d3b..4bb75a59d1f14f 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -15,104 +15,281 @@ * limitations under the License. */ +#include + +#include "diagnostic-logs-server.h" + #include #include #include #include -#include -#include -#include -#include +#include #include +#include + +#include + +using namespace chip; +using namespace chip::app; +using namespace chip::app::Clusters::DiagnosticLogs; +using chip::Protocols::InteractionModel::Status; + +static constexpr size_t kDiagnosticLogsLogProviderDelegateTableSize = + EMBER_AF_DIAGNOSTIC_LOGS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; +static_assert(kDiagnosticLogsLogProviderDelegateTableSize < kEmberInvalidEndpointIndex, + "DiagnosticLogs LogProviderDelegate table size error"); -#include +namespace chip { +namespace app { +namespace Clusters { +namespace DiagnosticLogs { -// We store our times as millisecond times, to save space. -using TimeInBufferType = chip::System::Clock::Milliseconds32::rep; +LogProviderDelegate * gLogProviderDelegateTable[kDiagnosticLogsLogProviderDelegateTableSize] = { nullptr }; -CHIP_ERROR DiagnosticLogsCommandHandler::PushLog(const chip::ByteSpan & payload) +LogProviderDelegate * GetLogProviderDelegate(EndpointId endpoint) { - auto now = std::chrono::duration_cast(chip::Server::GetInstance().TimeSinceInit()); - TimeInBufferType timeMs = now.count(); - chip::ByteSpan payloadTime(reinterpret_cast(&timeMs), sizeof(timeMs)); - return mBuffer.Push(payloadTime, payload); + uint16_t ep = + emberAfGetClusterServerEndpointIndex(endpoint, DiagnosticLogs::Id, EMBER_AF_DIAGNOSTIC_LOGS_CLUSTER_SERVER_ENDPOINT_COUNT); + return (ep >= kDiagnosticLogsLogProviderDelegateTableSize ? nullptr : gLogProviderDelegateTable[ep]); } -void DiagnosticLogsCommandHandler::InvokeCommand(HandlerContext & handlerContext) +bool isLogProviderDelegateNull(LogProviderDelegate * logProviderDelegate, EndpointId endpoint) { - HandleCommand( - handlerContext, [&](auto & _u, auto & payload) { - if (payload.requestedProtocol == chip::app::Clusters::DiagnosticLogs::TransferProtocolEnum::kUnknownEnumValue) - { - handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, - chip::Protocols::InteractionModel::Status::InvalidCommand); - return; - } + if (logProviderDelegate == nullptr) + { + ChipLogProgress(Zcl, "Diagnostic logs has no LogProviderDelegate set for endpoint:%u", endpoint); + return true; + } + return false; +} + +} // namespace DiagnosticLogs +} // namespace Clusters +} // namespace app +} // namespace chip + +DiagnosticLogsServer DiagnosticLogsServer::sInstance; + +void DiagnosticLogsServer::SetDefaultLogProviderDelegate(EndpointId endpoint, LogProviderDelegate * logProviderDelegate) +{ + uint16_t ep = + emberAfGetClusterServerEndpointIndex(endpoint, DiagnosticLogs::Id, EMBER_AF_DIAGNOSTIC_LOGS_CLUSTER_SERVER_ENDPOINT_COUNT); + // if endpoint is found + if (ep < kDiagnosticLogsLogProviderDelegateTableSize) + { + gLogProviderDelegateTable[ep] = logProviderDelegate; + } +} + +DiagnosticLogsServer & DiagnosticLogsServer::Instance() +{ + return sInstance; +} + +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER - switch (payload.intent) +bool DiagnosticLogsServer::IsBDXProtocolRequested(TransferProtocolEnum requestedProtocol) +{ + return requestedProtocol == TransferProtocolEnum::kBdx; +} + +bool DiagnosticLogsServer::HasValidFileDesignator(CharSpan transferFileDesignator) +{ + return (transferFileDesignator.size() > 0 && transferFileDesignator.size() <= kLogFileDesignatorMaxLen); +} + +CHIP_ERROR DiagnosticLogsServer::HandleLogRequestForBDXProtocol(Messaging::ExchangeContext * exchangeCtx, EndpointId endpointId, + IntentEnum intent, CharSpan fileDesignator) +{ + // if bdx is not supported and logs fit return in log content. if node doesn't support bdx have a compile time guard for the BDX + // code a) Create a handler for BDX. we need to get the exchange mgr from the command handler and create a new exchange context + // with the LogProviderDelegate pointing to the handler for BDX and send Init. b) when we get sendaccept send a response (how to + // send a delayed response save the command object). hold on to the commandhandler/obj and call the setstatus/addrepsonse. no + // send accept -> return in log content - how muchever fits send it. If a failure StatusReport is received in response to the + // SendInit message, the Node SHALL send a RetrieveLogsResponse command with a Status of Denied. where the Node is able to fit + // the entirety of the requested logs within the LogContent field, the Status field of the RetrieveLogsResponse SHALL be set to + // Exhausted + + // how to send a delayed response save the command object) - Handle object in CommandHandler. use that. set i am going to + // respond to you and continue. b) register with/ask the accessory for the blocks, specify the size of the blocks based on BDX + // negotiated block size - can accessory not support the block size? report an error -> send no logs in completion where do we + // define this api? this needs to be supported by accessories like doorlock etc. Seems like we would need a + // diagnosticlogsLogProviderDelegate here. c) When block is received, build up the block with incremented block number and send + // it across. d) request the next block once we receive a block ack. e) Send the next block until accessory says its sending the + // last block f) Send block eof g) Wait for block EOF ack and tear down the BDX stuff. h) any errors occur or bdx timeouts -> + // tear down the BDX stuff. + + VerifyOrReturnError(exchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); + + mIntent = intent; + + auto scopedPeerNodeId = exchangeCtx->GetSessionHandle()->AsSecureSession()->GetPeer(); + + LogProviderDelegate * logProviderDelegate = DiagnosticLogs::GetLogProviderDelegate(endpointId); + + VerifyOrReturnError(!(DiagnosticLogs::isLogProviderDelegateNull(logProviderDelegate, endpointId)), CHIP_ERROR_INCORRECT_STATE); + + CHIP_ERROR error = mDiagnosticLogsBDXTransferHandler.InitializeTransfer( + exchangeCtx, scopedPeerNodeId.GetFabricIndex(), scopedPeerNodeId.GetNodeId(), logProviderDelegate, intent, fileDesignator); + return error; +} + +void DiagnosticLogsServer::HandleBDXResponse(CHIP_ERROR error) +{ + LogErrorOnFailure(error); + + auto commandHandleRef = std::move(mAsyncCommandHandle); + auto commandHandle = commandHandleRef.Get(); + + if (commandHandle == nullptr) + { + ChipLogError(Controller, "DiagnosticLogsServer: Unable to handle BDX response. commandHandler is null"); + return; + } + + Commands::RetrieveLogsResponse::Type response; + if (error == CHIP_NO_ERROR) + { + response.status = StatusEnum::kSuccess; + commandHandle->AddResponse(mRequestPath, response); + } + else + { + // Fallback on the response payload log request + HandleLogRequestForResponsePayload(commandHandle, mRequestPath, mIntent); + mDiagnosticLogsBDXTransferHandler.Reset(); + mAsyncCommandHandle.Release(); + } +} + +void DiagnosticLogsServer::SetAsyncCommandHandleAndPath(CommandHandler * commandObj, const ConcreteCommandPath & commandPath) +{ + mAsyncCommandHandle = CommandHandler::Handle(commandObj); + mRequestPath = commandPath; +} + +#endif + +void DiagnosticLogsServer::HandleLogRequestForResponsePayload(CommandHandler * commandHandler, ConcreteCommandPath path, + IntentEnum intent) +{ + Commands::RetrieveLogsResponse::Type response; + mIntent = intent; + + EndpointId endpoint = path.mEndpointId; + LogProviderDelegate * logProviderDelegate = GetLogProviderDelegate(endpoint); + + if (isLogProviderDelegateNull(logProviderDelegate, endpoint)) + { + response.status = StatusEnum::kNoLogs; + commandHandler->AddResponse(path, response); + return; + } + + Platform::ScopedMemoryBuffer buffer; + + if (!buffer.Alloc(kLogContentMaxSize)) + { + ChipLogError(BDX, "buffer not allocated"); + response.status = StatusEnum::kNoLogs; + commandHandler->AddResponse(path, response); + return; + } + + mLogSessionHandle = logProviderDelegate->StartLogCollection(intent); + + MutableByteSpan mutableBuffer; + + mutableBuffer = MutableByteSpan(buffer.Get(), kLogContentMaxSize); + + bool isEOF = false; + + // Get the log next chunk + uint64_t bytesRead = logProviderDelegate->GetNextChunk(mLogSessionHandle, mutableBuffer, isEOF); + + if (bytesRead == 0) + { + response.status = StatusEnum::kNoLogs; + commandHandler->AddResponse(path, response); + return; + } + + // log fits. Return in response commandData and call end of log collection + if (bytesRead > 0) + { + if (isEOF) + { + response.status = StatusEnum::kSuccess; + } + else + { + response.status = StatusEnum::kExhausted; + } + response.logContent = ByteSpan(mutableBuffer.data(), kLogContentMaxSize); + } + else + { + response.status = StatusEnum::kNoLogs; + } + logProviderDelegate->EndLogCollection(mLogSessionHandle); + + commandHandler->AddResponse(path, response); + mLogSessionHandle = kInvalidLogSessionHandle; +} + +static void HandleRetrieveLogRequest(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, + TransferProtocolEnum protocol, IntentEnum intent, Optional transferFileDesignator) +{ + + if (protocol == TransferProtocolEnum::kResponsePayload) + { + DiagnosticLogsServer::Instance().HandleLogRequestForResponsePayload(commandObj, commandPath, intent); + } +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER + else + { + Commands::RetrieveLogsResponse::Type response; + if (!transferFileDesignator.HasValue() || + !DiagnosticLogsServer::Instance().HasValidFileDesignator(transferFileDesignator.Value())) + { + response.status = StatusEnum::kNoLogs; + commandObj->AddResponse(commandPath, response); + return; + } + + // if log fits just send it. + if (DiagnosticLogsServer::Instance().IsBDXProtocolRequested(protocol)) + { + CHIP_ERROR err = DiagnosticLogsServer::Instance().HandleLogRequestForBDXProtocol( + commandObj->GetExchangeContext(), commandPath.mEndpointId, intent, transferFileDesignator.Value()); + if (err != CHIP_NO_ERROR) { - case chip::app::Clusters::DiagnosticLogs::IntentEnum::kEndUserSupport: { - chip::app::Clusters::DiagnosticLogs::Commands::RetrieveLogsResponse::Type response; - if (mBuffer.IsEmpty()) - { - response.status = chip::app::Clusters::DiagnosticLogs::StatusEnum::kNoLogs; - handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response); - break; - } - - size_t logSize = mBuffer.GetFrontSize(); - TimeInBufferType timeFromBuffer; - VerifyOrDie(logSize > sizeof(timeFromBuffer)); - - chip::Platform::ScopedMemoryBuffer buf; - if (!buf.Calloc(logSize)) - { - response.status = chip::app::Clusters::DiagnosticLogs::StatusEnum::kBusy; - handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response); - break; - } - - // The entry is | time (4 bytes) | content (var size) | - chip::MutableByteSpan entry(buf.Get(), logSize); - CHIP_ERROR err = mBuffer.ReadFront(entry); - VerifyOrDie(err == CHIP_NO_ERROR); - memcpy(&timeFromBuffer, buf.Get(), sizeof(timeFromBuffer)); - - auto timestamp = chip::System::Clock::Milliseconds32(timeFromBuffer); - - response.status = chip::app::Clusters::DiagnosticLogs::StatusEnum::kSuccess; - response.logContent = chip::ByteSpan(buf.Get() + sizeof(timeFromBuffer), logSize - sizeof(timeFromBuffer)); - response.timeSinceBoot.SetValue(chip::System::Clock::Microseconds64(timestamp).count()); - handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response); - } - break; - case chip::app::Clusters::DiagnosticLogs::IntentEnum::kNetworkDiag: { - chip::app::Clusters::DiagnosticLogs::Commands::RetrieveLogsResponse::Type response; - response.status = chip::app::Clusters::DiagnosticLogs::StatusEnum::kNoLogs; - handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response); - } - break; - case chip::app::Clusters::DiagnosticLogs::IntentEnum::kCrashLogs: { - chip::app::Clusters::DiagnosticLogs::Commands::RetrieveLogsResponse::Type response; - response.status = chip::app::Clusters::DiagnosticLogs::StatusEnum::kNoLogs; - handlerContext.mCommandHandler.AddResponse(handlerContext.mRequestPath, response); - } - break; - case chip::app::Clusters::DiagnosticLogs::IntentEnum::kUnknownEnumValue: { - handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, - chip::Protocols::InteractionModel::Status::InvalidCommand); - break; - } + LogErrorOnFailure(err); + response.status = StatusEnum::kNoLogs; + commandObj->AddResponse(commandPath, response); + return; } - }); + DiagnosticLogsServer::Instance().SetAsyncCommandHandleAndPath(std::move(commandObj), std::move(commandPath)); + } + } +#endif } -bool emberAfDiagnosticLogsClusterRetrieveLogsRequestCallback( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::DiagnosticLogs::Commands::RetrieveLogsRequest::DecodableType & commandData) +bool emberAfDiagnosticLogsClusterRetrieveLogsRequestCallback(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, + const Commands::RetrieveLogsRequest::DecodableType & commandData) { - commandObj->AddStatus(commandPath, chip::Protocols::InteractionModel::Status::UnsupportedCommand); + if (commandData.requestedProtocol == TransferProtocolEnum::kUnknownEnumValue || + commandData.intent == IntentEnum::kUnknownEnumValue) + { + commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::InvalidCommand); + return true; + } + HandleRetrieveLogRequest(commandObj, commandPath, commandData.requestedProtocol, commandData.intent, + commandData.transferFileDesignator); return true; } -void MatterDiagnosticLogsPluginServerInitCallback() {} +void MatterDiagnosticLogsPluginServerInitCallback() +{ + // Nothing to do, the server init routine will be done in Instance::Init() +} diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h index 91348717c481c5..ca56f3da9927c2 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h @@ -18,31 +18,80 @@ #pragma once +#include + +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER +#include +#endif #include #include -#include +#include + +namespace chip { +namespace app { +namespace Clusters { +namespace DiagnosticLogs { + +// Spec mandated max file designator length +static constexpr uint8_t kLogFileDesignatorMaxLen = 32; + +static constexpr const uint16_t kDiagnosticLogsEndpoint = 0; -#include +// Spec mandated max size of the log content field in the Response paylod +static constexpr uint16_t kLogContentMaxSize = 1024; /// A reference implementation for DiagnosticLogs source. -class DiagnosticLogsCommandHandler : public chip::app::CommandHandlerInterface +class DiagnosticLogsServer { public: - static constexpr const uint16_t kDiagnosticLogsEndpoint = 0; - static constexpr const uint16_t kDiagnosticLogsBufferSize = 4 * 1024; // 4K internal memory to store text logs + static DiagnosticLogsServer & Instance(); + + /** + * Set the default delegate of the diagnostic logs cluster for the specified endpoint + * + * @param endpoint ID of the endpoint + * + * @param delegate The default lofg provider delegate at the endpoint + */ + void SetDefaultLogProviderDelegate(EndpointId endpoint, LogProviderDelegate * delegate); + + void HandleLogRequestForResponsePayload(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, + IntentEnum intent); + +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER + + void HandleBDXResponse(CHIP_ERROR error); - DiagnosticLogsCommandHandler() : - CommandHandlerInterface(chip::MakeOptional(chip::EndpointId(kDiagnosticLogsEndpoint)), - chip::app::Clusters::DiagnosticLogs::Id), - mBuffer(mStorage.data(), mStorage.size()) - {} + CHIP_ERROR HandleLogRequestForBDXProtocol(chip::Messaging::ExchangeContext * exchangeCtx, chip::EndpointId endpointId, + IntentEnum intent, chip::CharSpan fileDesignator); - // Inherited from CommandHandlerInterface - void InvokeCommand(HandlerContext & handlerContext) override; + void SetAsyncCommandHandleAndPath(CommandHandler * commandObj, const ConcreteCommandPath & commandPath); - CHIP_ERROR PushLog(const chip::ByteSpan & payload); + bool HasValidFileDesignator(chip::CharSpan transferFileDesignator); + + bool IsBDXProtocolRequested(TransferProtocolEnum requestedProtocol); + +#endif private: - std::array mStorage; - chip::BytesCircularBuffer mBuffer; +#if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER + + DiagnosticLogsBDXTransferHandler mDiagnosticLogsBDXTransferHandler; + +#endif + + LogSessionHandle mLogSessionHandle; + + chip::app::CommandHandler::Handle mAsyncCommandHandle; + chip::app::ConcreteCommandPath mRequestPath = chip::app::ConcreteCommandPath(0, 0, 0); + ; + IntentEnum mIntent; + + // Instance + static DiagnosticLogsServer sInstance; }; + +} // namespace DiagnosticLogs +} // namespace Clusters +} // namespace app +} // namespace chip diff --git a/src/darwin/Framework/CHIP/MTRDevice.h b/src/darwin/Framework/CHIP/MTRDevice.h index 2544deaa32280a..811d4bcd8660b8 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.h +++ b/src/darwin/Framework/CHIP/MTRDevice.h @@ -29,6 +29,17 @@ typedef NS_ENUM(NSUInteger, MTRDeviceState) { MTRDeviceStateUnreachable = 2 }; +/** + * This enum is used to specify the type of log requested from this device. + * + * The log types are : End User Support, Network Diagnostics and Crash logs. + */ +typedef NS_ENUM(NSInteger, MTRDiagnosticLogType) { + MTRDiagnosticLogTypeEndUserSupport = 0, // End user support log is requested + MTRDiagnosticLogTypeNetworkDiagnostics = 1, // Network Diagnostics log is requested + MTRDiagnosticLogTypeCrash = 2 // Crash log is requested +}; + @protocol MTRDeviceDelegate; @interface MTRDevice : NSObject @@ -201,6 +212,25 @@ typedef NS_ENUM(NSUInteger, MTRDeviceState) { completion:(MTRDeviceOpenCommissioningWindowHandler)completion MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0)); +/** + * Download log of the desired type from the device. + * + * Note: The consumer of this API should move the file that the NSURL in the MTRDiagnosticLogResult points to + * or open it for reading before the completion handler returns. Otherwise, the file will be deleted, and the data will be lost. + * + * @param type The type of log being requested. This should correspond to a value in the enum MTRDiagnosticLogType. + * @param timeout The timeout for getting the log. If the timeout expires, completion will be called with whatever + * has been retrieved by that point (which might be none or a partial log). + * If the timeout is set to 0, the request will not expire and completion will not be called until + * the log is fully retrieved or an error occurs. + * @param queue The queue on which completion will be called. + * @param completion The completion that will be called to pass in the URL for the requested log. + */ +- (void)downloadLogOfType:(MTRDiagnosticLogType)type + timeout:(NSTimeInterval)timeout + queue:(dispatch_queue_t)queue + completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion; + @end @protocol MTRDeviceDelegate diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index cb6c8594dfddb3..20109083aeb6d6 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -15,6 +15,7 @@ * limitations under the License. */ +#import #import #import @@ -24,21 +25,27 @@ #import "MTRBaseSubscriptionCallback.h" #import "MTRCluster.h" #import "MTRClusterConstants.h" +#import "MTRClusters_Internal.h" #import "MTRDeviceController_Internal.h" #import "MTRDevice_Internal.h" #import "MTRError_Internal.h" #import "MTREventTLVValueDecoder_Internal.h" #import "MTRLogging_Internal.h" +#import "NSDataSpanConversion.h" +#import "NSStringSpanConversion.h" #import "zap-generated/MTRCommandPayloads_Internal.h" #include "lib/core/CHIPError.h" #include "lib/core/DataModelTypes.h" #include +#include "MTRDiagnosticLogsTransferHandler.h" #include #include #include #include +#include +#include #include typedef void (^MTRDeviceAttributeReportHandler)(NSArray * _Nonnull); @@ -142,6 +149,7 @@ @interface MTRDevice () @property (nonatomic) chip::FabricIndex fabricIndex; @property (nonatomic) MTRWeakReference> * weakDelegate; @property (nonatomic) dispatch_queue_t delegateQueue; +@property (nonatomic) dispatch_source_t timerSource; @property (nonatomic) NSArray *> * unreportedEvents; /** @@ -1230,6 +1238,235 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator [baseDevice openCommissioningWindowWithDiscriminator:discriminator duration:duration queue:queue completion:completion]; } +- (NSString *)toLogTypeString:(MTRDiagnosticLogType)type +{ + switch (type) { + case MTRDiagnosticLogTypeEndUserSupport: + return @"EndUserSupport"; + case MTRDiagnosticLogTypeNetworkDiagnostics: + return @"NetworkDiagnostics"; + case MTRDiagnosticLogTypeCrash: + return @"Crash"; + default: + return @""; + } +} + +- (NSString *)_getFileDesignatorForLogType:(MTRDiagnosticLogType)type +{ + + NSString * fileDesignator = [NSString stringWithFormat:@"%@%@/%@", @"bdx:/", self.nodeID, [self toLogTypeString:type]]; + return fileDesignator; +} + +- (void)_startTimerForDownload:(NSTimeInterval)timeout + queue:(dispatch_queue_t)queue + completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion +{ + if (timeout > 0) { + _timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, DISPATCH_TIMER_STRICT, self.queue); + VerifyOrDie(_timerSource != nullptr); + + dispatch_source_set_timer( + _timerSource, dispatch_walltime(nullptr, static_cast(timeout * NSEC_PER_MSEC)), DISPATCH_TIME_FOREVER, 2 * NSEC_PER_MSEC); + + dispatch_source_set_event_handler(_timerSource, ^{ + dispatch_source_cancel(self->_timerSource); + + dispatch_async(queue, ^{ + NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeTimeout userInfo:nil]; + completion(nil, error); + }); + return; + }); + dispatch_resume(_timerSource); + } +} + +- (NSURL * _Nullable)_temporaryFileURLForDownload:(MTRDiagnosticLogType)type + queue:(dispatch_queue_t)queue + completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion +{ + NSDateFormatter * dateFormatter = [[NSDateFormatter alloc] init]; + dateFormatter.dateFormat = @"yyyy-MM-dd_HH:mm:ss.SSSZZZ"; + + NSString * timeString = [dateFormatter stringFromDate:NSDate.now]; + + NSString * fileName = [NSString stringWithFormat:@"%@_%@_%@", timeString, self.nodeID, [self toLogTypeString:type]]; + + NSURL * filePath = [NSURL fileURLWithPath:[NSTemporaryDirectory() stringByAppendingPathComponent:fileName] isDirectory:YES]; + NSError * error = nil; + + NSURL * downloadFileURL = [[NSFileManager defaultManager] URLForDirectory:NSItemReplacementDirectory + inDomain:NSUserDomainMask + appropriateForURL:filePath + create:YES + error:&error]; + if (downloadFileURL == nil || error != nil) { + return nil; + } + + if ([[NSFileManager defaultManager] createFileAtPath:[filePath path] contents:nil attributes:nil]) { + return filePath; + } + return nil; +} + +- (bool)_isErrorResponse:(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _Nullable)response +{ + // TODO: fix the comparision with kNoLogs and kExhausted + return (response == nil || (response.status != nil && [response.status intValue] != 0 && [response.status intValue] != 1) || response.logContent.length == 0); +} + +// Once the consumer calls this API- +// a) if types are not valid, return an error. no logs +// b) start a timer with the timeout if provided. + +// do the following until all requested log types are retrieved +// c) For each type requested in the bitmap, send a command to the diagnostic logs cluster with protocol BDX/intent type - log type +// d) when timeout expires, call the completion with what has been received in filepath array so far. set the timeout expired bool in the response +// e) if one file type was requested, call completion and send the filepath otherwise save the filepath in an array. +// f) if an error occurs or BDX times out and there are no more pending log files, call the completion with what has been received in filepath array so far. +// g) if more than one file types was requested, keep track of pending request and repeat c-> f +// h) if an error occurs or BDX times out and there are more pending requests, move on to the next log type. +// h) when all requested logs have been retreived successfully or an error/timeout occured send the array of filepaths +- (void)_downloadLogOfType:(MTRDiagnosticLogType)type + timeout:(NSTimeInterval)timeout + queue:(dispatch_queue_t)queue + completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion +{ + ChipLogError(Controller, "downloadLogOfType %ld", type); + if (type != MTRDiagnosticLogTypeEndUserSupport && type != MTRDiagnosticLogTypeNetworkDiagnostics && type != MTRDiagnosticLogTypeCrash) { + dispatch_async(queue, ^{ + NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]; + completion(nil, error); + }); + return; + } + + // Start a timer for the timeout and abort transfer and return the log result + [self _startTimerForDownload:timeout queue:queue completion:completion]; + + MTRClusterDiagnosticLogs * cluster = [[MTRClusterDiagnosticLogs alloc] initWithDevice:self endpointID:@(0) queue:queue]; + + if (cluster == nil) { + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + dispatch_async(queue, ^{ + NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]; + completion(nil, error); + }); + return; + } + + NSURL * filePath = [self _temporaryFileURLForDownload:type queue:queue completion:completion]; + + if (filePath == nil) { + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + dispatch_async(queue, ^{ + NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]; + completion(nil, error); + }); + } + + __block MTRDiagnosticLogsTransferHandler * diagnosticLogsTransferHandler = new MTRDiagnosticLogsTransferHandler(filePath, ^(bool result) { + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + if (result == YES) { + dispatch_async(queue, ^{ + completion(filePath, nil); + }); + } else { + dispatch_async(queue, ^{ + NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]; + completion(nil, error); + }); + } + delete (diagnosticLogsTransferHandler); + }); + + // Get the device commissionee and get the exchange manager to register for unsolicited message handler for BDX messages + [self.deviceController asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) { + if (commissioner == nil) { + dispatch_async(self.queue, ^{ + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + }); + dispatch_async(queue, ^{ + NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]; + completion(nil, error); + }); + + return; + } + + commissioner->ExchangeMgr()->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id, diagnosticLogsTransferHandler); + + dispatch_async(self.queue, ^{ + MTRDiagnosticLogsClusterRetrieveLogsRequestParams * requestParams = [[MTRDiagnosticLogsClusterRetrieveLogsRequestParams alloc] init]; + requestParams.intent = [NSNumber numberWithInteger:type]; + requestParams.requestedProtocol = [NSNumber numberWithUnsignedChar:chip::to_underlying(chip::app::Clusters::DiagnosticLogs::TransferProtocolEnum::kBdx)]; + requestParams.transferFileDesignator = [self _getFileDesignatorForLogType:type]; + + [cluster retrieveLogsRequestWithParams:requestParams expectedValues:nil expectedValueInterval:nil + completion:^(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _Nullable response, NSError * _Nullable error) { + // If we are in a BDX session and there is no error, do nothing. Completion will be called when BDX succeeds or fails. + if (diagnosticLogsTransferHandler->IsInBDXSession() && error == nil) { + return; + } + + if ([self _isErrorResponse:response]) { + NSLog(@"_downloadLogOfType error response %@", response); + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + + dispatch_async(queue, ^{ + completion(nil, error); + }); + + return; + } + + // If the response has a log content, copy it into the temporary location and send the URL otherwise we will send the full file once BDX succeeds. + if (response != nil && response.logContent != nil && response.logContent.length > 0) { + if ([response.logContent writeToURL:filePath atomically:YES]) { + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + dispatch_async(queue, ^{ + completion(filePath, nil); + }); + + return; + } + } + }]; + }); + } + errorHandler:^(NSError * error) { + if (self->_timerSource) { + dispatch_source_cancel(self->_timerSource); + } + dispatch_async(queue, ^{ + completion(nil, error); + }); + }]; +} + +- (void)downloadLogOfType:(MTRDiagnosticLogType)type + timeout:(NSTimeInterval)timeout + queue:(dispatch_queue_t)queue + completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion +{ + [self _downloadLogOfType:type timeout:timeout queue:queue completion:completion]; +} + #pragma mark - Cache management // assume lock is held diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h new file mode 100644 index 00000000000000..a13b63ef016336 --- /dev/null +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h @@ -0,0 +1,93 @@ +/** + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +// #include +// #include +// #include + +/** + * This class inherits from the AsyncResponder class and handles the BDX messages for a BDX transfer session. + * It overrides the HandleTransferSessionOutput virtual method and provides an implementation for it to handle + * the OutputEvents that are generated by the BDX transfer session state machine. + * + * An MTRDiagnosticLogsTransferHandler will be associated with a specific BDX transfer session. + * + * The lifecycle of this class is managed by the AsyncFacilitator class which calls the virtual CleanUp method + * that is implemented by this class to clean up and destroy itself and the AsyncFacilitator instances. + * Note: An object of this class can't be used after CleanUp has been called. + */ +class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { +public: + MTRDiagnosticLogsTransferHandler() + : mFileURL(nil) + { + } + + MTRDiagnosticLogsTransferHandler(NSURL * _Nonnull url, void (^_Nonnull callback)(bool)) + { + mFileURL = url; + mCallback = callback; + } + + ~MTRDiagnosticLogsTransferHandler(); + + void HandleTransferSessionOutput(chip::bdx::TransferSession::OutputEvent & event) override; + + void Reset(); + + bool IsInBDXSession() { return mIsInBDXSession; } + +protected: + CHIP_ERROR OnMessageReceived(chip::Messaging::ExchangeContext * _Nonnull ec, const chip::PayloadHeader & payloadHeader, + chip::System::PacketBufferHandle && payload) override; + +private: + CHIP_ERROR PrepareForTransfer(chip::System::Layer * _Nonnull layer, chip::FabricIndex fabricIndex, chip::NodeId nodeId); + + CHIP_ERROR ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId); + + CHIP_ERROR OnMessageToSend(chip::bdx::TransferSession::OutputEvent & event); + + CHIP_ERROR OnTransferSessionBegin(chip::bdx::TransferSession::OutputEvent & event); + + CHIP_ERROR OnTransferSessionEnd(chip::bdx::TransferSession::OutputEvent & event); + + CHIP_ERROR OnBlockReceived(chip::bdx::TransferSession::OutputEvent & event); + + // CHIP_ERROR OnUnsolicitedMessageReceived( + // const chip::PayloadHeader & payloadHeader, chip::Messaging::ExchangeDelegate * _Nonnull & newDelegate) override; + + // The fabric index of the node with which the BDX session is established. + chip::Optional mFabricIndex; + + // The node id of the node with which the BDX session is established. + chip::Optional mNodeId; + + chip::Messaging::ExchangeContext * _Nullable mExchangeCtx; + + NSURL * _Nullable mFileURL; + + NSFileHandle * _Nullable mFileHandle; + std::function mCallback; + + bool mIsInBDXSession = false; + + uint64_t downloadedBytes = 0; + uint64_t totalFileBytes = 0; +}; diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm new file mode 100644 index 00000000000000..9d7691f1664572 --- /dev/null +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -0,0 +1,296 @@ +/** + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import "MTRDeviceControllerFactory_Internal.h" +#import "MTRDeviceController_Internal.h" +#import "NSDataSpanConversion.h" + +#include "MTRDiagnosticLogsTransferHandler.h" +#include + +using namespace chip; +using namespace chip::bdx; +using namespace chip::app; + +// TODO Expose a method onto the delegate to make that configurable. +constexpr uint32_t kMaxBdxBlockSize = 1024; + +// Timeout for the BDX transfer session. The OTA Spec mandates this should be >= 5 minutes. +constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); +constexpr bdx::TransferRole kBdxRole = bdx::TransferRole::kReceiver; + +// Need DiagnosticLogsProcessorInterface handle to process block which will be implemented in darwin/linux + +// TODO: need to check how to handle MTDeviceController being shutdown. + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::PrepareForTransfer(System::Layer * _Nonnull layer, FabricIndex fabricIndex, NodeId nodeId) +{ + assertChipStackLockedByCurrentThread(); + + ReturnErrorOnFailure(ConfigureState(fabricIndex, nodeId)); + + BitFlags flags(bdx::TransferControlFlags::kReceiverDrive); + + return Responder::PrepareForTransfer(layer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout); +} + +MTRDiagnosticLogsTransferHandler::~MTRDiagnosticLogsTransferHandler() +{ +} + +bdx::StatusCode GetBdxStatusCodeFromChipError(CHIP_ERROR err) +{ + if (err == CHIP_ERROR_INCORRECT_STATE) { + return bdx::StatusCode::kUnexpectedMessage; + } + if (err == CHIP_ERROR_INVALID_ARGUMENT) { + return bdx::StatusCode::kBadMessageContents; + } + return bdx::StatusCode::kUnknown; +} + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnTransferSessionBegin(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + + VerifyOrReturnError(mFabricIndex.HasValue(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mNodeId.HasValue(), CHIP_ERROR_INCORRECT_STATE); + NSError * error = nil; + + mFileHandle = [NSFileHandle fileHandleForWritingToURL:mFileURL error:&error]; + + if (mFileHandle == nil || error != nil) { + // TODO: Map NSError to BDX error + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); + return CHIP_ERROR_INCORRECT_STATE; + } + + TransferSession::TransferAcceptData acceptData; + acceptData.ControlMode = bdx::TransferControlFlags::kSenderDrive; + acceptData.MaxBlockSize = mTransfer.GetTransferBlockSize(); + acceptData.StartOffset = mTransfer.GetStartOffset(); + acceptData.Length = mTransfer.GetTransferLength(); + + mTransfer.AcceptTransfer(acceptData); + mIsInBDXSession = true; + return CHIP_NO_ERROR; +} + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnTransferSessionEnd(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + + VerifyOrReturnError(mFabricIndex.HasValue(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mNodeId.HasValue(), CHIP_ERROR_INCORRECT_STATE); + + CHIP_ERROR error = CHIP_NO_ERROR; + if (event.EventType == TransferSession::OutputEventType::kTransferTimeout) { + error = CHIP_ERROR_TIMEOUT; + } else if (event.EventType != TransferSession::OutputEventType::kBlockReceived || !event.blockdata.IsEof) { + error = CHIP_ERROR_INTERNAL; + } + + if (error != CHIP_NO_ERROR) { + // Notify the MTRDevice via the callback that the BDX transfer has completed with error + if (mCallback) { + mCallback(NO); + } + Reset(); + return error; + } + + // Notify the MTRDevice via the callback that the BDX transfer has completed with success + if (mCallback) { + mCallback(YES); + } + Reset(); + return CHIP_NO_ERROR; +} + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnBlockReceived(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + + VerifyOrReturnError(mFabricIndex.HasValue(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mNodeId.HasValue(), CHIP_ERROR_INCORRECT_STATE); + + chip::ByteSpan blockData(event.blockdata.Data, event.blockdata.Length); + + if (mFileHandle != nil) { + [mFileHandle seekToEndOfFile]; + NSError * error = nil; + [mFileHandle writeData:AsData(blockData) error:&error]; + + if (error != nil) { + // TODO: map nserror to status code + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); + return CHIP_ERROR_INCORRECT_STATE; + } + } else { + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); + return CHIP_ERROR_INCORRECT_STATE; + } + + CHIP_ERROR err = mTransfer.PrepareBlockAck(); + LogErrorOnFailure(err); + if (err != CHIP_NO_ERROR) { + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); + return err; + } + if (event.blockdata.IsEof) { + if (mFileHandle != nil) { + [mFileHandle closeFile]; + mFileHandle = nullptr; + } + OnTransferSessionEnd(event); + } + + return CHIP_NO_ERROR; +} + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnMessageToSend(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + + VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); + // VerifyOrReturnError(mDelegate != nil, CHIP_ERROR_INCORRECT_STATE); + + Messaging::SendFlags sendFlags; + + // All messages sent from the Sender expect a response, except for a StatusReport which would indicate an error and + // the end of the transfer. + if (!event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport)) { + sendFlags.Set(Messaging::SendMessageFlags::kExpectResponse); + } + + auto & msgTypeData = event.msgTypeData; + // If there's an error sending the message, close the exchange and call ResetState. + // TODO: If we can remove the !mInitialized check in ResetState(), just calling ResetState() will suffice here. + CHIP_ERROR err + = mExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags); + if (err != CHIP_NO_ERROR) { + mExchangeCtx->Close(); + mExchangeCtx = nullptr; + Reset(); + } else if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport)) { + // If the send was successful for a status report, since we are not expecting a response the exchange context is + // already closed. We need to null out the reference to avoid having a dangling pointer. + mExchangeCtx = nullptr; + Reset(); + } + return err; +} + +void MTRDiagnosticLogsTransferHandler::HandleTransferSessionOutput(TransferSession::OutputEvent & event) +{ + assertChipStackLockedByCurrentThread(); + ChipLogError(BDX, "Got an event %s", event.ToString(event.EventType)); + CHIP_ERROR err = CHIP_NO_ERROR; + switch (event.EventType) { + case TransferSession::OutputEventType::kInitReceived: + ChipLogError(BDX, "HandleTransferSessionOutput called"); + err = OnTransferSessionBegin(event); + if (err != CHIP_NO_ERROR) { + LogErrorOnFailure(err); + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err))); + } + break; + case TransferSession::OutputEventType::kStatusReceived: + ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); + [[fallthrough]]; + case TransferSession::OutputEventType::kInternalError: + case TransferSession::OutputEventType::kTransferTimeout: + err = OnTransferSessionEnd(event); + break; + case TransferSession::OutputEventType::kBlockReceived: + err = OnBlockReceived(event); + if (err != CHIP_NO_ERROR) { + LogErrorOnFailure(err); + mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)); + } + break; + case TransferSession::OutputEventType::kMsgToSend: + err = OnMessageToSend(event); + break; + case TransferSession::OutputEventType::kQueryWithSkipReceived: + case TransferSession::OutputEventType::kQueryReceived: + case TransferSession::OutputEventType::kNone: + case TransferSession::OutputEventType::kAckReceived: + case TransferSession::OutputEventType::kAcceptReceived: + // Nothing to do. + break; + default: + // Should never happens. + chipDie(); + break; + } +} + +void MTRDiagnosticLogsTransferHandler::Reset() +{ + mIsInBDXSession = false; + mFileURL = nullptr; + mCallback = nullptr; + + if (mFileHandle != nil) { + [mFileHandle closeFile]; + mFileHandle = nullptr; + } + if (mExchangeCtx) { + mExchangeCtx->Close(); + mExchangeCtx = nullptr; + } + mFabricIndex.ClearValue(); + mNodeId.ClearValue(); + + mTransfer.Reset(); +} + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId) +{ + assertChipStackLockedByCurrentThread(); + + mFabricIndex.SetValue(fabricIndex); + mNodeId.SetValue(nodeId); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnMessageReceived( + Messaging::ExchangeContext * _Nonnull ec, const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload) +{ + CHIP_ERROR err; + mExchangeCtx = ec; + + // If we receive a ReceiveInit message, then we prepare for transfer + if (payloadHeader.HasMessageType(MessageType::SendInit)) { + NodeId nodeId = ec->GetSessionHandle()->GetPeer().GetNodeId(); + FabricIndex fabricIndex = ec->GetSessionHandle()->GetFabricIndex(); + + if (nodeId != kUndefinedNodeId && fabricIndex != kUndefinedFabricIndex) { + err = PrepareForTransfer(&(DeviceLayer::SystemLayer()), fabricIndex, nodeId); + if (err != CHIP_NO_ERROR) { + ChipLogError(Controller, "Failed to prepare for transfer for BDX"); + return err; + } + } + } + + TransferFacilitator::OnMessageReceived(ec, payloadHeader, std::move(payload)); + + return err; +} diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h index f798dab013285c..784007101d57f9 100644 --- a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h +++ b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h @@ -3144,7 +3144,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) @property (nonatomic, copy) NSNumber * _Nonnull requestedProtocol MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)); -@property (nonatomic, copy) NSString * _Nullable transferFileDesignator MTR_AVAILABLE(ios(16.5), macos(13.4), watchos(9.5), tvos(16.5)); +@property (nonatomic, copy) NSString * _Nullable transferFileDesignator MTR_AVAILABLE(ios(16.5), macos(13.0), watchos(9.5), tvos(16.5)); /** * Controls whether the command is a timed command (using Timed Invoke). * diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 083d241d104263..2d261a46168c26 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -38,6 +38,7 @@ #import static const uint16_t kPairingTimeoutInSeconds = 10; +static const uint16_t kDownloadLogTimeoutInSeconds = 1000; static const uint16_t kTimeoutInSeconds = 3; static const uint64_t kDeviceId = 0x12344321; static NSString * kOnboardingPayload = @"MT:-24J0AFN00KA0648G00"; @@ -275,7 +276,7 @@ - (void)test000_SetUp // just exists to make the setup not look like it's happening inside other // tests. } - +/* - (void)test001_ReadAttribute { XCTestExpectation * expectation = @@ -2695,6 +2696,32 @@ - (void)test900_SubscribeAllAttributes // Wait for report [self waitForExpectations:[NSArray arrayWithObject:reportExpectation] timeout:kTimeoutInSeconds]; +}*/ + +- (void)test901_DownloadLogTypeEndUserSupport +{ + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + XCTestExpectation * expectation = [self expectationWithDescription:@"Log Transfer Complete"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + + dispatch_async(queue, ^{ + MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; + XCTAssertNotNil(device); + [device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { + XCTAssertNil(error); + XCTAssertNotNil(logResult); + + NSError * attributesError = nil; + NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + + size_t fileSize = [fileAttributes fileSize]; + XCTAssertTrue(fileSize > 0); + [expectation fulfill]; + }]; + }); + [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; } - (void)test999_TearDown diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj index 33032722024600..a4ae0d8bbe17b8 100644 --- a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj +++ b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj @@ -299,6 +299,8 @@ B4E2621B2AA0D02000DBA5BC /* SleepCommand.mm in Sources */ = {isa = PBXBuildFile; fileRef = B4E262192AA0D01D00DBA5BC /* SleepCommand.mm */; }; B4E2621E2AA0D02D00DBA5BC /* WaitForCommissioneeCommand.mm in Sources */ = {isa = PBXBuildFile; fileRef = B4E2621C2AA0D02A00DBA5BC /* WaitForCommissioneeCommand.mm */; }; BA09EB43247477BA00605257 /* libCHIP.a in Frameworks */ = {isa = PBXBuildFile; fileRef = BA09EB3F2474762900605257 /* libCHIP.a */; }; + CFA0EDAD2AD9D75600C7FA40 /* MTRDiagnosticLogsTransferHandler.mm in Sources */ = {isa = PBXBuildFile; fileRef = CFA0EDAC2AD9D75600C7FA40 /* MTRDiagnosticLogsTransferHandler.mm */; }; + CFA0EDAF2AD9D76000C7FA40 /* MTRDiagnosticLogsTransferHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = CFA0EDAE2AD9D76000C7FA40 /* MTRDiagnosticLogsTransferHandler.h */; }; D4772A46285AE98400383630 /* MTRClusterConstants.h in Headers */ = {isa = PBXBuildFile; fileRef = D4772A45285AE98300383630 /* MTRClusterConstants.h */; settings = {ATTRIBUTES = (Public, ); }; }; /* End PBXBuildFile section */ @@ -660,6 +662,8 @@ B4E2621C2AA0D02A00DBA5BC /* WaitForCommissioneeCommand.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WaitForCommissioneeCommand.mm; sourceTree = ""; }; BA09EB3F2474762900605257 /* libCHIP.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libCHIP.a; path = lib/libCHIP.a; sourceTree = BUILT_PRODUCTS_DIR; }; BA107AEE2470CFBB004287EB /* chip_xcode_build_connector.sh */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.script.sh; path = chip_xcode_build_connector.sh; sourceTree = ""; }; + CFA0EDAC2AD9D75600C7FA40 /* MTRDiagnosticLogsTransferHandler.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDiagnosticLogsTransferHandler.mm; sourceTree = ""; }; + CFA0EDAE2AD9D76000C7FA40 /* MTRDiagnosticLogsTransferHandler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDiagnosticLogsTransferHandler.h; sourceTree = ""; }; D437613E285BDC0D0051FEA2 /* MTRErrorTestUtils.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRErrorTestUtils.h; sourceTree = ""; }; D437613F285BDC0D0051FEA2 /* MTRTestKeys.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRTestKeys.h; sourceTree = ""; }; D4376140285BDC0D0051FEA2 /* MTRTestStorage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRTestStorage.h; sourceTree = ""; }; @@ -1083,6 +1087,8 @@ B202528F2459E34F00F97062 /* CHIP */ = { isa = PBXGroup; children = ( + CFA0EDAE2AD9D76000C7FA40 /* MTRDiagnosticLogsTransferHandler.h */, + CFA0EDAC2AD9D75600C7FA40 /* MTRDiagnosticLogsTransferHandler.mm */, 1E4D655129C30A8700BC3478 /* MTRCommissionableBrowser.mm */, 1E4D654C29C208DD00BC3478 /* MTRCommissionableBrowser.h */, 1E4D654D29C208DD00BC3478 /* MTRCommissionableBrowserDelegate.h */, @@ -1414,6 +1420,7 @@ 3CF134AF289D90FF0017A19E /* MTROperationalCertificateIssuer.h in Headers */, 3CF134AB289D8DF70017A19E /* MTRDeviceAttestationInfo.h in Headers */, B2E0D7B2245B0B5C003C5B48 /* MTRManualSetupPayloadParser.h in Headers */, + CFA0EDAF2AD9D76000C7FA40 /* MTRDiagnosticLogsTransferHandler.h in Headers */, 3CF134A7289D8ADA0017A19E /* MTRCSRInfo.h in Headers */, B2E0D7B1245B0B5C003C5B48 /* Matter.h in Headers */, 7596A84428762729004DAE0E /* MTRDevice.h in Headers */, @@ -1746,6 +1753,7 @@ 5A6FEC9827B5C6AF00F25F42 /* MTRDeviceOverXPC.mm in Sources */, 514654492A72F9DF00904E61 /* MTRDemuxingStorage.mm in Sources */, 1E4D655229C30A8700BC3478 /* MTRCommissionableBrowser.mm in Sources */, + CFA0EDAD2AD9D75600C7FA40 /* MTRDiagnosticLogsTransferHandler.mm in Sources */, 3DA1A3562ABAB3B4004F0BB9 /* MTRAsyncWorkQueue.mm in Sources */, 3DECCB722934AFE200585AEC /* MTRLogging.mm in Sources */, 7596A84528762729004DAE0E /* MTRDevice.mm in Sources */, diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 30d6d6b8a65b36..b580a140e87969 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2020-2022 Project CHIP Authors + * Copyright (c) 2020-2023 Project CHIP Authors * Copyright (c) 2019 Google LLC. * Copyright (c) 2013-2018 Nest Labs, Inc. * @@ -1600,6 +1600,16 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; #define CHIP_CONFIG_SYNCHRONOUS_REPORTS_ENABLED 0 #endif +/** + * @def CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER + * + * @brief Enables support for diagnostic logs transfer using the BDX protocol + * + */ +#ifndef CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER +#define CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER 0 +#endif + /** * @} */ diff --git a/src/protocols/bdx/BdxTransferSession.cpp b/src/protocols/bdx/BdxTransferSession.cpp index f6c54985ab5cd1..515c7ed2dd92fe 100644 --- a/src/protocols/bdx/BdxTransferSession.cpp +++ b/src/protocols/bdx/BdxTransferSession.cpp @@ -48,7 +48,6 @@ void PrepareOutgoingMessageEvent(MessageType messageType, chip::bdx::TransferSes chip::bdx::TransferSession::MessageTypeData & outputMsgType) { static_assert(std::is_same, uint8_t>::value, "Cast is not safe"); - pendingOutput = chip::bdx::TransferSession::OutputEventType::kMsgToSend; outputMsgType.ProtocolId = chip::Protocols::MessageTypeTraits::ProtocolId(); outputMsgType.MessageType = static_cast(messageType); @@ -321,7 +320,12 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) VerifyOrReturnError(mState == TransferState::kTransferInProgress, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mRole == TransferRole::kSender, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mPendingOutput == OutputEventType::kNone, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); + bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || + (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); + if (checkAwaitingResponse) + { + VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); + } // Verify non-zero data is provided and is no longer than MaxBlockSize (BlockEOF may contain 0 length data) VerifyOrReturnError((inData.Data != nullptr) && (inData.Length <= mTransferMaxBlockSize), CHIP_ERROR_INVALID_ARGUMENT); @@ -346,7 +350,8 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) } mAwaitingResponse = true; - mLastBlockNum = mNextBlockNum++; + + mLastBlockNum = mNextBlockNum++; PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); @@ -527,6 +532,7 @@ CHIP_ERROR TransferSession::HandleStatusReportMessage(const PayloadHeader & head void TransferSession::HandleTransferInit(MessageType msgType, System::PacketBufferHandle msgData) { + VerifyOrReturn(mState == TransferState::kAwaitingInitMsg, PrepareStatusReport(StatusCode::kUnexpectedMessage)); if (mRole == TransferRole::kSender) @@ -564,7 +570,6 @@ void TransferSession::HandleTransferInit(MessageType msgType, System::PacketBuff mPendingOutput = OutputEventType::kInitReceived; mState = TransferState::kNegotiateTransferParams; - #if CHIP_AUTOMATION_LOGGING transferInit.LogMessage(msgType); #endif // CHIP_AUTOMATION_LOGGING @@ -687,18 +692,26 @@ void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgDat void TransferSession::HandleBlock(System::PacketBufferHandle msgData) { - VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + VerifyOrReturn(mRole == TransferRole::kReceiver || mRole == TransferRole::kSender, + PrepareStatusReport(StatusCode::kUnexpectedMessage)); VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || + (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); + if (checkAwaitingResponse) + { + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + } Block blockMsg; const CHIP_ERROR err = blockMsg.Parse(msgData.Retain()); VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); + if (mControlMode == TransferControlFlags::kReceiverDrive) + { + VerifyOrReturn(blockMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); + } - VerifyOrReturn(blockMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); VerifyOrReturn((blockMsg.DataLength > 0) && (blockMsg.DataLength <= mTransferMaxBlockSize), PrepareStatusReport(StatusCode::kBadMessageContents)); - if (IsTransferLengthDefinite()) { VerifyOrReturn(mNumBytesProcessed + blockMsg.DataLength <= mTransferLength, @@ -727,16 +740,24 @@ void TransferSession::HandleBlockEOF(System::PacketBufferHandle msgData) { VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + + bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || + (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); + if (checkAwaitingResponse) + { + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + } BlockEOF blockEOFMsg; const CHIP_ERROR err = blockEOFMsg.Parse(msgData.Retain()); VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); - VerifyOrReturn(blockEOFMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); + if (mControlMode == TransferControlFlags::kReceiverDrive) + { + VerifyOrReturn(blockEOFMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); + } VerifyOrReturn(blockEOFMsg.DataLength <= mTransferMaxBlockSize, PrepareStatusReport(StatusCode::kBadMessageContents)); - mBlockEventData.Data = blockEOFMsg.Data; mBlockEventData.Length = blockEOFMsg.DataLength; mBlockEventData.IsEof = true; mBlockEventData.BlockCounter = blockEOFMsg.BlockCounter; @@ -870,7 +891,6 @@ CHIP_ERROR TransferSession::VerifyProposedMode(const BitFlags Date: Mon, 30 Oct 2023 23:08:14 -0700 Subject: [PATCH 02/16] Clean up code --- .../diagnostic-logs-provider-delegate-impl.h | 6 +- ...diagnostic-logs-provider-delegate-impl.cpp | 34 ++-- .../all-clusters-app/linux/AppOptions.cpp | 39 ++++- examples/all-clusters-app/linux/AppOptions.h | 3 + .../all-clusters-app/linux/main-common.cpp | 5 + .../bdx/DiagnosticLogsBDXTransferHandler.cpp | 101 +++-------- .../bdx/DiagnosticLogsBDXTransferHandler.h | 11 +- .../diagnostic-logs-server.cpp | 64 ++++--- .../diagnostic-logs-server.h | 8 +- src/darwin/Framework/CHIP/MTRDevice.h | 7 +- src/darwin/Framework/CHIP/MTRDevice.mm | 161 +++++++----------- .../CHIP/MTRDiagnosticLogsTransferHandler.h | 18 +- .../CHIP/MTRDiagnosticLogsTransferHandler.mm | 116 ++++++------- .../Framework/CHIPTests/MTRDeviceTests.m | 63 ++++++- src/protocols/bdx/BdxTransferSession.cpp | 2 +- src/protocols/bdx/TransferFacilitator.cpp | 8 +- 16 files changed, 318 insertions(+), 328 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h b/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h index 7abeefcff96d01..85dc68eeb79bb6 100644 --- a/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h @@ -52,9 +52,9 @@ class LogProvider : public LogProviderDelegate void SetCrashLogFileDesignator(const char * logFileName); - LogProvider() = default; + LogProvider() { }; - ~LogProvider() = default; + ~LogProvider() { }; static inline LogProvider & getLogProvider() { return sInstance; } @@ -65,7 +65,7 @@ class LogProvider : public LogProviderDelegate char mNetworkDiagnosticsLogFileDesignator[kLogFileDesignatorMaxLen]; char mCrashLogFileDesignator[kLogFileDesignatorMaxLen]; - // std::ifstream mFileStream; + std::ifstream mFileStream; LogSessionHandle mLogSessionHandle; diff --git a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp index ab03b73d193ae2..163e4a20691afd 100644 --- a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp @@ -35,17 +35,14 @@ LogSessionHandle LogProvider::StartLogCollection(IntentEnum logType) // Open the file of type const char * fileName = GetLogFilePath(logType); - ChipLogError(BDX, "get file name %s", fileName); if (fileName != nullptr) { - // - /*std::ifstream mFileStream(fileName, std::ifstream::in); - //mFileStream.open(fileName, std::ifstream::in); + mFileStream.open(fileName, std::ios_base::binary | std::ios_base::in); if (!mFileStream.good()) { ChipLogError(BDX, "Failed to open the log file"); return kInvalidLogSessionHandle; - }*/ + } sLogSessionHandle++; mLogSessionHandle = sLogSessionHandle; return mLogSessionHandle; @@ -63,39 +60,34 @@ uint64_t LogProvider::GetNextChunk(LogSessionHandle logSessionHandle, chip::Muta return kChunkSizeZero; } - const char * fd = "/tmp/bdxlogs.txt"; - - std::ifstream logFile(fd, std::ifstream::in); - if (!logFile.good()) + if (!mFileStream.is_open()) { - ChipLogError(BDX, "Failed to open the log file"); + ChipLogError(BDX, "File is not open"); return kChunkSizeZero; } - logFile.seekg(static_cast(mTotalNumberOfBytesConsumed)); - logFile.read(reinterpret_cast(outBuffer.data()), kLogContentMaxSize); + mFileStream.seekg(static_cast(mTotalNumberOfBytesConsumed)); + mFileStream.read(reinterpret_cast(outBuffer.data()), kLogContentMaxSize); - if (!(logFile.good() || logFile.eof())) + if (!(mFileStream.good() || mFileStream.eof())) { ChipLogError(BDX, "Failed to read the log file"); - logFile.close(); + mFileStream.close(); return kChunkSizeZero; } - outIsEOF = (logFile.peek() == EOF); - uint64_t bytesRead = static_cast(logFile.gcount()); + uint64_t bytesRead = static_cast(mFileStream.gcount()); + outIsEOF = (mFileStream.peek() == EOF); - ChipLogError(BDX, "GetNextChunk bytesRead %llu outIsEOF %d", bytesRead, outIsEOF); mTotalNumberOfBytesConsumed += bytesRead; - logFile.close(); return bytesRead; } void LogProvider::EndLogCollection(LogSessionHandle logSessionHandle) { - if (logSessionHandle == mLogSessionHandle) + if (logSessionHandle == mLogSessionHandle && mFileStream.is_open()) { - // logFile.close(); + mFileStream.close(); } } @@ -110,7 +102,6 @@ uint64_t LogProvider::GetTotalNumberOfBytesConsumed(LogSessionHandle logSessionH const char * LogProvider::GetLogFilePath(IntentEnum logType) { - ChipLogError(BDX, "GetLogFilePath %hu", logType); switch (logType) { case IntentEnum::kEndUserSupport: @@ -126,7 +117,6 @@ const char * LogProvider::GetLogFilePath(IntentEnum logType) void LogProvider::SetEndUserSupportLogFileDesignator(const char * logFileName) { - ChipLogError(BDX, "SetEndUserSupportLogFileDesignator %s", logFileName); strncpy(mEndUserSupportLogFileDesignator, logFileName, strlen(logFileName)); } diff --git a/examples/all-clusters-app/linux/AppOptions.cpp b/examples/all-clusters-app/linux/AppOptions.cpp index 93ec06f4d1359d..734b75e2ed0e4b 100644 --- a/examples/all-clusters-app/linux/AppOptions.cpp +++ b/examples/all-clusters-app/linux/AppOptions.cpp @@ -18,9 +18,9 @@ #include "AppOptions.h" -#include "diagnostic-logs-provider-delegate-impl.h" #include #include +#include using namespace chip::ArgParser; using namespace chip::app::Clusters::DiagnosticLogs; @@ -37,6 +37,10 @@ constexpr uint16_t kOptionCrashFilePath = 0xFF05; static chip::Credentials::Examples::TestHarnessDACProvider mDacProvider; +static char mEndUserSupportLogFileDesignator[kLogFileDesignatorMaxLen]; +static char mNetworkDiagnosticsLogFileDesignator[kLogFileDesignatorMaxLen]; +static char mCrashLogFileDesignator[kLogFileDesignatorMaxLen]; + bool AppOptions::HandleOptions(const char * program, OptionSet * options, int identifier, const char * name, const char * value) { bool retval = true; @@ -53,15 +57,27 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id } case kOptionEndUserSupportFilePath: { ChipLogError(BDX, "kOptionEndUserSupportFilePath setting end user fd"); - LogProvider::getLogProvider().SetEndUserSupportLogFileDesignator(value); + if (strlen(value) > kLogFileDesignatorMaxLen) + { + PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, strlen(value)); + } + strncpy(mEndUserSupportLogFileDesignator, value, strlen(value)); break; } case kOptionNetworkDiagnosticsFilePath: { - LogProvider::getLogProvider().SetNetworkDiagnosticsLogFileDesignator(value); + if (strlen(value) > kLogFileDesignatorMaxLen) + { + PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, strlen(value)); + } + strncpy(mNetworkDiagnosticsLogFileDesignator, value, strlen(value)); break; } case kOptionCrashFilePath: { - LogProvider::getLogProvider().SetCrashLogFileDesignator(value); + if (strlen(value) > kLogFileDesignatorMaxLen) + { + PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, strlen(value)); + } + strncpy(mCrashLogFileDesignator, value, strlen(value)); break; } default: @@ -105,3 +121,18 @@ chip::Credentials::DeviceAttestationCredentialsProvider * AppOptions::GetDACProv { return &mDacProvider; } + +char * AppOptions::GetEndUserSupportLogFileDesignator() +{ + return mEndUserSupportLogFileDesignator; +} + +char * AppOptions::GetNetworkDiagnosticsLogFileDesignator() +{ + return mNetworkDiagnosticsLogFileDesignator; +} + +char * AppOptions::GetCrashLogFileDesignator() +{ + return mCrashLogFileDesignator; +} diff --git a/examples/all-clusters-app/linux/AppOptions.h b/examples/all-clusters-app/linux/AppOptions.h index 3073c66176331f..8a0fb4d3fba965 100644 --- a/examples/all-clusters-app/linux/AppOptions.h +++ b/examples/all-clusters-app/linux/AppOptions.h @@ -27,6 +27,9 @@ class AppOptions public: static chip::ArgParser::OptionSet * GetOptions(); static chip::Credentials::DeviceAttestationCredentialsProvider * GetDACProvider(); + static char * GetEndUserSupportLogFileDesignator(); + static char * GetNetworkDiagnosticsLogFileDesignator(); + static char * GetCrashLogFileDesignator(); private: static bool HandleOptions(const char * program, chip::ArgParser::OptionSet * options, int identifier, const char * name, diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index 22552531de684d..759284b2f76a92 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -17,6 +17,7 @@ */ #include "AllClustersCommandDelegate.h" +#include "AppOptions.h" #include "WindowCoveringManager.h" #include "air-quality-instance.h" #include "diagnostic-logs-provider-delegate-impl.h" @@ -274,4 +275,8 @@ void emberAfDiagnosticLogsClusterInitCallback(chip::EndpointId endpoint) { ChipLogProgress(NotSpecified, "SetDefaultLogProviderDelegate"); DiagnosticLogsServer::Instance().SetDefaultLogProviderDelegate(endpoint, &LogProvider::getLogProvider()); + LogProvider::getLogProvider().SetEndUserSupportLogFileDesignator(AppOptions::GetEndUserSupportLogFileDesignator()); + LogProvider::getLogProvider().SetNetworkDiagnosticsLogFileDesignator(AppOptions::GetNetworkDiagnosticsLogFileDesignator()); + LogProvider::getLogProvider().SetCrashLogFileDesignator(AppOptions::GetCrashLogFileDesignator()); + } diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp index 24182173416273..1994bd9c14f2a0 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp @@ -55,9 +55,6 @@ CHIP_ERROR DiagnosticLogsBDXTransferHandler::InitializeTransfer(chip::Messaging: { if (mInitialized) { - // Prevent a new node connection since another is active. - VerifyOrReturnError(mFabricIndex.Value() == fabricIndex && mNodeId.Value() == nodeId, CHIP_ERROR_BUSY); - // Reset stale connection from the same Node if exists. Reset(); } @@ -67,12 +64,11 @@ CHIP_ERROR DiagnosticLogsBDXTransferHandler::InitializeTransfer(chip::Messaging: mExchangeCtx = exchangeCtx->GetExchangeMgr()->NewContext(exchangeCtx->GetSessionHandle(), this); VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY); + mIntent = intent; mDelegate = delegate; - mFabricIndex.SetValue(fabricIndex); mNodeId.SetValue(nodeId); - mNumBytesSent = 0; TransferSession::TransferInitData initOptions; @@ -96,24 +92,28 @@ CHIP_ERROR DiagnosticLogsBDXTransferHandler::InitializeTransfer(chip::Messaging: void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSession::OutputEvent & event) { CHIP_ERROR err = CHIP_NO_ERROR; + if (event.EventType != TransferSession::OutputEventType::kNone) + { + ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(event.EventType)); + } - ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(event.EventType)); switch (event.EventType) { case TransferSession::OutputEventType::kAckEOFReceived: - mStopPolling = true; // Stop polling the TransferSession only after receiving BlockAckEOF + mStopPolling = true; Reset(); break; case TransferSession::OutputEventType::kStatusReceived: ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_INTERNAL); Reset(); break; case TransferSession::OutputEventType::kInternalError: - ChipLogError(BDX, "InternalError"); + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_INTERNAL); Reset(); break; case TransferSession::OutputEventType::kTransferTimeout: - ChipLogError(BDX, "Transfer timed out"); + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_TIMEOUT); Reset(); break; case TransferSession::OutputEventType::kMsgToSend: { @@ -139,76 +139,29 @@ void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSessi else { ChipLogError(BDX, "SendMessage failed: %" CHIP_ERROR_FORMAT, err.Format()); + DiagnosticLogsServer::Instance().HandleBDXResponse(err); Reset(); } break; } - case TransferSession::OutputEventType::kAcceptReceived: { - uint16_t blockSize = mTransfer.GetTransferBlockSize(); - - uint16_t bytesToRead = blockSize; - - if (mTransfer.GetTransferLength() > 0 && mNumBytesSent + blockSize > mTransfer.GetTransferLength()) - { - // cast should be safe because of condition above - bytesToRead = static_cast(mTransfer.GetTransferLength() - mNumBytesSent); - } - - chip::System::PacketBufferHandle blockBuf = chip::System::PacketBufferHandle::New(bytesToRead); - if (blockBuf.IsNull()) - { - DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_NO_MEMORY); - return; - } - + case TransferSession::OutputEventType::kAcceptReceived: + { mLogSessionHandle = mDelegate->StartLogCollection(mIntent); if (mLogSessionHandle == kInvalidLogSessionHandle) { + ChipLogError(BDX, "Invalid log session handle"); DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_ERROR_INCORRECT_STATE); - return; - } - - // Send a response to the RetreiveLogRequest since we got a SendAccept message from the requestor. - DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_NO_ERROR); - - MutableByteSpan buffer; - - buffer = MutableByteSpan(blockBuf->Start(), bytesToRead); - - bool isEOF = false; - - // Get the log next chunk - uint64_t bytesRead = mDelegate->GetNextChunk(mLogSessionHandle, buffer, isEOF); - - if (bytesRead == 0) - { mTransfer.AbortTransfer(StatusCode::kUnknown); return; } - - if (isEOF) - { - mDelegate->EndLogCollection(mLogSessionHandle); - mLogSessionHandle = kInvalidLogSessionHandle; - } - - TransferSession::BlockData blockData; - blockData.Data = blockBuf->Start(); - blockData.Length = static_cast(bytesRead); - blockData.IsEof = isEOF; - mNumBytesSent += static_cast(blockData.Length); - - err = mTransfer.PrepareBlock(blockData); - if (err != CHIP_NO_ERROR) - { - ChipLogError(BDX, "PrepareBlock failed: %" CHIP_ERROR_FORMAT, err.Format()); - mTransfer.AbortTransfer(StatusCode::kUnknown); - } - break; + // Send a response to the RetreiveLogRequest since we got a SendAccept message. + DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_NO_ERROR); } - case TransferSession::OutputEventType::kAckReceived: { + // Fallthrough + case TransferSession::OutputEventType::kAckReceived: + { uint16_t blockSize = mTransfer.GetTransferBlockSize(); uint16_t bytesToRead = blockSize; @@ -274,17 +227,6 @@ void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSessi void DiagnosticLogsBDXTransferHandler::Reset() { assertChipStackLockedByCurrentThread(); - if (mNodeId.HasValue() && mFabricIndex.HasValue()) - { - ChipLogProgress(Controller, - "Resetting state for Diagnostic Logs Provider; no longer sending logs to node id 0x" ChipLogFormatX64 - ", fabric index %u", - ChipLogValueX64(mNodeId.Value()), mFabricIndex.Value()); - } - else - { - ChipLogProgress(Controller, "Resetting state for Diagnostic Logs Provider"); - } mFabricIndex.ClearValue(); mNodeId.ClearValue(); @@ -297,7 +239,12 @@ void DiagnosticLogsBDXTransferHandler::Reset() mExchangeCtx = nullptr; } - mDelegate = nullptr; + if (mDelegate != nullptr) + { + mDelegate->EndLogCollection(mLogSessionHandle); + mDelegate = nullptr; + } + mLogSessionHandle = kInvalidLogSessionHandle; mInitialized = false; mNumBytesSent = 0; } diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h index 8f7bda5edf0e60..a7e1a0ecc302e0 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h @@ -31,15 +31,15 @@ namespace app { namespace Clusters { namespace DiagnosticLogs { /** - * An implementation of the handler than initiates a BDX transfer as a Sender using the synchronous Sender Drive + * The BDX transfer handler than initiates a BDX transfer session as a Sender using the synchronous Sender Drive * transfer mode. It gets the chunks of the log from the accessory and sends the block accross to the receiver until * all the blocks have been transferred and the accessory reports that end of file is reached. */ class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator { public: - DiagnosticLogsBDXTransferHandler() {} - ~DiagnosticLogsBDXTransferHandler() {} + DiagnosticLogsBDXTransferHandler() {}; + ~DiagnosticLogsBDXTransferHandler() {}; CHIP_ERROR Init(); @@ -52,6 +52,9 @@ class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator void Reset(); private: + + void SendNextBlock(MutableByteSpan & buffer); + chip::Optional mFabricIndex; chip::Optional mNodeId; @@ -66,8 +69,6 @@ class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator LogProviderDelegate * mDelegate; IntentEnum mIntent; - - char mFileDesignator[chip::bdx::kMaxFileDesignatorLen]; }; } // namespace DiagnosticLogs diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index 4bb75a59d1f14f..b19c96b2525c06 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -1,6 +1,6 @@ /** * - * Copyright (c) 2021 Project CHIP Authors + * Copyright (c) 2023 Project CHIP Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,7 +37,7 @@ using chip::Protocols::InteractionModel::Status; static constexpr size_t kDiagnosticLogsLogProviderDelegateTableSize = EMBER_AF_DIAGNOSTIC_LOGS_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; static_assert(kDiagnosticLogsLogProviderDelegateTableSize < kEmberInvalidEndpointIndex, - "DiagnosticLogs LogProviderDelegate table size error"); + "DiagnosticLogs: log provider delegate table size error"); namespace chip { namespace app { @@ -57,7 +57,7 @@ bool isLogProviderDelegateNull(LogProviderDelegate * logProviderDelegate, Endpoi { if (logProviderDelegate == nullptr) { - ChipLogProgress(Zcl, "Diagnostic logs has no LogProviderDelegate set for endpoint:%u", endpoint); + ChipLogProgress(Zcl, "Diagnosticlogs: no log provider delegate set for endpoint:%u", endpoint); return true; } return false; @@ -74,7 +74,6 @@ void DiagnosticLogsServer::SetDefaultLogProviderDelegate(EndpointId endpoint, Lo { uint16_t ep = emberAfGetClusterServerEndpointIndex(endpoint, DiagnosticLogs::Id, EMBER_AF_DIAGNOSTIC_LOGS_CLUSTER_SERVER_ENDPOINT_COUNT); - // if endpoint is found if (ep < kDiagnosticLogsLogProviderDelegateTableSize) { gLogProviderDelegateTable[ep] = logProviderDelegate; @@ -101,23 +100,6 @@ bool DiagnosticLogsServer::HasValidFileDesignator(CharSpan transferFileDesignato CHIP_ERROR DiagnosticLogsServer::HandleLogRequestForBDXProtocol(Messaging::ExchangeContext * exchangeCtx, EndpointId endpointId, IntentEnum intent, CharSpan fileDesignator) { - // if bdx is not supported and logs fit return in log content. if node doesn't support bdx have a compile time guard for the BDX - // code a) Create a handler for BDX. we need to get the exchange mgr from the command handler and create a new exchange context - // with the LogProviderDelegate pointing to the handler for BDX and send Init. b) when we get sendaccept send a response (how to - // send a delayed response save the command object). hold on to the commandhandler/obj and call the setstatus/addrepsonse. no - // send accept -> return in log content - how muchever fits send it. If a failure StatusReport is received in response to the - // SendInit message, the Node SHALL send a RetrieveLogsResponse command with a Status of Denied. where the Node is able to fit - // the entirety of the requested logs within the LogContent field, the Status field of the RetrieveLogsResponse SHALL be set to - // Exhausted - - // how to send a delayed response save the command object) - Handle object in CommandHandler. use that. set i am going to - // respond to you and continue. b) register with/ask the accessory for the blocks, specify the size of the blocks based on BDX - // negotiated block size - can accessory not support the block size? report an error -> send no logs in completion where do we - // define this api? this needs to be supported by accessories like doorlock etc. Seems like we would need a - // diagnosticlogsLogProviderDelegate here. c) When block is received, build up the block with incremented block number and send - // it across. d) request the next block once we receive a block ack. e) Send the next block until accessory says its sending the - // last block f) Send block eof g) Wait for block EOF ack and tear down the BDX stuff. h) any errors occur or bdx timeouts -> - // tear down the BDX stuff. VerifyOrReturnError(exchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -128,8 +110,9 @@ CHIP_ERROR DiagnosticLogsServer::HandleLogRequestForBDXProtocol(Messaging::Excha LogProviderDelegate * logProviderDelegate = DiagnosticLogs::GetLogProviderDelegate(endpointId); VerifyOrReturnError(!(DiagnosticLogs::isLogProviderDelegateNull(logProviderDelegate, endpointId)), CHIP_ERROR_INCORRECT_STATE); - - CHIP_ERROR error = mDiagnosticLogsBDXTransferHandler.InitializeTransfer( + + mDiagnosticLogsBDXTransferHandler = new DiagnosticLogsBDXTransferHandler(); + CHIP_ERROR error = mDiagnosticLogsBDXTransferHandler->InitializeTransfer( exchangeCtx, scopedPeerNodeId.GetFabricIndex(), scopedPeerNodeId.GetNodeId(), logProviderDelegate, intent, fileDesignator); return error; } @@ -143,7 +126,7 @@ void DiagnosticLogsServer::HandleBDXResponse(CHIP_ERROR error) if (commandHandle == nullptr) { - ChipLogError(Controller, "DiagnosticLogsServer: Unable to handle BDX response. commandHandler is null"); + ChipLogError(Zcl, "HandleBDXResponse - commandHandler is null"); return; } @@ -155,10 +138,7 @@ void DiagnosticLogsServer::HandleBDXResponse(CHIP_ERROR error) } else { - // Fallback on the response payload log request - HandleLogRequestForResponsePayload(commandHandle, mRequestPath, mIntent); - mDiagnosticLogsBDXTransferHandler.Reset(); - mAsyncCommandHandle.Release(); + SendErrorResponseAndReset(commandHandle, mRequestPath, StatusEnum::kNoLogs); } } @@ -168,6 +148,18 @@ void DiagnosticLogsServer::SetAsyncCommandHandleAndPath(CommandHandler * command mRequestPath = commandPath; } +void DiagnosticLogsServer::SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, StatusEnum status) +{ + Commands::RetrieveLogsResponse::Type response; + if (commandHandler != nullptr) + { + response.status = status; + commandHandler->AddResponse(path, response); + } + //mDiagnosticLogsBDXTransferHandler->Reset(); + //delete(mDiagnosticLogsBDXTransferHandler); +} + #endif void DiagnosticLogsServer::HandleLogRequestForResponsePayload(CommandHandler * commandHandler, ConcreteCommandPath path, @@ -190,13 +182,20 @@ void DiagnosticLogsServer::HandleLogRequestForResponsePayload(CommandHandler * c if (!buffer.Alloc(kLogContentMaxSize)) { - ChipLogError(BDX, "buffer not allocated"); + ChipLogError(Zcl, "buffer not allocated"); response.status = StatusEnum::kNoLogs; commandHandler->AddResponse(path, response); return; } mLogSessionHandle = logProviderDelegate->StartLogCollection(intent); + + if (mLogSessionHandle == kInvalidLogSessionHandle) + { + response.status = StatusEnum::kNoLogs; + commandHandler->AddResponse(path, response); + return; + } MutableByteSpan mutableBuffer; @@ -252,12 +251,12 @@ static void HandleRetrieveLogRequest(CommandHandler * commandObj, const Concrete if (!transferFileDesignator.HasValue() || !DiagnosticLogsServer::Instance().HasValidFileDesignator(transferFileDesignator.Value())) { + ChipLogError(Zcl, "HandleRetrieveLogRequest - fileDesignator not valid for BDX protocol"); response.status = StatusEnum::kNoLogs; commandObj->AddResponse(commandPath, response); return; } - // if log fits just send it. if (DiagnosticLogsServer::Instance().IsBDXProtocolRequested(protocol)) { CHIP_ERROR err = DiagnosticLogsServer::Instance().HandleLogRequestForBDXProtocol( @@ -289,7 +288,4 @@ bool emberAfDiagnosticLogsClusterRetrieveLogsRequestCallback(CommandHandler * co return true; } -void MatterDiagnosticLogsPluginServerInitCallback() -{ - // Nothing to do, the server init routine will be done in Instance::Init() -} +void MatterDiagnosticLogsPluginServerInitCallback() {} diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h index ca56f3da9927c2..aab63ec3343357 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2021 Project CHIP Authors + * Copyright (c) 2023 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -70,13 +70,15 @@ class DiagnosticLogsServer bool HasValidFileDesignator(chip::CharSpan transferFileDesignator); bool IsBDXProtocolRequested(TransferProtocolEnum requestedProtocol); + + void SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, StatusEnum status); #endif private: #if CHIP_CONFIG_ENABLE_BDX_LOG_TRANSFER - DiagnosticLogsBDXTransferHandler mDiagnosticLogsBDXTransferHandler; + DiagnosticLogsBDXTransferHandler * mDiagnosticLogsBDXTransferHandler; #endif @@ -84,10 +86,8 @@ class DiagnosticLogsServer chip::app::CommandHandler::Handle mAsyncCommandHandle; chip::app::ConcreteCommandPath mRequestPath = chip::app::ConcreteCommandPath(0, 0, 0); - ; IntentEnum mIntent; - // Instance static DiagnosticLogsServer sInstance; }; diff --git a/src/darwin/Framework/CHIP/MTRDevice.h b/src/darwin/Framework/CHIP/MTRDevice.h index 811d4bcd8660b8..c5b1ab0f418414 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.h +++ b/src/darwin/Framework/CHIP/MTRDevice.h @@ -215,8 +215,8 @@ typedef NS_ENUM(NSInteger, MTRDiagnosticLogType) { /** * Download log of the desired type from the device. * - * Note: The consumer of this API should move the file that the NSURL in the MTRDiagnosticLogResult points to - * or open it for reading before the completion handler returns. Otherwise, the file will be deleted, and the data will be lost. + * Note: The consumer of this API should move the file that the logResult points to or open it for reading before the + * completion handler returns. Otherwise, the file will be deleted, and the data will be lost. * * @param type The type of log being requested. This should correspond to a value in the enum MTRDiagnosticLogType. * @param timeout The timeout for getting the log. If the timeout expires, completion will be called with whatever @@ -224,7 +224,8 @@ typedef NS_ENUM(NSInteger, MTRDiagnosticLogType) { * 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 pass in the URL for the requested log. + * @param completion The completion that will be called to return the URL of the requested log if successful. Otherwise + * returns an error. */ - (void)downloadLogOfType:(MTRDiagnosticLogType)type timeout:(NSTimeInterval)timeout diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 20109083aeb6d6..bd0767f6d8b7c8 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -150,6 +150,7 @@ @interface MTRDevice () @property (nonatomic) MTRWeakReference> * weakDelegate; @property (nonatomic) dispatch_queue_t delegateQueue; @property (nonatomic) dispatch_source_t timerSource; +@property (nonatomic) MTRDiagnosticLogsTransferHandler * diagnosticLogsTransferHandler; @property (nonatomic) NSArray *> * unreportedEvents; /** @@ -1238,13 +1239,13 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator [baseDevice openCommissioningWindowWithDiscriminator:discriminator duration:duration queue:queue completion:completion]; } -- (NSString *)toLogTypeString:(MTRDiagnosticLogType)type +- (NSString *)_toLogTypeString:(MTRDiagnosticLogType)type { switch (type) { case MTRDiagnosticLogTypeEndUserSupport: return @"EndUserSupport"; case MTRDiagnosticLogTypeNetworkDiagnostics: - return @"NetworkDiagnostics"; + return @"NetworkDiag"; case MTRDiagnosticLogTypeCrash: return @"Crash"; default: @@ -1255,32 +1256,28 @@ - (NSString *)toLogTypeString:(MTRDiagnosticLogType)type - (NSString *)_getFileDesignatorForLogType:(MTRDiagnosticLogType)type { - NSString * fileDesignator = [NSString stringWithFormat:@"%@%@/%@", @"bdx:/", self.nodeID, [self toLogTypeString:type]]; + NSString * fileDesignator = [NSString stringWithFormat:@"%@%@/%@", @"bdx:/", self.nodeID, [self _toLogTypeString:type]]; return fileDesignator; } - (void)_startTimerForDownload:(NSTimeInterval)timeout - queue:(dispatch_queue_t)queue - completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion { - if (timeout > 0) { - _timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, DISPATCH_TIMER_STRICT, self.queue); - VerifyOrDie(_timerSource != nullptr); + _timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, DISPATCH_TIMER_STRICT, self.queue); + VerifyOrDie(_timerSource != nullptr); - dispatch_source_set_timer( - _timerSource, dispatch_walltime(nullptr, static_cast(timeout * NSEC_PER_MSEC)), DISPATCH_TIME_FOREVER, 2 * NSEC_PER_MSEC); + dispatch_source_set_timer( + _timerSource, dispatch_walltime(nullptr, static_cast(timeout * NSEC_PER_MSEC)), DISPATCH_TIME_FOREVER, 2 * NSEC_PER_MSEC); - dispatch_source_set_event_handler(_timerSource, ^{ - dispatch_source_cancel(self->_timerSource); - - dispatch_async(queue, ^{ - NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeTimeout userInfo:nil]; - completion(nil, error); - }); - return; + dispatch_source_set_event_handler(_timerSource, ^{ + dispatch_async(self.queue, ^{ + if (self->_diagnosticLogsTransferHandler != nil) + { + self->_diagnosticLogsTransferHandler->AbortTransfer( chip::bdx::StatusCode::kUnknown); + } }); - dispatch_resume(_timerSource); - } + dispatch_source_cancel(self->_timerSource); + }); + dispatch_resume(_timerSource); } - (NSURL * _Nullable)_temporaryFileURLForDownload:(MTRDiagnosticLogType)type @@ -1292,7 +1289,7 @@ - (NSURL * _Nullable)_temporaryFileURLForDownload:(MTRDiagnosticLogType)type NSString * timeString = [dateFormatter stringFromDate:NSDate.now]; - NSString * fileName = [NSString stringWithFormat:@"%@_%@_%@", timeString, self.nodeID, [self toLogTypeString:type]]; + NSString * fileName = [NSString stringWithFormat:@"%@_%@_%@", timeString, self.nodeID, [self _toLogTypeString:type]]; NSURL * filePath = [NSURL fileURLWithPath:[NSTemporaryDirectory() stringByAppendingPathComponent:fileName] isDirectory:YES]; NSError * error = nil; @@ -1318,131 +1315,105 @@ - (bool)_isErrorResponse:(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _ return (response == nil || (response.status != nil && [response.status intValue] != 0 && [response.status intValue] != 1) || response.logContent.length == 0); } -// Once the consumer calls this API- -// a) if types are not valid, return an error. no logs -// b) start a timer with the timeout if provided. - -// do the following until all requested log types are retrieved -// c) For each type requested in the bitmap, send a command to the diagnostic logs cluster with protocol BDX/intent type - log type -// d) when timeout expires, call the completion with what has been received in filepath array so far. set the timeout expired bool in the response -// e) if one file type was requested, call completion and send the filepath otherwise save the filepath in an array. -// f) if an error occurs or BDX times out and there are no more pending log files, call the completion with what has been received in filepath array so far. -// g) if more than one file types was requested, keep track of pending request and repeat c-> f -// h) if an error occurs or BDX times out and there are more pending requests, move on to the next log type. -// h) when all requested logs have been retreived successfully or an error/timeout occured send the array of filepaths +-(void)_invokeCompletion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion + filepath:(NSURL * _Nullable)filepath + queue:(dispatch_queue_t)queue + error:(NSError * _Nullable)error +{ + if (self->_diagnosticLogsTransferHandler != nil) + { + delete(self->_diagnosticLogsTransferHandler); + } + dispatch_async(queue, ^{ + completion(filepath, error); + }); +} + +-(void)_invokeCompletionWithError:(void (^)(NSURL * _Nullable logResult, NSError * error))completion + queue:(dispatch_queue_t)queue + error:(NSError * _Nullable)error +{ + [self _invokeCompletion:completion filepath:nil queue:queue error:error]; +} + + - (void)_downloadLogOfType:(MTRDiagnosticLogType)type timeout:(NSTimeInterval)timeout queue:(dispatch_queue_t)queue completion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion { - ChipLogError(Controller, "downloadLogOfType %ld", type); if (type != MTRDiagnosticLogTypeEndUserSupport && type != MTRDiagnosticLogTypeNetworkDiagnostics && type != MTRDiagnosticLogTypeCrash) { - dispatch_async(queue, ^{ - NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]; - completion(nil, error); - }); + [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]]; return; } - // Start a timer for the timeout and abort transfer and return the log result - [self _startTimerForDownload:timeout queue:queue completion:completion]; - MTRClusterDiagnosticLogs * cluster = [[MTRClusterDiagnosticLogs alloc] initWithDevice:self endpointID:@(0) queue:queue]; - if (cluster == nil) { - if (self->_timerSource) { - dispatch_source_cancel(self->_timerSource); - } - dispatch_async(queue, ^{ - NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]; - completion(nil, error); - }); + [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]]; return; } NSURL * filePath = [self _temporaryFileURLForDownload:type queue:queue completion:completion]; - if (filePath == nil) { - if (self->_timerSource) { - dispatch_source_cancel(self->_timerSource); - } - dispatch_async(queue, ^{ - NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]; - completion(nil, error); - }); + [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]]; } - __block MTRDiagnosticLogsTransferHandler * diagnosticLogsTransferHandler = new MTRDiagnosticLogsTransferHandler(filePath, ^(bool result) { - if (self->_timerSource) { - dispatch_source_cancel(self->_timerSource); - } + if (self->_diagnosticLogsTransferHandler != nullptr && self->_diagnosticLogsTransferHandler->IsInBDXSession()) + { + [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRInteractionErrorDomain code:MTRInteractionErrorCodeBusy userInfo:nil]]; + } + + self->_diagnosticLogsTransferHandler = new MTRDiagnosticLogsTransferHandler(filePath, ^(bool result) { if (result == YES) { - dispatch_async(queue, ^{ - completion(filePath, nil); - }); + [self _invokeCompletion:completion filepath:filePath queue:queue error:nil]; } else { - dispatch_async(queue, ^{ - NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]; - completion(nil, error); - }); + [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]]; } - delete (diagnosticLogsTransferHandler); }); // Get the device commissionee and get the exchange manager to register for unsolicited message handler for BDX messages [self.deviceController asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) { if (commissioner == nil) { - dispatch_async(self.queue, ^{ - if (self->_timerSource) { - dispatch_source_cancel(self->_timerSource); - } - }); - dispatch_async(queue, ^{ - NSError * error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:nil]; - completion(nil, error); - }); - + [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]]; return; } - commissioner->ExchangeMgr()->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id, diagnosticLogsTransferHandler); + commissioner->ExchangeMgr()->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id, self->_diagnosticLogsTransferHandler); dispatch_async(self.queue, ^{ + // Start a timer if a timeout is provided + if (timeout > 0) + { + [self _startTimerForDownload:timeout]; + } + MTRDiagnosticLogsClusterRetrieveLogsRequestParams * requestParams = [[MTRDiagnosticLogsClusterRetrieveLogsRequestParams alloc] init]; requestParams.intent = [NSNumber numberWithInteger:type]; requestParams.requestedProtocol = [NSNumber numberWithUnsignedChar:chip::to_underlying(chip::app::Clusters::DiagnosticLogs::TransferProtocolEnum::kBdx)]; requestParams.transferFileDesignator = [self _getFileDesignatorForLogType:type]; - [cluster retrieveLogsRequestWithParams:requestParams expectedValues:nil expectedValueInterval:nil completion:^(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _Nullable response, NSError * _Nullable error) { // If we are in a BDX session and there is no error, do nothing. Completion will be called when BDX succeeds or fails. - if (diagnosticLogsTransferHandler->IsInBDXSession() && error == nil) { + if (self->_diagnosticLogsTransferHandler->IsInBDXSession() && error == nil) { return; } - + if ([self _isErrorResponse:response]) { - NSLog(@"_downloadLogOfType error response %@", response); if (self->_timerSource) { dispatch_source_cancel(self->_timerSource); } - dispatch_async(queue, ^{ - completion(nil, error); - }); - + [self _invokeCompletionWithError:completion queue:queue error:error]; return; } - // If the response has a log content, copy it into the temporary location and send the URL otherwise we will send the full file once BDX succeeds. + // If the response has a log content, copy it into the temporary location and send the URL. if (response != nil && response.logContent != nil && response.logContent.length > 0) { if ([response.logContent writeToURL:filePath atomically:YES]) { if (self->_timerSource) { dispatch_source_cancel(self->_timerSource); } - dispatch_async(queue, ^{ - completion(filePath, nil); - }); - + [self _invokeCompletion:completion filepath:filePath queue:queue error:nil]; return; } } @@ -1453,9 +1424,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type if (self->_timerSource) { dispatch_source_cancel(self->_timerSource); } - dispatch_async(queue, ^{ - completion(nil, error); - }); + [self _invokeCompletionWithError:completion queue:queue error:error]; }]; } diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h index a13b63ef016336..c03c7c8feefa5f 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h @@ -45,13 +45,13 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { mCallback = callback; } - ~MTRDiagnosticLogsTransferHandler(); + ~MTRDiagnosticLogsTransferHandler() {}; void HandleTransferSessionOutput(chip::bdx::TransferSession::OutputEvent & event) override; + + void AbortTransfer(chip::bdx::StatusCode reason); - void Reset(); - - bool IsInBDXSession() { return mIsInBDXSession; } + bool IsInBDXSession() { return mInitialized; } protected: CHIP_ERROR OnMessageReceived(chip::Messaging::ExchangeContext * _Nonnull ec, const chip::PayloadHeader & payloadHeader, @@ -69,9 +69,8 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { CHIP_ERROR OnTransferSessionEnd(chip::bdx::TransferSession::OutputEvent & event); CHIP_ERROR OnBlockReceived(chip::bdx::TransferSession::OutputEvent & event); - - // CHIP_ERROR OnUnsolicitedMessageReceived( - // const chip::PayloadHeader & payloadHeader, chip::Messaging::ExchangeDelegate * _Nonnull & newDelegate) override; + + void Reset(); // The fabric index of the node with which the BDX session is established. chip::Optional mFabricIndex; @@ -86,8 +85,9 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { NSFileHandle * _Nullable mFileHandle; std::function mCallback; - bool mIsInBDXSession = false; + bool mInitialized = false; + + bool isBlockEOFSent = false; uint64_t downloadedBytes = 0; - uint64_t totalFileBytes = 0; }; diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index 9d7691f1664572..13238f1d56d5fb 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -26,21 +26,16 @@ using namespace chip::bdx; using namespace chip::app; -// TODO Expose a method onto the delegate to make that configurable. constexpr uint32_t kMaxBdxBlockSize = 1024; // Timeout for the BDX transfer session. The OTA Spec mandates this should be >= 5 minutes. constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); constexpr bdx::TransferRole kBdxRole = bdx::TransferRole::kReceiver; -// Need DiagnosticLogsProcessorInterface handle to process block which will be implemented in darwin/linux - -// TODO: need to check how to handle MTDeviceController being shutdown. - CHIP_ERROR MTRDiagnosticLogsTransferHandler::PrepareForTransfer(System::Layer * _Nonnull layer, FabricIndex fabricIndex, NodeId nodeId) { assertChipStackLockedByCurrentThread(); - + ReturnErrorOnFailure(ConfigureState(fabricIndex, nodeId)); BitFlags flags(bdx::TransferControlFlags::kReceiverDrive); @@ -48,10 +43,6 @@ return Responder::PrepareForTransfer(layer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout); } -MTRDiagnosticLogsTransferHandler::~MTRDiagnosticLogsTransferHandler() -{ -} - bdx::StatusCode GetBdxStatusCodeFromChipError(CHIP_ERROR err) { if (err == CHIP_ERROR_INCORRECT_STATE) { @@ -75,8 +66,9 @@ if (mFileHandle == nil || error != nil) { // TODO: Map NSError to BDX error - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); - return CHIP_ERROR_INCORRECT_STATE; + CHIP_ERROR error = CHIP_ERROR_INCORRECT_STATE; + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(error))); + return error; } TransferSession::TransferAcceptData acceptData; @@ -86,7 +78,7 @@ acceptData.Length = mTransfer.GetTransferLength(); mTransfer.AcceptTransfer(acceptData); - mIsInBDXSession = true; + mInitialized = true; return CHIP_NO_ERROR; } @@ -100,25 +92,16 @@ CHIP_ERROR error = CHIP_NO_ERROR; if (event.EventType == TransferSession::OutputEventType::kTransferTimeout) { error = CHIP_ERROR_TIMEOUT; - } else if (event.EventType != TransferSession::OutputEventType::kBlockReceived || !event.blockdata.IsEof) { + } else if (!isBlockEOFSent) { error = CHIP_ERROR_INTERNAL; } - - if (error != CHIP_NO_ERROR) { - // Notify the MTRDevice via the callback that the BDX transfer has completed with error - if (mCallback) { - mCallback(NO); - } - Reset(); - return error; - } - - // Notify the MTRDevice via the callback that the BDX transfer has completed with success - if (mCallback) { - mCallback(YES); + // Notify the MTRDevice via the callback that the BDX transfer has completed with error or success. + if (mCallback) + { + mCallback(error != CHIP_NO_ERROR ? NO : YES); } Reset(); - return CHIP_NO_ERROR; + return error; } CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnBlockReceived(TransferSession::OutputEvent & event) @@ -136,27 +119,20 @@ [mFileHandle writeData:AsData(blockData) error:&error]; if (error != nil) { - // TODO: map nserror to status code - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); - return CHIP_ERROR_INCORRECT_STATE; + // TODO: Map NSError to BDX error + return mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE)); } } else { - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); - return CHIP_ERROR_INCORRECT_STATE; + return mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE)); } CHIP_ERROR err = mTransfer.PrepareBlockAck(); LogErrorOnFailure(err); if (err != CHIP_NO_ERROR) { - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE))); - return err; + return mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)); } if (event.blockdata.IsEof) { - if (mFileHandle != nil) { - [mFileHandle closeFile]; - mFileHandle = nullptr; - } - OnTransferSessionEnd(event); + mFileHandle = nil; } return CHIP_NO_ERROR; @@ -166,8 +142,7 @@ { assertChipStackLockedByCurrentThread(); - VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); - // VerifyOrReturnError(mDelegate != nil, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mExchangeCtx != nil, CHIP_ERROR_INCORRECT_STATE); Messaging::SendFlags sendFlags; @@ -178,19 +153,17 @@ } auto & msgTypeData = event.msgTypeData; - // If there's an error sending the message, close the exchange and call ResetState. - // TODO: If we can remove the !mInitialized check in ResetState(), just calling ResetState() will suffice here. + // If there's an error sending the message, close the exchange and call Reset. CHIP_ERROR err = mExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags); if (err != CHIP_NO_ERROR) { mExchangeCtx->Close(); - mExchangeCtx = nullptr; + mExchangeCtx = nil; Reset(); } else if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport)) { // If the send was successful for a status report, since we are not expecting a response the exchange context is // already closed. We need to null out the reference to avoid having a dangling pointer. - mExchangeCtx = nullptr; - Reset(); + mExchangeCtx = nil; } return err; } @@ -198,19 +171,18 @@ void MTRDiagnosticLogsTransferHandler::HandleTransferSessionOutput(TransferSession::OutputEvent & event) { assertChipStackLockedByCurrentThread(); - ChipLogError(BDX, "Got an event %s", event.ToString(event.EventType)); + ChipLogError(BDX, "HandleTransferSessionOutput: Got an event %s", event.ToString(event.EventType)); CHIP_ERROR err = CHIP_NO_ERROR; switch (event.EventType) { case TransferSession::OutputEventType::kInitReceived: - ChipLogError(BDX, "HandleTransferSessionOutput called"); err = OnTransferSessionBegin(event); if (err != CHIP_NO_ERROR) { LogErrorOnFailure(err); - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err))); + mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)); } break; case TransferSession::OutputEventType::kStatusReceived: - ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); + ChipLogError(BDX, "HandleTransferSessionOutput: Got StatusReport %x", static_cast(event.statusData.statusCode)); [[fallthrough]]; case TransferSession::OutputEventType::kInternalError: case TransferSession::OutputEventType::kTransferTimeout: @@ -223,12 +195,26 @@ mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)); } break; + case TransferSession::OutputEventType::kAckEOFReceived: + // Need to call OnTransferSessionEnd(event). Need to fix this and remove isBlockEOFSent. + break; case TransferSession::OutputEventType::kMsgToSend: - err = OnMessageToSend(event); + err = OnMessageToSend(event); + if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) + { + // TODO: This is a hack for determinin that the Ack EOF is sent before cleaning up. + // Need to fix this. + isBlockEOFSent = true; + } break; + case TransferSession::OutputEventType::kNone: + if (isBlockEOFSent) + { + OnTransferSessionEnd(event); + } + break; case TransferSession::OutputEventType::kQueryWithSkipReceived: case TransferSession::OutputEventType::kQueryReceived: - case TransferSession::OutputEventType::kNone: case TransferSession::OutputEventType::kAckReceived: case TransferSession::OutputEventType::kAcceptReceived: // Nothing to do. @@ -240,24 +226,25 @@ } } +void MTRDiagnosticLogsTransferHandler::AbortTransfer(chip::bdx::StatusCode reason) +{ + mTransfer.AbortTransfer(reason); +} + void MTRDiagnosticLogsTransferHandler::Reset() { - mIsInBDXSession = false; - mFileURL = nullptr; - mCallback = nullptr; + assertChipStackLockedByCurrentThread(); + mInitialized = false; + mFileURL = nil; + mFileHandle = nil; - if (mFileHandle != nil) { - [mFileHandle closeFile]; - mFileHandle = nullptr; - } + Responder::ResetTransfer(); if (mExchangeCtx) { mExchangeCtx->Close(); - mExchangeCtx = nullptr; + mExchangeCtx = nil; } mFabricIndex.ClearValue(); mNodeId.ClearValue(); - - mTransfer.Reset(); } CHIP_ERROR MTRDiagnosticLogsTransferHandler::ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId) @@ -273,6 +260,7 @@ CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnMessageReceived( Messaging::ExchangeContext * _Nonnull ec, const PayloadHeader & payloadHeader, System::PacketBufferHandle && payload) { + VerifyOrReturnError(ec != nil, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR err; mExchangeCtx = ec; @@ -284,7 +272,7 @@ if (nodeId != kUndefinedNodeId && fabricIndex != kUndefinedFabricIndex) { err = PrepareForTransfer(&(DeviceLayer::SystemLayer()), fabricIndex, nodeId); if (err != CHIP_NO_ERROR) { - ChipLogError(Controller, "Failed to prepare for transfer for BDX"); + ChipLogError(BDX, "Failed to prepare for transfer for BDX"); return err; } } diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 2d261a46168c26..ffc3cb5a97af77 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -38,7 +38,7 @@ #import static const uint16_t kPairingTimeoutInSeconds = 10; -static const uint16_t kDownloadLogTimeoutInSeconds = 1000; +static const uint16_t kDownloadLogTimeoutInSeconds = 70; static const uint16_t kTimeoutInSeconds = 3; static const uint64_t kDeviceId = 0x12344321; static NSString * kOnboardingPayload = @"MT:-24J0AFN00KA0648G00"; @@ -276,7 +276,7 @@ - (void)test000_SetUp // just exists to make the setup not look like it's happening inside other // tests. } -/* + - (void)test001_ReadAttribute { XCTestExpectation * expectation = @@ -2696,13 +2696,13 @@ - (void)test900_SubscribeAllAttributes // Wait for report [self waitForExpectations:[NSArray arrayWithObject:reportExpectation] timeout:kTimeoutInSeconds]; -}*/ +} -- (void)test901_DownloadLogTypeEndUserSupport +- (void)test901_DownloadEndUserSupportLog_NoTimeout { MTRDeviceController * controller = sController; XCTAssertNotNil(controller); - XCTestExpectation * expectation = [self expectationWithDescription:@"Log Transfer Complete"]; + XCTestExpectation * expectation = [self expectationWithDescription:@"End User Support Log Transfer Complete"]; dispatch_queue_t queue = dispatch_get_main_queue(); @@ -2716,6 +2716,59 @@ - (void)test901_DownloadLogTypeEndUserSupport NSError * attributesError = nil; NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + size_t fileSize = [fileAttributes fileSize]; + XCTAssertTrue(fileSize > 0); + NSLog(@"test901_DownloadEndUserSupportLog_NoTimeout expectation fulfill"); + [expectation fulfill]; + }]; + }); + [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; +} + +- (void)test902_DownloadNetworkDiagnosticsLog_NoTimeout +{ + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + XCTestExpectation * expectation = [self expectationWithDescription:@"Network Diagnostics Log Transfer Complete"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + + dispatch_async(queue, ^{ + MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; + XCTAssertNotNil(device); + [device downloadLogOfType:MTRDiagnosticLogTypeNetworkDiagnostics timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { + XCTAssertNil(error); + XCTAssertNotNil(logResult); + + NSError * attributesError = nil; + NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + + size_t fileSize = [fileAttributes fileSize]; + XCTAssertTrue(fileSize > 0); + [expectation fulfill]; + }]; + }); + [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; +} + +- (void)test903_DownloadCrashLog_NoTimeout +{ + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + XCTestExpectation * expectation = [self expectationWithDescription:@"Crash Log Transfer Complete"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + + dispatch_async(queue, ^{ + MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; + XCTAssertNotNil(device); + [device downloadLogOfType:MTRDiagnosticLogTypeCrash timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { + XCTAssertNil(error); + XCTAssertNotNil(logResult); + + NSError * attributesError = nil; + NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + size_t fileSize = [fileAttributes fileSize]; XCTAssertTrue(fileSize > 0); [expectation fulfill]; diff --git a/src/protocols/bdx/BdxTransferSession.cpp b/src/protocols/bdx/BdxTransferSession.cpp index 515c7ed2dd92fe..b506048e28c21d 100644 --- a/src/protocols/bdx/BdxTransferSession.cpp +++ b/src/protocols/bdx/BdxTransferSession.cpp @@ -391,7 +391,7 @@ CHIP_ERROR TransferSession::PrepareBlockAck() mState = TransferState::kTransferDone; mAwaitingResponse = false; } - + ChipLogError(BDX, "sending block ack %hhu", msgType); PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); return CHIP_NO_ERROR; diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp index fbe563187b6fb9..4d573038b8a683 100644 --- a/src/protocols/bdx/TransferFacilitator.cpp +++ b/src/protocols/bdx/TransferFacilitator.cpp @@ -68,7 +68,11 @@ void TransferFacilitator::OnResponseTimeout(Messaging::ExchangeContext * ec) void TransferFacilitator::PollTimerHandler(chip::System::Layer * systemLayer, void * appState) { VerifyOrReturn(appState != nullptr); - static_cast(appState)->PollForOutput(); + VerifyOrReturn(systemLayer != nullptr); + if (appState != nullptr) + { + static_cast(appState)->PollForOutput(); + } } void TransferFacilitator::PollForOutput() @@ -114,6 +118,7 @@ CHIP_ERROR Responder::PrepareForTransfer(System::Layer * layer, TransferRole rol void Responder::ResetTransfer() { mTransfer.Reset(); + mSystemLayer = nullptr; ChipLogProgress(BDX, "Stop polling for messages"); mStopPolling = true; } @@ -135,6 +140,7 @@ CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, void Initiator::ResetTransfer() { mTransfer.Reset(); + mSystemLayer = nullptr; ChipLogProgress(BDX, "Stop polling for messages"); mStopPolling = true; } From 0349632b9e73fad3a5416c3293e0478054ee82db Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 31 Oct 2023 06:10:00 +0000 Subject: [PATCH 03/16] Restyled by whitespace --- examples/all-clusters-app/linux/main-common.cpp | 2 +- src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp | 2 +- src/app/bdx/DiagnosticLogsBDXTransferHandler.h | 4 ++-- .../diagnostic-logs-server/diagnostic-logs-server.cpp | 4 ++-- .../diagnostic-logs-server/diagnostic-logs-server.h | 2 +- src/darwin/Framework/CHIP/MTRDevice.mm | 4 ++-- .../Framework/CHIP/MTRDiagnosticLogsTransferHandler.h | 6 +++--- .../Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm | 4 ++-- src/protocols/bdx/TransferFacilitator.cpp | 2 +- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index 759284b2f76a92..3b965823119b60 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -278,5 +278,5 @@ void emberAfDiagnosticLogsClusterInitCallback(chip::EndpointId endpoint) LogProvider::getLogProvider().SetEndUserSupportLogFileDesignator(AppOptions::GetEndUserSupportLogFileDesignator()); LogProvider::getLogProvider().SetNetworkDiagnosticsLogFileDesignator(AppOptions::GetNetworkDiagnosticsLogFileDesignator()); LogProvider::getLogProvider().SetCrashLogFileDesignator(AppOptions::GetCrashLogFileDesignator()); - + } diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp index 1994bd9c14f2a0..ba82e957260251 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp @@ -64,7 +64,7 @@ CHIP_ERROR DiagnosticLogsBDXTransferHandler::InitializeTransfer(chip::Messaging: mExchangeCtx = exchangeCtx->GetExchangeMgr()->NewContext(exchangeCtx->GetSessionHandle(), this); VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY); - + mIntent = intent; mDelegate = delegate; mFabricIndex.SetValue(fabricIndex); diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h index a7e1a0ecc302e0..09e9ed5f72b08b 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h @@ -52,9 +52,9 @@ class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator void Reset(); private: - + void SendNextBlock(MutableByteSpan & buffer); - + chip::Optional mFabricIndex; chip::Optional mNodeId; diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index b19c96b2525c06..e363d3215b3376 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -110,7 +110,7 @@ CHIP_ERROR DiagnosticLogsServer::HandleLogRequestForBDXProtocol(Messaging::Excha LogProviderDelegate * logProviderDelegate = DiagnosticLogs::GetLogProviderDelegate(endpointId); VerifyOrReturnError(!(DiagnosticLogs::isLogProviderDelegateNull(logProviderDelegate, endpointId)), CHIP_ERROR_INCORRECT_STATE); - + mDiagnosticLogsBDXTransferHandler = new DiagnosticLogsBDXTransferHandler(); CHIP_ERROR error = mDiagnosticLogsBDXTransferHandler->InitializeTransfer( exchangeCtx, scopedPeerNodeId.GetFabricIndex(), scopedPeerNodeId.GetNodeId(), logProviderDelegate, intent, fileDesignator); @@ -189,7 +189,7 @@ void DiagnosticLogsServer::HandleLogRequestForResponsePayload(CommandHandler * c } mLogSessionHandle = logProviderDelegate->StartLogCollection(intent); - + if (mLogSessionHandle == kInvalidLogSessionHandle) { response.status = StatusEnum::kNoLogs; diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h index aab63ec3343357..b248dd88b34506 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h @@ -70,7 +70,7 @@ class DiagnosticLogsServer bool HasValidFileDesignator(chip::CharSpan transferFileDesignator); bool IsBDXProtocolRequested(TransferProtocolEnum requestedProtocol); - + void SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, StatusEnum status); #endif diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index d67bafd3094599..00a37878cab59d 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1469,7 +1469,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type { [self _startTimerForDownload:timeout]; } - + MTRDiagnosticLogsClusterRetrieveLogsRequestParams * requestParams = [[MTRDiagnosticLogsClusterRetrieveLogsRequestParams alloc] init]; requestParams.intent = [NSNumber numberWithInteger:type]; requestParams.requestedProtocol = [NSNumber numberWithUnsignedChar:chip::to_underlying(chip::app::Clusters::DiagnosticLogs::TransferProtocolEnum::kBdx)]; @@ -1480,7 +1480,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type if (self->_diagnosticLogsTransferHandler->IsInBDXSession() && error == nil) { return; } - + if ([self _isErrorResponse:response]) { if (self->_timerSource) { dispatch_source_cancel(self->_timerSource); diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h index c03c7c8feefa5f..6fc3366da3c503 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h @@ -48,7 +48,7 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { ~MTRDiagnosticLogsTransferHandler() {}; void HandleTransferSessionOutput(chip::bdx::TransferSession::OutputEvent & event) override; - + void AbortTransfer(chip::bdx::StatusCode reason); bool IsInBDXSession() { return mInitialized; } @@ -69,7 +69,7 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { CHIP_ERROR OnTransferSessionEnd(chip::bdx::TransferSession::OutputEvent & event); CHIP_ERROR OnBlockReceived(chip::bdx::TransferSession::OutputEvent & event); - + void Reset(); // The fabric index of the node with which the BDX session is established. @@ -86,7 +86,7 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { std::function mCallback; bool mInitialized = false; - + bool isBlockEOFSent = false; uint64_t downloadedBytes = 0; diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index 13238f1d56d5fb..77dce1c90b4bde 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -35,7 +35,7 @@ CHIP_ERROR MTRDiagnosticLogsTransferHandler::PrepareForTransfer(System::Layer * _Nonnull layer, FabricIndex fabricIndex, NodeId nodeId) { assertChipStackLockedByCurrentThread(); - + ReturnErrorOnFailure(ConfigureState(fabricIndex, nodeId)); BitFlags flags(bdx::TransferControlFlags::kReceiverDrive); @@ -199,7 +199,7 @@ // Need to call OnTransferSessionEnd(event). Need to fix this and remove isBlockEOFSent. break; case TransferSession::OutputEventType::kMsgToSend: - err = OnMessageToSend(event); + err = OnMessageToSend(event); if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { // TODO: This is a hack for determinin that the Ack EOF is sent before cleaning up. diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp index 4d573038b8a683..c9da274ed792ac 100644 --- a/src/protocols/bdx/TransferFacilitator.cpp +++ b/src/protocols/bdx/TransferFacilitator.cpp @@ -140,7 +140,7 @@ CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, void Initiator::ResetTransfer() { mTransfer.Reset(); - mSystemLayer = nullptr; + mSystemLayer = nullptr; ChipLogProgress(BDX, "Stop polling for messages"); mStopPolling = true; } From 1ce1262521f87e4ff0adfba92af271745ae5a236 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 31 Oct 2023 06:10:02 +0000 Subject: [PATCH 04/16] Restyled by clang-format --- .../diagnostic-logs-provider-delegate-impl.h | 4 +-- .../all-clusters-app/linux/AppOptions.cpp | 11 ++++--- .../all-clusters-app/linux/main-common.cpp | 1 - .../bdx/DiagnosticLogsBDXTransferHandler.cpp | 12 +++---- .../bdx/DiagnosticLogsBDXTransferHandler.h | 5 ++- .../diagnostic-logs-server.cpp | 13 ++++---- .../diagnostic-logs-server.h | 3 +- src/darwin/Framework/CHIP/MTRDevice.mm | 31 ++++++++----------- .../CHIP/MTRDiagnosticLogsTransferHandler.mm | 17 +++++----- 9 files changed, 45 insertions(+), 52 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h b/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h index 85dc68eeb79bb6..f39b5dfc595fa3 100644 --- a/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/all-clusters-app/all-clusters-common/include/diagnostic-logs-provider-delegate-impl.h @@ -52,9 +52,9 @@ class LogProvider : public LogProviderDelegate void SetCrashLogFileDesignator(const char * logFileName); - LogProvider() { }; + LogProvider(){}; - ~LogProvider() { }; + ~LogProvider(){}; static inline LogProvider & getLogProvider() { return sInstance; } diff --git a/examples/all-clusters-app/linux/AppOptions.cpp b/examples/all-clusters-app/linux/AppOptions.cpp index 734b75e2ed0e4b..ee79f7a0e1c0cd 100644 --- a/examples/all-clusters-app/linux/AppOptions.cpp +++ b/examples/all-clusters-app/linux/AppOptions.cpp @@ -18,9 +18,9 @@ #include "AppOptions.h" +#include #include #include -#include using namespace chip::ArgParser; using namespace chip::app::Clusters::DiagnosticLogs; @@ -59,7 +59,8 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id ChipLogError(BDX, "kOptionEndUserSupportFilePath setting end user fd"); if (strlen(value) > kLogFileDesignatorMaxLen) { - PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, strlen(value)); + PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, + strlen(value)); } strncpy(mEndUserSupportLogFileDesignator, value, strlen(value)); break; @@ -67,7 +68,8 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id case kOptionNetworkDiagnosticsFilePath: { if (strlen(value) > kLogFileDesignatorMaxLen) { - PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, strlen(value)); + PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, + strlen(value)); } strncpy(mNetworkDiagnosticsLogFileDesignator, value, strlen(value)); break; @@ -75,7 +77,8 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id case kOptionCrashFilePath: { if (strlen(value) > kLogFileDesignatorMaxLen) { - PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, strlen(value)); + PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, + strlen(value)); } strncpy(mCrashLogFileDesignator, value, strlen(value)); break; diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index 3b965823119b60..9a8678428d7209 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -278,5 +278,4 @@ void emberAfDiagnosticLogsClusterInitCallback(chip::EndpointId endpoint) LogProvider::getLogProvider().SetEndUserSupportLogFileDesignator(AppOptions::GetEndUserSupportLogFileDesignator()); LogProvider::getLogProvider().SetNetworkDiagnosticsLogFileDesignator(AppOptions::GetNetworkDiagnosticsLogFileDesignator()); LogProvider::getLogProvider().SetCrashLogFileDesignator(AppOptions::GetCrashLogFileDesignator()); - } diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp index ba82e957260251..dbf96eba804681 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp @@ -145,8 +145,7 @@ void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSessi break; } - case TransferSession::OutputEventType::kAcceptReceived: - { + case TransferSession::OutputEventType::kAcceptReceived: { mLogSessionHandle = mDelegate->StartLogCollection(mIntent); if (mLogSessionHandle == kInvalidLogSessionHandle) @@ -160,8 +159,7 @@ void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSessi DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_NO_ERROR); } // Fallthrough - case TransferSession::OutputEventType::kAckReceived: - { + case TransferSession::OutputEventType::kAckReceived: { uint16_t blockSize = mTransfer.GetTransferBlockSize(); uint16_t bytesToRead = blockSize; @@ -242,11 +240,11 @@ void DiagnosticLogsBDXTransferHandler::Reset() if (mDelegate != nullptr) { mDelegate->EndLogCollection(mLogSessionHandle); - mDelegate = nullptr; + mDelegate = nullptr; } mLogSessionHandle = kInvalidLogSessionHandle; - mInitialized = false; - mNumBytesSent = 0; + mInitialized = false; + mNumBytesSent = 0; } #endif diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h index 09e9ed5f72b08b..cce34f5778940f 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.h +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.h @@ -38,8 +38,8 @@ namespace DiagnosticLogs { class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator { public: - DiagnosticLogsBDXTransferHandler() {}; - ~DiagnosticLogsBDXTransferHandler() {}; + DiagnosticLogsBDXTransferHandler(){}; + ~DiagnosticLogsBDXTransferHandler(){}; CHIP_ERROR Init(); @@ -52,7 +52,6 @@ class DiagnosticLogsBDXTransferHandler : public chip::bdx::Initiator void Reset(); private: - void SendNextBlock(MutableByteSpan & buffer); chip::Optional mFabricIndex; diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index e363d3215b3376..489c1956ca4fee 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -111,8 +111,8 @@ CHIP_ERROR DiagnosticLogsServer::HandleLogRequestForBDXProtocol(Messaging::Excha VerifyOrReturnError(!(DiagnosticLogs::isLogProviderDelegateNull(logProviderDelegate, endpointId)), CHIP_ERROR_INCORRECT_STATE); - mDiagnosticLogsBDXTransferHandler = new DiagnosticLogsBDXTransferHandler(); - CHIP_ERROR error = mDiagnosticLogsBDXTransferHandler->InitializeTransfer( + mDiagnosticLogsBDXTransferHandler = new DiagnosticLogsBDXTransferHandler(); + CHIP_ERROR error = mDiagnosticLogsBDXTransferHandler->InitializeTransfer( exchangeCtx, scopedPeerNodeId.GetFabricIndex(), scopedPeerNodeId.GetNodeId(), logProviderDelegate, intent, fileDesignator); return error; } @@ -138,7 +138,7 @@ void DiagnosticLogsServer::HandleBDXResponse(CHIP_ERROR error) } else { - SendErrorResponseAndReset(commandHandle, mRequestPath, StatusEnum::kNoLogs); + SendErrorResponseAndReset(commandHandle, mRequestPath, StatusEnum::kNoLogs); } } @@ -148,7 +148,8 @@ void DiagnosticLogsServer::SetAsyncCommandHandleAndPath(CommandHandler * command mRequestPath = commandPath; } -void DiagnosticLogsServer::SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, StatusEnum status) +void DiagnosticLogsServer::SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, + chip::app::ConcreteCommandPath path, StatusEnum status) { Commands::RetrieveLogsResponse::Type response; if (commandHandler != nullptr) @@ -156,8 +157,8 @@ void DiagnosticLogsServer::SendErrorResponseAndReset(chip::app::CommandHandler * response.status = status; commandHandler->AddResponse(path, response); } - //mDiagnosticLogsBDXTransferHandler->Reset(); - //delete(mDiagnosticLogsBDXTransferHandler); + // mDiagnosticLogsBDXTransferHandler->Reset(); + // delete(mDiagnosticLogsBDXTransferHandler); } #endif diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h index b248dd88b34506..ffec67fed78bc4 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.h @@ -71,7 +71,8 @@ class DiagnosticLogsServer bool IsBDXProtocolRequested(TransferProtocolEnum requestedProtocol); - void SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, StatusEnum status); + void SendErrorResponseAndReset(chip::app::CommandHandler * commandHandler, chip::app::ConcreteCommandPath path, + StatusEnum status); #endif diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 00a37878cab59d..b9ff6d2ec4023e 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1353,9 +1353,8 @@ - (void)_startTimerForDownload:(NSTimeInterval)timeout dispatch_source_set_event_handler(_timerSource, ^{ dispatch_async(self.queue, ^{ - if (self->_diagnosticLogsTransferHandler != nil) - { - self->_diagnosticLogsTransferHandler->AbortTransfer( chip::bdx::StatusCode::kUnknown); + if (self->_diagnosticLogsTransferHandler != nil) { + self->_diagnosticLogsTransferHandler->AbortTransfer(chip::bdx::StatusCode::kUnknown); } }); dispatch_source_cancel(self->_timerSource); @@ -1398,28 +1397,26 @@ - (bool)_isErrorResponse:(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _ return (response == nil || (response.status != nil && [response.status intValue] != 0 && [response.status intValue] != 1) || response.logContent.length == 0); } --(void)_invokeCompletion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion - filepath:(NSURL * _Nullable)filepath - queue:(dispatch_queue_t)queue - error:(NSError * _Nullable)error +- (void)_invokeCompletion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion + filepath:(NSURL * _Nullable)filepath + queue:(dispatch_queue_t)queue + error:(NSError * _Nullable)error { - if (self->_diagnosticLogsTransferHandler != nil) - { - delete(self->_diagnosticLogsTransferHandler); + if (self->_diagnosticLogsTransferHandler != nil) { + delete (self->_diagnosticLogsTransferHandler); } dispatch_async(queue, ^{ completion(filepath, error); }); } --(void)_invokeCompletionWithError:(void (^)(NSURL * _Nullable logResult, NSError * error))completion - queue:(dispatch_queue_t)queue - error:(NSError * _Nullable)error +- (void)_invokeCompletionWithError:(void (^)(NSURL * _Nullable logResult, NSError * error))completion + queue:(dispatch_queue_t)queue + error:(NSError * _Nullable)error { [self _invokeCompletion:completion filepath:nil queue:queue error:error]; } - - (void)_downloadLogOfType:(MTRDiagnosticLogType)type timeout:(NSTimeInterval)timeout queue:(dispatch_queue_t)queue @@ -1441,8 +1438,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]]; } - if (self->_diagnosticLogsTransferHandler != nullptr && self->_diagnosticLogsTransferHandler->IsInBDXSession()) - { + if (self->_diagnosticLogsTransferHandler != nullptr && self->_diagnosticLogsTransferHandler->IsInBDXSession()) { [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRInteractionErrorDomain code:MTRInteractionErrorCodeBusy userInfo:nil]]; } @@ -1465,8 +1461,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type dispatch_async(self.queue, ^{ // Start a timer if a timeout is provided - if (timeout > 0) - { + if (timeout > 0) { [self _startTimerForDownload:timeout]; } diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index 77dce1c90b4bde..c3292b45c89b98 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -96,8 +96,7 @@ error = CHIP_ERROR_INTERNAL; } // Notify the MTRDevice via the callback that the BDX transfer has completed with error or success. - if (mCallback) - { + if (mCallback) { mCallback(error != CHIP_NO_ERROR ? NO : YES); } Reset(); @@ -196,23 +195,21 @@ } break; case TransferSession::OutputEventType::kAckEOFReceived: - // Need to call OnTransferSessionEnd(event). Need to fix this and remove isBlockEOFSent. - break; + // Need to call OnTransferSessionEnd(event). Need to fix this and remove isBlockEOFSent. + break; case TransferSession::OutputEventType::kMsgToSend: err = OnMessageToSend(event); - if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) - { + if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { // TODO: This is a hack for determinin that the Ack EOF is sent before cleaning up. // Need to fix this. isBlockEOFSent = true; } break; case TransferSession::OutputEventType::kNone: - if (isBlockEOFSent) - { + if (isBlockEOFSent) { OnTransferSessionEnd(event); - } - break; + } + break; case TransferSession::OutputEventType::kQueryWithSkipReceived: case TransferSession::OutputEventType::kQueryReceived: case TransferSession::OutputEventType::kAckReceived: From 426d697a98961bb01ff659750ac8e0c3516f8114 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Wed, 1 Nov 2023 14:22:02 -0700 Subject: [PATCH 05/16] Revert changes to BDXTransferSession - More clean up --- .../bdx/DiagnosticLogsBDXTransferHandler.cpp | 2 +- .../CHIP/MTRDiagnosticLogsTransferHandler.h | 4 -- .../CHIP/MTRDiagnosticLogsTransferHandler.mm | 23 ++++------ src/protocols/bdx/BdxTransferSession.cpp | 46 ++++++------------- 4 files changed, 23 insertions(+), 52 deletions(-) diff --git a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp index dbf96eba804681..3bcb9656c10de3 100644 --- a/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp +++ b/src/app/bdx/DiagnosticLogsBDXTransferHandler.cpp @@ -157,8 +157,8 @@ void DiagnosticLogsBDXTransferHandler::HandleTransferSessionOutput(TransferSessi } // Send a response to the RetreiveLogRequest since we got a SendAccept message. DiagnosticLogsServer::Instance().HandleBDXResponse(CHIP_NO_ERROR); + [[fallthrough]]; } - // Fallthrough case TransferSession::OutputEventType::kAckReceived: { uint16_t blockSize = mTransfer.GetTransferBlockSize(); uint16_t bytesToRead = blockSize; diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h index 6fc3366da3c503..d6c2f0ebe76806 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h @@ -86,8 +86,4 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { std::function mCallback; bool mInitialized = false; - - bool isBlockEOFSent = false; - - uint64_t downloadedBytes = 0; }; diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index c3292b45c89b98..8dc72575e69d10 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -38,7 +38,7 @@ ReturnErrorOnFailure(ConfigureState(fabricIndex, nodeId)); - BitFlags flags(bdx::TransferControlFlags::kReceiverDrive); + BitFlags flags(bdx::TransferControlFlags::kSenderDrive); return Responder::PrepareForTransfer(layer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout); } @@ -85,16 +85,16 @@ CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnTransferSessionEnd(TransferSession::OutputEvent & event) { assertChipStackLockedByCurrentThread(); - VerifyOrReturnError(mFabricIndex.HasValue(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mNodeId.HasValue(), CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR error = CHIP_NO_ERROR; + if (event.EventType == TransferSession::OutputEventType::kTransferTimeout) { error = CHIP_ERROR_TIMEOUT; - } else if (!isBlockEOFSent) { + } else if (event.EventType != TransferSession::OutputEventType::kMsgToSend || !event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { error = CHIP_ERROR_INTERNAL; } + // Notify the MTRDevice via the callback that the BDX transfer has completed with error or success. if (mCallback) { mCallback(error != CHIP_NO_ERROR ? NO : YES); @@ -170,7 +170,8 @@ void MTRDiagnosticLogsTransferHandler::HandleTransferSessionOutput(TransferSession::OutputEvent & event) { assertChipStackLockedByCurrentThread(); - ChipLogError(BDX, "HandleTransferSessionOutput: Got an event %s", event.ToString(event.EventType)); + ChipLogError(BDX, "Got an event %s", event.ToString(event.EventType)); + CHIP_ERROR err = CHIP_NO_ERROR; switch (event.EventType) { case TransferSession::OutputEventType::kInitReceived: @@ -181,7 +182,7 @@ } break; case TransferSession::OutputEventType::kStatusReceived: - ChipLogError(BDX, "HandleTransferSessionOutput: Got StatusReport %x", static_cast(event.statusData.statusCode)); + ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); [[fallthrough]]; case TransferSession::OutputEventType::kInternalError: case TransferSession::OutputEventType::kTransferTimeout: @@ -195,21 +196,14 @@ } break; case TransferSession::OutputEventType::kAckEOFReceived: - // Need to call OnTransferSessionEnd(event). Need to fix this and remove isBlockEOFSent. break; case TransferSession::OutputEventType::kMsgToSend: err = OnMessageToSend(event); if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { - // TODO: This is a hack for determinin that the Ack EOF is sent before cleaning up. - // Need to fix this. - isBlockEOFSent = true; + err = OnTransferSessionEnd(event); } break; case TransferSession::OutputEventType::kNone: - if (isBlockEOFSent) { - OnTransferSessionEnd(event); - } - break; case TransferSession::OutputEventType::kQueryWithSkipReceived: case TransferSession::OutputEventType::kQueryReceived: case TransferSession::OutputEventType::kAckReceived: @@ -225,6 +219,7 @@ void MTRDiagnosticLogsTransferHandler::AbortTransfer(chip::bdx::StatusCode reason) { + assertChipStackLockedByCurrentThread(); mTransfer.AbortTransfer(reason); } diff --git a/src/protocols/bdx/BdxTransferSession.cpp b/src/protocols/bdx/BdxTransferSession.cpp index b506048e28c21d..f6c54985ab5cd1 100644 --- a/src/protocols/bdx/BdxTransferSession.cpp +++ b/src/protocols/bdx/BdxTransferSession.cpp @@ -48,6 +48,7 @@ void PrepareOutgoingMessageEvent(MessageType messageType, chip::bdx::TransferSes chip::bdx::TransferSession::MessageTypeData & outputMsgType) { static_assert(std::is_same, uint8_t>::value, "Cast is not safe"); + pendingOutput = chip::bdx::TransferSession::OutputEventType::kMsgToSend; outputMsgType.ProtocolId = chip::Protocols::MessageTypeTraits::ProtocolId(); outputMsgType.MessageType = static_cast(messageType); @@ -320,12 +321,7 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) VerifyOrReturnError(mState == TransferState::kTransferInProgress, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mRole == TransferRole::kSender, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mPendingOutput == OutputEventType::kNone, CHIP_ERROR_INCORRECT_STATE); - bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || - (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); - if (checkAwaitingResponse) - { - VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); - } + VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); // Verify non-zero data is provided and is no longer than MaxBlockSize (BlockEOF may contain 0 length data) VerifyOrReturnError((inData.Data != nullptr) && (inData.Length <= mTransferMaxBlockSize), CHIP_ERROR_INVALID_ARGUMENT); @@ -350,8 +346,7 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) } mAwaitingResponse = true; - - mLastBlockNum = mNextBlockNum++; + mLastBlockNum = mNextBlockNum++; PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); @@ -391,7 +386,7 @@ CHIP_ERROR TransferSession::PrepareBlockAck() mState = TransferState::kTransferDone; mAwaitingResponse = false; } - ChipLogError(BDX, "sending block ack %hhu", msgType); + PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); return CHIP_NO_ERROR; @@ -532,7 +527,6 @@ CHIP_ERROR TransferSession::HandleStatusReportMessage(const PayloadHeader & head void TransferSession::HandleTransferInit(MessageType msgType, System::PacketBufferHandle msgData) { - VerifyOrReturn(mState == TransferState::kAwaitingInitMsg, PrepareStatusReport(StatusCode::kUnexpectedMessage)); if (mRole == TransferRole::kSender) @@ -570,6 +564,7 @@ void TransferSession::HandleTransferInit(MessageType msgType, System::PacketBuff mPendingOutput = OutputEventType::kInitReceived; mState = TransferState::kNegotiateTransferParams; + #if CHIP_AUTOMATION_LOGGING transferInit.LogMessage(msgType); #endif // CHIP_AUTOMATION_LOGGING @@ -692,26 +687,18 @@ void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgDat void TransferSession::HandleBlock(System::PacketBufferHandle msgData) { - VerifyOrReturn(mRole == TransferRole::kReceiver || mRole == TransferRole::kSender, - PrepareStatusReport(StatusCode::kUnexpectedMessage)); + VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || - (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); - if (checkAwaitingResponse) - { - VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - } + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); Block blockMsg; const CHIP_ERROR err = blockMsg.Parse(msgData.Retain()); VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); - if (mControlMode == TransferControlFlags::kReceiverDrive) - { - VerifyOrReturn(blockMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); - } + VerifyOrReturn(blockMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); VerifyOrReturn((blockMsg.DataLength > 0) && (blockMsg.DataLength <= mTransferMaxBlockSize), PrepareStatusReport(StatusCode::kBadMessageContents)); + if (IsTransferLengthDefinite()) { VerifyOrReturn(mNumBytesProcessed + blockMsg.DataLength <= mTransferLength, @@ -740,24 +727,16 @@ void TransferSession::HandleBlockEOF(System::PacketBufferHandle msgData) { VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - - bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || - (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); - if (checkAwaitingResponse) - { - VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - } + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); BlockEOF blockEOFMsg; const CHIP_ERROR err = blockEOFMsg.Parse(msgData.Retain()); VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); - if (mControlMode == TransferControlFlags::kReceiverDrive) - { - VerifyOrReturn(blockEOFMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); - } + VerifyOrReturn(blockEOFMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); VerifyOrReturn(blockEOFMsg.DataLength <= mTransferMaxBlockSize, PrepareStatusReport(StatusCode::kBadMessageContents)); + mBlockEventData.Data = blockEOFMsg.Data; mBlockEventData.Length = blockEOFMsg.DataLength; mBlockEventData.IsEof = true; mBlockEventData.BlockCounter = blockEOFMsg.BlockCounter; @@ -891,6 +870,7 @@ CHIP_ERROR TransferSession::VerifyProposedMode(const BitFlags Date: Thu, 2 Nov 2023 12:51:52 -0700 Subject: [PATCH 06/16] Fix the MTRDeviceTests. generate a log file and update darwin.yaml --- .github/workflows/darwin.yaml | 2 +- .../diagnostic-logs-server.cpp | 2 - src/darwin/Framework/CHIP/MTRDevice.mm | 4 +- .../CHIP/MTRDiagnosticLogsTransferHandler.mm | 11 +- .../Framework/CHIPTests/MTRDeviceTests.m | 233 ++++++++++++------ 5 files changed, 159 insertions(+), 93 deletions(-) diff --git a/.github/workflows/darwin.yaml b/.github/workflows/darwin.yaml index 8b7e44ef82ee00..bb031c48471269 100644 --- a/.github/workflows/darwin.yaml +++ b/.github/workflows/darwin.yaml @@ -103,7 +103,7 @@ jobs: # target versions instead? run: | mkdir -p /tmp/darwin/framework-tests - ../../../out/debug/chip-all-clusters-app --interface-id -1 > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) & + ../../../out/debug/chip-all-clusters-app --interface-id -1 --end_user_support_log /tmp/endusersupportlog.txt --network_diagnostics_log /tmp/networkdiagnosticslog.txt --crash_log /tmp/crashlog.txt > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) & ../../../out/debug/chip-all-clusters-app --interface-id -1 --dac_provider ../../../credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json --product-id 32768 --discriminator 3839 --secured-device-port 5539 --KVS /tmp/chip-all-clusters-app-kvs2 > >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid-err.log >&2) & # Disable BLE because the app does not have the permission to use # it and that may crash the CI. diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index 489c1956ca4fee..91b1ff5554cba4 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -27,8 +27,6 @@ #include #include -#include - using namespace chip; using namespace chip::app; using namespace chip::app::Clusters::DiagnosticLogs; diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index b9ff6d2ec4023e..942a659e3d5b6d 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1393,8 +1393,8 @@ - (NSURL * _Nullable)_temporaryFileURLForDownload:(MTRDiagnosticLogType)type - (bool)_isErrorResponse:(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _Nullable)response { - // TODO: fix the comparision with kNoLogs and kExhausted - return (response == nil || (response.status != nil && [response.status intValue] != 0 && [response.status intValue] != 1) || response.logContent.length == 0); + chip::app::Clusters::DiagnosticLogs::StatusEnum statusValue = static_cast(response.status.intValue); + return (response == nil || (response.status != nil && statusValue != chip::app::Clusters::DiagnosticLogs::StatusEnum::kNoLogs && statusValue != chip::app::Clusters::DiagnosticLogs::StatusEnum::kExhausted) || response.logContent.length == 0); } - (void)_invokeCompletion:(void (^)(NSURL * _Nullable logResult, NSError * error))completion diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index 8dc72575e69d10..4624d604b1e701 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -65,10 +65,9 @@ mFileHandle = [NSFileHandle fileHandleForWritingToURL:mFileURL error:&error]; if (mFileHandle == nil || error != nil) { - // TODO: Map NSError to BDX error - CHIP_ERROR error = CHIP_ERROR_INCORRECT_STATE; - LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(error))); - return error; + CHIP_ERROR err = [MTRError errorToCHIPErrorCode:error]; + LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err))); + return err; } TransferSession::TransferAcceptData acceptData; @@ -118,8 +117,8 @@ [mFileHandle writeData:AsData(blockData) error:&error]; if (error != nil) { - // TODO: Map NSError to BDX error - return mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE)); + CHIP_ERROR err = [MTRError errorToCHIPErrorCode:error]; + return mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)); } } else { return mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(CHIP_ERROR_INCORRECT_STATE)); diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 2197e3e342ed4f..9932690f2f71f5 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -38,7 +38,7 @@ #import static const uint16_t kPairingTimeoutInSeconds = 10; -static const uint16_t kDownloadLogTimeoutInSeconds = 70; +static const uint16_t kDownloadLogTimeoutInSeconds = 120; static const uint16_t kTimeoutInSeconds = 3; static const uint64_t kDeviceId = 0x12344321; static NSString * kOnboardingPayload = @"MT:-24J0AFN00KA0648G00"; @@ -2623,7 +2623,155 @@ - (void)test028_TimeZoneAndDST #endif // MTR_ENABLE_PROVISIONAL } -- (void)test900_SubscribeAllAttributes +/** + * Given a path relative to the Matter root, create an absolute path to the file. + */ +- (NSString *)absolutePathFor:(NSString *)matterRootRelativePath +{ + // Find the right absolute path to our file. PWD should + // point to our src/darwin/Framework. + NSString * pwd = [[NSProcessInfo processInfo] environment][@"PWD"]; + NSMutableArray * pathComponents = [[NSMutableArray alloc] init]; + [pathComponents addObject:[pwd substringToIndex:(pwd.length - @"src/darwin/Framework".length)]]; + [pathComponents addObjectsFromArray:[matterRootRelativePath pathComponents]]; + return [NSString pathWithComponents:pathComponents]; +} + +/** + * Create a task given a path relative to the Matter root. + */ +- (NSTask *)createTaskForPath:(NSString *)path +{ + NSTask * task = [[NSTask alloc] init]; + [task setLaunchPath:[self absolutePathFor:path]]; + return task; +} + +- (NSError *)generateLogFile:(NSString *)outFile +{ + NSTask * appTask = [self createTaskForPath:@"out/debug/all-clusters-app/chip-all-clusters-app"]; + + // Remove the file if one exists + [[NSFileManager defaultManager] removeItemAtPath:outFile error:nil]; + + // Create the file + [[NSFileManager defaultManager] createFileAtPath:outFile contents:nil attributes:nil]; + + NSFileHandle * handle = [NSFileHandle fileHandleForUpdatingAtPath:outFile]; + appTask.standardOutput = handle; + NSError * error = nil; + + [appTask launchAndReturnError:&error]; + return error; +} + +- (void)test029_DownloadEndUserSupportLog_NoTimeout +{ + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + XCTestExpectation * expectation = [self expectationWithDescription:@"End User Support Log Transfer Complete"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + + dispatch_async(queue, ^{ + MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; + XCTAssertNotNil(device); + + NSString * outFile = [NSString stringWithFormat:@"/tmp/endusersupportlog.txt"]; + NSError * error = [self generateLogFile:outFile]; + + if (error != nil) { + NSLog(@"Failed to generate log file"); + return; + } + + [device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { + XCTAssertNil(error); + XCTAssertNotNil(logResult); + + NSError * attributesError = nil; + NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + + size_t fileSize = [fileAttributes fileSize]; + XCTAssertTrue(fileSize > 0); + NSLog(@"test901_DownloadEndUserSupportLog_NoTimeout expectation fulfill"); + [expectation fulfill]; + }]; + }); + [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; +} + +- (void)test030_DownloadNetworkDiagnosticsLog_NoTimeout +{ + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + XCTestExpectation * expectation = [self expectationWithDescription:@"Network Diagnostics Log Transfer Complete"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + + dispatch_async(queue, ^{ + MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; + XCTAssertNotNil(device); + + NSString * outFile = [NSString stringWithFormat:@"/tmp/networkdiagnosticslog.txt"]; + NSError * error = [self generateLogFile:outFile]; + + if (error != nil) { + NSLog(@"Failed to generate log file"); + return; + } + + [device downloadLogOfType:MTRDiagnosticLogTypeNetworkDiagnostics timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { + XCTAssertNil(error); + XCTAssertNotNil(logResult); + + NSError * attributesError = nil; + NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + + size_t fileSize = [fileAttributes fileSize]; + XCTAssertTrue(fileSize > 0); + [expectation fulfill]; + }]; + }); + [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; +} + +- (void)test031_DownloadCrashLog_NoTimeout +{ + MTRDeviceController * controller = sController; + XCTAssertNotNil(controller); + XCTestExpectation * expectation = [self expectationWithDescription:@"Crash Log Transfer Complete"]; + + dispatch_queue_t queue = dispatch_get_main_queue(); + + dispatch_async(queue, ^{ + MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; + XCTAssertNotNil(device); + + NSString * outFile = [NSString stringWithFormat:@"/tmp/crashlog.txt"]; + NSError * error = [self generateLogFile:outFile]; + + if (error != nil) { + NSLog(@"Failed to generate log file"); + return; + } + + [device downloadLogOfType:MTRDiagnosticLogTypeCrash timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { + XCTAssertNil(error); + XCTAssertNotNil(logResult); + + NSError * attributesError = nil; + NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; + + size_t fileSize = [fileAttributes fileSize]; + XCTAssertTrue(fileSize > 0); + [expectation fulfill]; + }]; + }); + [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; +} + +/*- (void)test900_SubscribeAllAttributes { MTRBaseDevice * device = GetConnectedDevice(); dispatch_queue_t queue = dispatch_get_main_queue(); @@ -2794,86 +2942,7 @@ - (void)test900_SubscribeAllAttributes // Wait for report [self waitForExpectations:[NSArray arrayWithObject:reportExpectation] timeout:kTimeoutInSeconds]; -} - -- (void)test901_DownloadEndUserSupportLog_NoTimeout -{ - MTRDeviceController * controller = sController; - XCTAssertNotNil(controller); - XCTestExpectation * expectation = [self expectationWithDescription:@"End User Support Log Transfer Complete"]; - - dispatch_queue_t queue = dispatch_get_main_queue(); - - dispatch_async(queue, ^{ - MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; - XCTAssertNotNil(device); - [device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { - XCTAssertNil(error); - XCTAssertNotNil(logResult); - - NSError * attributesError = nil; - NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; - - size_t fileSize = [fileAttributes fileSize]; - XCTAssertTrue(fileSize > 0); - NSLog(@"test901_DownloadEndUserSupportLog_NoTimeout expectation fulfill"); - [expectation fulfill]; - }]; - }); - [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; -} - -- (void)test902_DownloadNetworkDiagnosticsLog_NoTimeout -{ - MTRDeviceController * controller = sController; - XCTAssertNotNil(controller); - XCTestExpectation * expectation = [self expectationWithDescription:@"Network Diagnostics Log Transfer Complete"]; - - dispatch_queue_t queue = dispatch_get_main_queue(); - - dispatch_async(queue, ^{ - MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; - XCTAssertNotNil(device); - [device downloadLogOfType:MTRDiagnosticLogTypeNetworkDiagnostics timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { - XCTAssertNil(error); - XCTAssertNotNil(logResult); - - NSError * attributesError = nil; - NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; - - size_t fileSize = [fileAttributes fileSize]; - XCTAssertTrue(fileSize > 0); - [expectation fulfill]; - }]; - }); - [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; -} - -- (void)test903_DownloadCrashLog_NoTimeout -{ - MTRDeviceController * controller = sController; - XCTAssertNotNil(controller); - XCTestExpectation * expectation = [self expectationWithDescription:@"Crash Log Transfer Complete"]; - - dispatch_queue_t queue = dispatch_get_main_queue(); - - dispatch_async(queue, ^{ - MTRDevice * device = [MTRDevice deviceWithNodeID:@(kDeviceId) controller:controller]; - XCTAssertNotNil(device); - [device downloadLogOfType:MTRDiagnosticLogTypeCrash timeout:0 queue:queue completion:^(NSURL * _Nullable logResult, NSError * error) { - XCTAssertNil(error); - XCTAssertNotNil(logResult); - - NSError * attributesError = nil; - NSDictionary * fileAttributes = [[NSFileManager defaultManager] attributesOfItemAtPath:[logResult path] error:&attributesError]; - - size_t fileSize = [fileAttributes fileSize]; - XCTAssertTrue(fileSize > 0); - [expectation fulfill]; - }]; - }); - [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; -} +}*/ - (void)test999_TearDown { From bc2cbc07f6737e98559831b60477842c3daa9b79 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 2 Nov 2023 13:50:46 -0700 Subject: [PATCH 07/16] regenerate zap files --- .../all-clusters-app.matter | 46 ------------- .../all-clusters-common/all-clusters-app.zap | 65 +------------------ 2 files changed, 3 insertions(+), 108 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter index 66cb808ab93ad3..f1bf5691c89526 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -1530,51 +1530,6 @@ server cluster NetworkCommissioning = 49 { command access(invoke: administer) ReorderNetwork(ReorderNetworkRequest): NetworkConfigResponse = 8; } -/** The cluster provides commands for retrieving unstructured diagnostic logs from a Node that may be used to aid in diagnostics. */ -client cluster DiagnosticLogs = 50 { - enum IntentEnum : ENUM8 { - kEndUserSupport = 0; - kNetworkDiag = 1; - kCrashLogs = 2; - } - - enum StatusEnum : ENUM8 { - kSuccess = 0; - kExhausted = 1; - kNoLogs = 2; - kBusy = 3; - kDenied = 4; - } - - enum TransferProtocolEnum : ENUM8 { - kResponsePayload = 0; - kBDX = 1; - } - - readonly attribute command_id generatedCommandList[] = 65528; - readonly attribute command_id acceptedCommandList[] = 65529; - readonly attribute event_id eventList[] = 65530; - readonly attribute attrib_id attributeList[] = 65531; - readonly attribute bitmap32 featureMap = 65532; - readonly attribute int16u clusterRevision = 65533; - - request struct RetrieveLogsRequestRequest { - IntentEnum intent = 0; - TransferProtocolEnum requestedProtocol = 1; - optional char_string<32> transferFileDesignator = 2; - } - - response struct RetrieveLogsResponse = 1 { - StatusEnum status = 0; - LONG_OCTET_STRING logContent = 1; - optional epoch_us UTCTimeStamp = 2; - optional systime_us timeSinceBoot = 3; - } - - /** Retrieving diagnostic logs from a Node */ - command RetrieveLogsRequest(RetrieveLogsRequestRequest): RetrieveLogsResponse = 0; -} - /** The cluster provides commands for retrieving unstructured diagnostic logs from a Node that may be used to aid in diagnostics. */ server cluster DiagnosticLogs = 50 { enum IntentEnum : enum8 { @@ -5218,7 +5173,6 @@ endpoint 0 { device type ma_powersource = 17, version 1; binding cluster OtaSoftwareUpdateProvider; - binding cluster DiagnosticLogs; server cluster Identify { ram attribute identifyTime default = 0x0000; diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap index 8b6d1495869b1e..ad2a4d8752466f 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap @@ -2628,66 +2628,6 @@ } ] }, - { - "name": "Diagnostic Logs", - "code": 50, - "mfgCode": null, - "define": "DIAGNOSTIC_LOGS_CLUSTER", - "side": "client", - "enabled": 1, - "commands": [ - { - "name": "RetrieveLogsRequest", - "code": 0, - "mfgCode": null, - "source": "client", - "isIncoming": 0, - "isEnabled": 1 - }, - { - "name": "RetrieveLogsResponse", - "code": 1, - "mfgCode": null, - "source": "server", - "isIncoming": 1, - "isEnabled": 1 - } - ], - "attributes": [ - { - "name": "FeatureMap", - "code": 65532, - "mfgCode": null, - "side": "client", - "type": "bitmap32", - "included": 1, - "storageOption": "RAM", - "singleton": 0, - "bounded": 0, - "defaultValue": "0", - "reportable": 1, - "minInterval": 1, - "maxInterval": 65534, - "reportableChange": 0 - }, - { - "name": "ClusterRevision", - "code": 65533, - "mfgCode": null, - "side": "client", - "type": "int16u", - "included": 1, - "storageOption": "RAM", - "singleton": 0, - "bounded": 0, - "defaultValue": "1", - "reportable": 1, - "minInterval": 1, - "maxInterval": 65534, - "reportableChange": 0 - } - ] - }, { "name": "Diagnostic Logs", "code": 50, @@ -14988,7 +14928,7 @@ "code": 3, "mfgCode": null, "side": "server", - "type": "temperature", + "type": "int16u", "included": 1, "storageOption": "RAM", "singleton": 0, @@ -22033,5 +21973,6 @@ "endpointId": 65534, "networkId": 0 } - ] + ], + "log": [] } \ No newline at end of file From dcadb9135b4815da11eebd9a97fff8c404991a97 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 2 Nov 2023 14:12:09 -0700 Subject: [PATCH 08/16] Revert incorrect changes to MTRCommandPayloadsObjc.h --- .../Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h index d95bd5c04dbdde..9e6b0e19d006a4 100644 --- a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h +++ b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h @@ -3144,7 +3144,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) @property (nonatomic, copy) NSNumber * _Nonnull requestedProtocol MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)); -@property (nonatomic, copy) NSString * _Nullable transferFileDesignator MTR_AVAILABLE(ios(16.5), macos(13.0), watchos(9.5), tvos(16.5)); +@property (nonatomic, copy) NSString * _Nullable transferFileDesignator MTR_AVAILABLE(ios(16.5), macos(13.4), watchos(9.5), tvos(16.5)); /** * Controls whether the command is a timed command (using Timed Invoke). * From 4a67f96739344cb2f75c06cc27b9a5db37cef1ba Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 2 Nov 2023 14:19:55 -0700 Subject: [PATCH 09/16] Revert TransferFacilitator changes to PollOutput --- src/protocols/bdx/TransferFacilitator.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp index c9da274ed792ac..604eec88d40975 100644 --- a/src/protocols/bdx/TransferFacilitator.cpp +++ b/src/protocols/bdx/TransferFacilitator.cpp @@ -68,11 +68,7 @@ void TransferFacilitator::OnResponseTimeout(Messaging::ExchangeContext * ec) void TransferFacilitator::PollTimerHandler(chip::System::Layer * systemLayer, void * appState) { VerifyOrReturn(appState != nullptr); - VerifyOrReturn(systemLayer != nullptr); - if (appState != nullptr) - { - static_cast(appState)->PollForOutput(); - } + static_cast(appState)->PollForOutput(); } void TransferFacilitator::PollForOutput() From 76aa77221a528426fa870b968c85259b80429de9 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 2 Nov 2023 14:54:26 -0700 Subject: [PATCH 10/16] Test clean up --- src/darwin/Framework/CHIPTests/MTRDeviceTests.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 9932690f2f71f5..7958fe3e6dd4fa 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -38,7 +38,7 @@ #import static const uint16_t kPairingTimeoutInSeconds = 10; -static const uint16_t kDownloadLogTimeoutInSeconds = 120; +static const uint16_t kDownloadLogTimeoutInSeconds = 50; static const uint16_t kTimeoutInSeconds = 3; static const uint64_t kDeviceId = 0x12344321; static NSString * kOnboardingPayload = @"MT:-24J0AFN00KA0648G00"; @@ -2771,7 +2771,7 @@ - (void)test031_DownloadCrashLog_NoTimeout [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kDownloadLogTimeoutInSeconds]; } -/*- (void)test900_SubscribeAllAttributes +- (void)test900_SubscribeAllAttributes { MTRBaseDevice * device = GetConnectedDevice(); dispatch_queue_t queue = dispatch_get_main_queue(); @@ -2942,7 +2942,7 @@ - (void)test031_DownloadCrashLog_NoTimeout // Wait for report [self waitForExpectations:[NSArray arrayWithObject:reportExpectation] timeout:kTimeoutInSeconds]; -}*/ +} - (void)test999_TearDown { From 0028414d9b7dcccfe1fd26ca07b7a72b36713195 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 2 Nov 2023 15:56:03 -0700 Subject: [PATCH 11/16] Clean up AppOptions --- examples/all-clusters-app/linux/AppOptions.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/examples/all-clusters-app/linux/AppOptions.cpp b/examples/all-clusters-app/linux/AppOptions.cpp index ee79f7a0e1c0cd..0345945ba1a0ab 100644 --- a/examples/all-clusters-app/linux/AppOptions.cpp +++ b/examples/all-clusters-app/linux/AppOptions.cpp @@ -47,7 +47,6 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id switch (identifier) { case kOptionDacProviderFilePath: - ChipLogError(BDX, "dac provider path"); mDacProvider.Init(value); break; case kOptionMinCommissioningTimeout: { @@ -56,7 +55,6 @@ bool AppOptions::HandleOptions(const char * program, OptionSet * options, int id break; } case kOptionEndUserSupportFilePath: { - ChipLogError(BDX, "kOptionEndUserSupportFilePath setting end user fd"); if (strlen(value) > kLogFileDesignatorMaxLen) { PrintArgError("%s: Invalid file path length. Must be less that %d: %d\n", program, kLogFileDesignatorMaxLen, From 52ec06eddd3ac5fc35d3eabee133576a93bf2894 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Thu, 2 Nov 2023 16:06:45 -0700 Subject: [PATCH 12/16] Fix the path for all-clusters-app for CI --- src/darwin/Framework/CHIPTests/MTRDeviceTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 7958fe3e6dd4fa..91223cad5ee8b6 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -2649,7 +2649,7 @@ - (NSTask *)createTaskForPath:(NSString *)path - (NSError *)generateLogFile:(NSString *)outFile { - NSTask * appTask = [self createTaskForPath:@"out/debug/all-clusters-app/chip-all-clusters-app"]; + NSTask * appTask = [self createTaskForPath:@"out/debug/chip-all-clusters-app"]; // Remove the file if one exists [[NSFileManager defaultManager] removeItemAtPath:outFile error:nil]; From baca50ed4ce04c293570c0dc14edaabcc5c1e327 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Fri, 3 Nov 2023 11:37:33 -0700 Subject: [PATCH 13/16] Fix clang tidy issue --- .../src/diagnostic-logs-provider-delegate-impl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp index 163e4a20691afd..78afc8cda1d53f 100644 --- a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp @@ -45,12 +45,12 @@ LogSessionHandle LogProvider::StartLogCollection(IntentEnum logType) } sLogSessionHandle++; mLogSessionHandle = sLogSessionHandle; - return mLogSessionHandle; } else { - return kInvalidLogSessionHandle; + mLogSessionHandle = kInvalidLogSessionHandle; } + return mLogSessionHandle; } uint64_t LogProvider::GetNextChunk(LogSessionHandle logSessionHandle, chip::MutableByteSpan & outBuffer, bool & outIsEOF) From b5e2f4e56293b487e39f945ef5917fa72afa75e2 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 3 Nov 2023 20:53:55 +0000 Subject: [PATCH 14/16] Restyled by clang-format --- .../src/diagnostic-logs-provider-delegate-impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp index 78afc8cda1d53f..401afc1e7359f8 100644 --- a/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/diagnostic-logs-provider-delegate-impl.cpp @@ -48,7 +48,7 @@ LogSessionHandle LogProvider::StartLogCollection(IntentEnum logType) } else { - mLogSessionHandle = kInvalidLogSessionHandle; + mLogSessionHandle = kInvalidLogSessionHandle; } return mLogSessionHandle; } From 7294b5a7d7a1badfc19fcff74ce5b9c5ad056492 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Sun, 5 Nov 2023 10:29:21 -0800 Subject: [PATCH 15/16] Call reset before deleting the MTRDiagnosticLogsTransferHandler object --- src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index 4624d604b1e701..8caada05554dbb 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -93,12 +93,12 @@ } else if (event.EventType != TransferSession::OutputEventType::kMsgToSend || !event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { error = CHIP_ERROR_INTERNAL; } + Reset(); // Notify the MTRDevice via the callback that the BDX transfer has completed with error or success. if (mCallback) { mCallback(error != CHIP_NO_ERROR ? NO : YES); } - Reset(); return error; } From e0fc837fcf25c6ee53289eec7f19fe93a3597889 Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Mon, 6 Nov 2023 21:30:15 -0800 Subject: [PATCH 16/16] Fix MTRDevice tests for diagnostic logs --- .../diagnostic-logs-server/diagnostic-logs-server.cpp | 1 + src/darwin/Framework/CHIP/MTRDevice.mm | 4 ++-- src/protocols/bdx/TransferFacilitator.cpp | 2 -- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp index 91b1ff5554cba4..19cdad18e6db45 100644 --- a/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp +++ b/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 1591c31e525c69..e4c76cd6a4a4dd 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -1437,7 +1437,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidState userInfo:nil]]; } - if (self->_diagnosticLogsTransferHandler != nullptr && self->_diagnosticLogsTransferHandler->IsInBDXSession()) { + if (self->_diagnosticLogsTransferHandler != nil && self->_diagnosticLogsTransferHandler->IsInBDXSession()) { [self _invokeCompletionWithError:completion queue:queue error:[NSError errorWithDomain:MTRInteractionErrorDomain code:MTRInteractionErrorCodeBusy userInfo:nil]]; } @@ -1471,7 +1471,7 @@ - (void)_downloadLogOfType:(MTRDiagnosticLogType)type [cluster retrieveLogsRequestWithParams:requestParams expectedValues:nil expectedValueInterval:nil completion:^(MTRDiagnosticLogsClusterRetrieveLogsResponseParams * _Nullable response, NSError * _Nullable error) { // If we are in a BDX session and there is no error, do nothing. Completion will be called when BDX succeeds or fails. - if (self->_diagnosticLogsTransferHandler->IsInBDXSession() && error == nil) { + if (self->_diagnosticLogsTransferHandler != nil && self->_diagnosticLogsTransferHandler->IsInBDXSession() && error == nil) { return; } diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp index 604eec88d40975..fbe563187b6fb9 100644 --- a/src/protocols/bdx/TransferFacilitator.cpp +++ b/src/protocols/bdx/TransferFacilitator.cpp @@ -114,7 +114,6 @@ CHIP_ERROR Responder::PrepareForTransfer(System::Layer * layer, TransferRole rol void Responder::ResetTransfer() { mTransfer.Reset(); - mSystemLayer = nullptr; ChipLogProgress(BDX, "Stop polling for messages"); mStopPolling = true; } @@ -136,7 +135,6 @@ CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, void Initiator::ResetTransfer() { mTransfer.Reset(); - mSystemLayer = nullptr; ChipLogProgress(BDX, "Stop polling for messages"); mStopPolling = true; }