From 851e00ffdb7d5f8b039e8b745692f96a40d4e77a Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 18 Sep 2023 16:26:12 -0700 Subject: [PATCH] [border-agent] smaller enhancements (#9432) This commit contains smaller changes in `BorderAgent` class: - Define `kUdpPort` as a private constants in `BorderAgent` class. - Add `SendMessage()` method to send message through `SecureAgent`. - Add `LogError()` as a method so the correct log module is used. - Remove unused `mMessageInfo` member variable. - Harmonize log messages. --- src/core/meshcop/border_agent.cpp | 52 +++++++++++++++++++++---------- src/core/meshcop/border_agent.hpp | 29 ++++++++--------- 2 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index 6548d5829..0cdfac27a 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -53,9 +53,8 @@ namespace MeshCoP { RegisterLogModule("BorderAgent"); -namespace { -constexpr uint16_t kBorderAgentUdpPort = OPENTHREAD_CONFIG_BORDER_AGENT_UDP_PORT; ///< UDP port of border agent service. -} +//---------------------------------------------------------------------------------------------------------------------- +// `BorderAgent::ForwardContext` void BorderAgent::ForwardContext::Init(Instance &aInstance, const Coap::Message &aMessage, @@ -90,6 +89,9 @@ Error BorderAgent::ForwardContext::ToHeader(Coap::Message &aMessage, uint8_t aCo return aMessage.SetToken(mToken, mTokenLength); } +//---------------------------------------------------------------------------------------------------------------------- +// `BorderAgent` + Coap::Message::Code BorderAgent::CoapCodeFromError(Error aError) { Coap::Message::Code code; @@ -119,7 +121,7 @@ void BorderAgent::SendErrorMessage(ForwardContext &aForwardContext, Error aError VerifyOrExit((message = Get().NewPriorityMessage()) != nullptr, error = kErrorNoBufs); SuccessOrExit(error = aForwardContext.ToHeader(*message, CoapCodeFromError(aError))); - SuccessOrExit(error = Get().SendMessage(*message, Get().GetMessageInfo())); + SuccessOrExit(error = SendMessage(*message)); exit: FreeMessageOnError(message, error); @@ -149,13 +151,18 @@ void BorderAgent::SendErrorMessage(const Coap::Message &aRequest, bool aSeparate SuccessOrExit(error = message->SetTokenFromMessage(aRequest)); - SuccessOrExit(error = Get().SendMessage(*message, Get().GetMessageInfo())); + SuccessOrExit(error = SendMessage(*message)); exit: FreeMessageOnError(message, error); LogError("send error CoAP message", error); } +Error BorderAgent::SendMessage(Coap::Message &aMessage) +{ + return Get().SendMessage(aMessage, Get().GetMessageInfo()); +} + void BorderAgent::HandleCoapResponse(void *aContext, otMessage *aMessage, const otMessageInfo *aMessageInfo, @@ -192,7 +199,7 @@ void BorderAgent::HandleCoapResponse(ForwardContext &aForwardContext, const Coap Get().AddUnicastAddress(mCommissionerAloc); IgnoreError(Get().AddReceiver(mUdpReceiver)); - LogInfo("commissioner accepted: session ID=%d, ALOC=%s", sessionId, + LogInfo("Commissioner accepted - SessionId:%u ALOC:%s", sessionId, mCommissionerAloc.GetAddress().ToString().AsCString()); } } @@ -222,10 +229,10 @@ exit: BorderAgent::BorderAgent(Instance &aInstance) : InstanceLocator(aInstance) - , mUdpReceiver(BorderAgent::HandleUdpReceive, this) - , mTimer(aInstance) , mState(kStateStopped) , mUdpProxyPort(0) + , mUdpReceiver(BorderAgent::HandleUdpReceive, this) + , mTimer(aInstance) #if OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE , mIdInitialized(false) #endif @@ -334,6 +341,11 @@ exit: LogError("send proxy stream", error); } +bool BorderAgent::HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo) +{ + return static_cast(aContext)->HandleUdpReceive(AsCoreType(aMessage), AsCoreType(aMessageInfo)); +} + bool BorderAgent::HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { Error error; @@ -369,7 +381,7 @@ bool BorderAgent::HandleUdpReceive(const Message &aMessage, const Ip6::MessageIn SuccessOrExit(error = Tlv::Append(*message, aMessageInfo.GetPeerAddr())); - SuccessOrExit(error = Get().SendMessage(*message, Get().GetMessageInfo())); + SuccessOrExit(error = SendMessage(*message)); LogInfo("Sent to commissioner on ProxyRx (c/ur)"); @@ -377,7 +389,7 @@ exit: FreeMessageOnError(message, error); if (error != kErrorDestinationAddressFiltered) { - LogError("Notify commissioner on ProxyRx (c/ur)", error); + LogError("notify commissioner on ProxyRx (c/ur)", error); } return error != kErrorDestinationAddressFiltered; @@ -410,8 +422,7 @@ Error BorderAgent::ForwardToCommissioner(Coap::Message &aForwardMessage, const M SuccessOrExit(error = aForwardMessage.AppendBytesFromMessage(aMessage, aMessage.GetOffset(), aMessage.GetLength() - aMessage.GetOffset())); - SuccessOrExit(error = - Get().SendMessage(aForwardMessage, Get().GetMessageInfo())); + SuccessOrExit(error = SendMessage(aForwardMessage)); LogInfo("Sent to commissioner"); @@ -603,7 +614,7 @@ void BorderAgent::Start(void) VerifyOrExit(mState == kStateStopped, error = kErrorNone); Get().GetPskc(pskc); - SuccessOrExit(error = Get().Start(kBorderAgentUdpPort)); + SuccessOrExit(error = Get().Start(kUdpPort)); SuccessOrExit(error = Get().SetPsk(pskc.m8, Pskc::kSize)); pskc.Clear(); @@ -615,10 +626,7 @@ void BorderAgent::Start(void) LogInfo("Border Agent start listening on port %u", GetUdpPort()); exit: - if (error != kErrorNone) - { - LogWarn("failed to start Border Agent on port %d: %s", kBorderAgentUdpPort, ErrorToString(error)); - } + LogError("start agent", error); } void BorderAgent::HandleTimeout(void) @@ -661,6 +669,16 @@ exit: return; } +#if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_WARN) +void BorderAgent::LogError(const char *aActionText, Error aError) +{ + if (aError != kErrorNone) + { + LogWarn("Failed to %s: %s", aActionText, ErrorToString(aError)); + } +} +#endif + } // namespace MeshCoP } // namespace ot diff --git a/src/core/meshcop/border_agent.hpp b/src/core/meshcop/border_agent.hpp index c16c1c80b..fadd87202 100644 --- a/src/core/meshcop/border_agent.hpp +++ b/src/core/meshcop/border_agent.hpp @@ -156,6 +156,9 @@ public: uint16_t GetUdpProxyPort(void) const { return mUdpProxyPort; } private: + static constexpr uint16_t kUdpPort = OPENTHREAD_CONFIG_BORDER_AGENT_UDP_PORT; + static constexpr uint32_t kKeepAliveTimeout = 50 * 1000; // Timeout to reject a commissioner (in msec) + class ForwardContext : public InstanceLocatorInit { public: @@ -176,6 +179,7 @@ private: void HandleNotifierEvents(Events aEvents); Coap::Message::Code CoapCodeFromError(Error aError); + Error SendMessage(Coap::Message &aMessage); void SendErrorMessage(ForwardContext &aForwardContext, Error aError); void SendErrorMessage(const Coap::Message &aRequest, bool aSeparate, Error aError); @@ -191,27 +195,24 @@ private: const otMessageInfo *aMessageInfo, Error aResult); void HandleCoapResponse(ForwardContext &aForwardContext, const Coap::Message *aResponse, Error aResult); - Error ForwardToLeader(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, Uri aUri); Error ForwardToCommissioner(Coap::Message &aForwardMessage, const Message &aMessage); - static bool HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo) - { - return static_cast(aContext)->HandleUdpReceive(AsCoreType(aMessage), AsCoreType(aMessageInfo)); - } - bool HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); + static bool HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo); + bool HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); - static constexpr uint32_t kKeepAliveTimeout = 50 * 1000; // Timeout to reject a commissioner. +#if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_WARN) + void LogError(const char *aActionText, Error aError); +#else + void LogError(const char *, Error) {} +#endif using TimeoutTimer = TimerMilliIn; - Ip6::MessageInfo mMessageInfo; - - Ip6::Udp::Receiver mUdpReceiver; ///< The UDP receiver to receive packets from external commissioner + State mState; + uint16_t mUdpProxyPort; + Ip6::Udp::Receiver mUdpReceiver; Ip6::Netif::UnicastAddress mCommissionerAloc; - - TimeoutTimer mTimer; - State mState; - uint16_t mUdpProxyPort; + TimeoutTimer mTimer; #if OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE Id mId; bool mIdInitialized;