From fc159a706f0f1fbdffb526cad8e5847209dd6b14 Mon Sep 17 00:00:00 2001 From: kangping Date: Thu, 29 Oct 2020 13:13:03 +0800 Subject: [PATCH] [net-diag] invoke the callback when failed to get the response (#5653) This commit fixes the bug that the Network Diagnostic callback is not called when we failed to get the response (e.g. the request timeouted). This results in no command output in the CLI. --- .github/workflows/simulation-1.2.yml | 1 + include/openthread/instance.h | 2 +- include/openthread/netdiag.h | 11 +++++++--- src/cli/cli.cpp | 28 ++++++++++++++++++-------- src/cli/cli.hpp | 7 +++++-- src/core/thread/network_diagnostic.cpp | 17 ++++++++-------- 6 files changed, 44 insertions(+), 22 deletions(-) diff --git a/.github/workflows/simulation-1.2.yml b/.github/workflows/simulation-1.2.yml index fd592319a6c..5975c60a65e 100644 --- a/.github/workflows/simulation-1.2.yml +++ b/.github/workflows/simulation-1.2.yml @@ -228,6 +228,7 @@ jobs: MULTIPLY: 3 PYTHONUNBUFFERED: 1 VERBOSE: 1 + OTBR_COMMIT: "3cf6ff1126dca96803252514691f40d0554046bf" # Date: Mon Oct 26 15:27:04 2020 +0800 steps: - uses: actions/checkout@v2 - name: Build OTBR Docker diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 108b2e24f2f..c22a828f034 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (41) +#define OPENTHREAD_API_VERSION (42) /** * @addtogroup api-instance diff --git a/include/openthread/netdiag.h b/include/openthread/netdiag.h index b35e95ef677..376780f1305 100644 --- a/include/openthread/netdiag.h +++ b/include/openthread/netdiag.h @@ -283,13 +283,18 @@ otError otThreadGetNextDiagnosticTlv(const otMessage * aMessage, /** * This function pointer is called when Network Diagnostic Get response is received. * + * @param[in] aError The error when failed to get the response. * @param[in] aMessage A pointer to the message buffer containing the received Network Diagnostic - * Get response payload. - * @param[in] aMessageInfo A pointer to the message info for @p aMessage. + * Get response payload. Available only when @p aError is `OT_ERROR_NONE`. + * @param[in] aMessageInfo A pointer to the message info for @p aMessage. Available only when + * @p aError is `OT_ERROR_NONE`. * @param[in] aContext A pointer to application-specific context. * */ -typedef void (*otReceiveDiagnosticGetCallback)(otMessage *aMessage, const otMessageInfo *aMessageInfo, void *aContext); +typedef void (*otReceiveDiagnosticGetCallback)(otError aError, + otMessage * aMessage, + const otMessageInfo *aMessageInfo, + void * aContext); /** * This function registers a callback to provide received raw Network Diagnostic Get response payload. diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 184395c233c..c2c0634d960 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -4249,28 +4249,34 @@ otError Interpreter::ProcessNetworkDiagnostic(uint8_t aArgsLength, char *aArgs[] return error; } -void Interpreter::HandleDiagnosticGetResponse(otMessage *aMessage, const otMessageInfo *aMessageInfo, void *aContext) +void Interpreter::HandleDiagnosticGetResponse(otError aError, + otMessage * aMessage, + const otMessageInfo *aMessageInfo, + void * aContext) { static_cast(aContext)->HandleDiagnosticGetResponse( - *aMessage, *static_cast(aMessageInfo)); + aError, aMessage, static_cast(aMessageInfo)); } -void Interpreter::HandleDiagnosticGetResponse(const otMessage &aMessage, const Ip6::MessageInfo &) +void Interpreter::HandleDiagnosticGetResponse(otError aError, const otMessage *aMessage, const Ip6::MessageInfo *) { uint8_t buf[16]; uint16_t bytesToPrint; uint16_t bytesPrinted = 0; - uint16_t length = otMessageGetLength(&aMessage) - otMessageGetOffset(&aMessage); + uint16_t length; otNetworkDiagTlv diagTlv; otNetworkDiagIterator iterator = OT_NETWORK_DIAGNOSTIC_ITERATOR_INIT; - otError error = OT_ERROR_NONE; + + SuccessOrExit(aError); OutputFormat("DIAG_GET.rsp/ans: "); + length = otMessageGetLength(aMessage) - otMessageGetOffset(aMessage); + while (length > 0) { bytesToPrint = (length < sizeof(buf)) ? length : sizeof(buf); - otMessageRead(&aMessage, otMessageGetOffset(&aMessage) + bytesPrinted, buf, bytesToPrint); + otMessageRead(aMessage, otMessageGetOffset(aMessage) + bytesPrinted, buf, bytesToPrint); OutputBytes(buf, static_cast(bytesToPrint)); @@ -4281,7 +4287,7 @@ void Interpreter::HandleDiagnosticGetResponse(const otMessage &aMessage, const I OutputLine(""); // Output Network Diagnostic TLV values in standard YAML format. - while ((error = otThreadGetNextDiagnosticTlv(&aMessage, &iterator, &diagTlv)) == OT_ERROR_NONE) + while ((aError = otThreadGetNextDiagnosticTlv(aMessage, &iterator, &diagTlv)) == OT_ERROR_NONE) { uint16_t column = 0; switch (diagTlv.mType) @@ -4358,7 +4364,13 @@ void Interpreter::HandleDiagnosticGetResponse(const otMessage &aMessage, const I } } - OutputResult(error == OT_ERROR_NOT_FOUND ? OT_ERROR_NONE : error); + if (aError == OT_ERROR_NOT_FOUND) + { + aError = OT_ERROR_NONE; + } + +exit: + OutputResult(aError); } void Interpreter::OutputSpaces(uint16_t aCount) diff --git a/src/cli/cli.hpp b/src/cli/cli.hpp index b84c9881d96..392497fbbd4 100644 --- a/src/cli/cli.hpp +++ b/src/cli/cli.hpp @@ -485,8 +485,11 @@ class Interpreter static void HandleLinkPcapReceive(const otRadioFrame *aFrame, bool aIsTx, void *aContext); #if OPENTHREAD_FTD || OPENTHREAD_CONFIG_TMF_NETWORK_DIAG_MTD_ENABLE - void HandleDiagnosticGetResponse(const otMessage &aMessage, const Ip6::MessageInfo &aMessageInfo); - static void HandleDiagnosticGetResponse(otMessage *aMessage, const otMessageInfo *aMessageInfo, void *aContext); + void HandleDiagnosticGetResponse(otError aError, const otMessage *aMessage, const Ip6::MessageInfo *aMessageInfo); + static void HandleDiagnosticGetResponse(otError aError, + otMessage * aMessage, + const otMessageInfo *aMessageInfo, + void * aContext); void OutputSpaces(uint16_t aCount); void OutputMode(const otLinkModeConfig &aMode, uint16_t aColumn); void OutputConnectivity(const otNetworkDiagConnectivity &aConnectivity, uint16_t aColumn); diff --git a/src/core/thread/network_diagnostic.cpp b/src/core/thread/network_diagnostic.cpp index 179529623ae..d94bcb69b2a 100644 --- a/src/core/thread/network_diagnostic.cpp +++ b/src/core/thread/network_diagnostic.cpp @@ -141,17 +141,18 @@ void NetworkDiagnostic::HandleDiagnosticGetResponse(Coap::Message * aMes const Ip6::MessageInfo *aMessageInfo, otError aResult) { - VerifyOrExit(aResult == OT_ERROR_NONE); - VerifyOrExit(aMessage && aMessage->GetCode() == Coap::kCodeChanged); - - otLogInfoNetDiag("Received diagnostic get response"); + SuccessOrExit(aResult); + VerifyOrExit(aMessage->GetCode() == Coap::kCodeChanged, aResult = OT_ERROR_FAILED); +exit: if (mReceiveDiagnosticGetCallback) { - mReceiveDiagnosticGetCallback(aMessage, aMessageInfo, mReceiveDiagnosticGetCallbackContext); + mReceiveDiagnosticGetCallback(aResult, aMessage, aMessageInfo, mReceiveDiagnosticGetCallbackContext); + } + else + { + otLogDebgNetDiag("Received diagnostic get response, error = %s", otThreadErrorToString(aResult)); } - -exit: return; } @@ -171,7 +172,7 @@ void NetworkDiagnostic::HandleDiagnosticGetAnswer(Coap::Message &aMessage, const if (mReceiveDiagnosticGetCallback) { - mReceiveDiagnosticGetCallback(&aMessage, &aMessageInfo, mReceiveDiagnosticGetCallbackContext); + mReceiveDiagnosticGetCallback(OT_ERROR_NONE, &aMessage, &aMessageInfo, mReceiveDiagnosticGetCallbackContext); } SuccessOrExit(Get().SendEmptyAck(aMessage, aMessageInfo));