From b25989af188373de460b8fd7318e8740c39b11ed Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Mon, 19 Jul 2021 20:01:40 -0700 Subject: [PATCH 1/9] BdxEndpoint class for ExchangeContext integration --- src/protocols/bdx/BUILD.gn | 2 + src/protocols/bdx/BdxEndpoint.cpp | 97 +++++++++++++++++++++++ src/protocols/bdx/BdxEndpoint.h | 124 ++++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+) create mode 100644 src/protocols/bdx/BdxEndpoint.cpp create mode 100644 src/protocols/bdx/BdxEndpoint.h diff --git a/src/protocols/bdx/BUILD.gn b/src/protocols/bdx/BUILD.gn index fe39bcc3e0ea4a..2b26e4ee9645f6 100644 --- a/src/protocols/bdx/BUILD.gn +++ b/src/protocols/bdx/BUILD.gn @@ -18,6 +18,8 @@ static_library("bdx") { output_name = "libBdx" sources = [ + "BdxEndpoint.cpp", + "BdxEndpoint.h", "BdxMessages.cpp", "BdxMessages.h", "BdxTransferSession.cpp", diff --git a/src/protocols/bdx/BdxEndpoint.cpp b/src/protocols/bdx/BdxEndpoint.cpp new file mode 100644 index 00000000000000..a8415c23640ead --- /dev/null +++ b/src/protocols/bdx/BdxEndpoint.cpp @@ -0,0 +1,97 @@ +/* + * + * Copyright (c) 2021 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 "BdxEndpoint.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace chip { +namespace bdx { + +CHIP_ERROR Endpoint::OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PacketHeader & packetHeader, + const chip::PayloadHeader & payloadHeader, chip::System::PacketBufferHandle && payload) +{ + if (mExchangeCtx == nullptr) + { + mExchangeCtx = ec; + } + + ChipLogDetail(BDX, "%s: message 0x%x protocol %u", __FUNCTION__, static_cast(payloadHeader.GetMessageType()), + payloadHeader.GetProtocolID().GetProtocolId()); + CHIP_ERROR err = mTransfer.HandleMessageReceived(payloadHeader, std::move(payload), System::Platform::Clock::GetMonotonicMilliseconds()); + if (err != CHIP_NO_ERROR) + { + ChipLogError(BDX, "failed to handle message: %s", ErrorStr(err)); + } + return err; +} + +void Endpoint::OnResponseTimeout(Messaging::ExchangeContext * ec) +{ + ChipLogError(BDX, "%s, ec: %d", __FUNCTION__, ec->GetExchangeId()); + mExchangeCtx = nullptr; + mTransfer.Reset(); +} + +void Endpoint::PollTimerHandler(chip::System::Layer * systemLayer, void * appState, CHIP_ERROR error) +{ + VerifyOrReturn(appState != nullptr); + static_cast(appState)->PollForOutput(); +} + +void Endpoint::PollForOutput() +{ + TransferSession::OutputEvent outEvent; + mTransfer.PollOutput(outEvent, System::Platform::Clock::GetMonotonicMilliseconds()); + HandleTransferSessionOutput(outEvent); + + DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); +} + +void Endpoint::ScheduleImmediatePoll() +{ + DeviceLayer::SystemLayer.StartTimer(kImmediatePollDelayMs, PollTimerHandler, this); +} + +CHIP_ERROR Responder::PrepareForTransfer(TransferRole role, BitFlags xferControlOpts, uint16_t maxBlockSize, + uint32_t timeoutMs, uint32_t pollFreqMs) +{ + mPollFreqMs = pollFreqMs; + ReturnErrorOnFailure(mTransfer.WaitForTransfer(role, xferControlOpts, maxBlockSize, timeoutMs)); + DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); + return CHIP_NO_ERROR; +} + +CHIP_ERROR Initiator::InitiateTransfer(TransferRole role, const TransferSession::TransferInitData & initData, uint32_t timeoutMs, + uint32_t pollFreqMs) +{ + mPollFreqMs = pollFreqMs; + ReturnErrorOnFailure(mTransfer.StartTransfer(role, initData, timeoutMs)); + DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); + return CHIP_NO_ERROR; +} + +} // namespace bdx +} // namespace chip diff --git a/src/protocols/bdx/BdxEndpoint.h b/src/protocols/bdx/BdxEndpoint.h new file mode 100644 index 00000000000000..a99f8eee6b7213 --- /dev/null +++ b/src/protocols/bdx/BdxEndpoint.h @@ -0,0 +1,124 @@ +/* + * + * Copyright (c) 2021 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. + */ + +/** + * @file BdxEndpoint.h + * + * This file defines interfaces for connecting the BDX state machine (TransferSession) to the messaging layer. + */ + +#include +#include +#include +#include +#include +#include + +#pragma once + +namespace chip { +namespace bdx { + +/** + * An abstract class with methods for handling BDX messages from an ExchangeContext and polling a TransferSession state machine. + * + * This class contains a repeating timer which regurlaly polls the TransferSession state machine. This class does not define any + * methods for beginning a transfer or initializing the underlying TransferSession object. A CHIP node may have many Endpoints but + * only one Endpoint should be used for each BDX transfer. + */ +class Endpoint : public Messaging::ExchangeDelegate +{ +public: + Endpoint() : mExchangeCtx(nullptr), mPollFreqMs(kDefaultPollFreqMs) {} + ~Endpoint() = default; + +private: + // Inherited from ExchangeContext + CHIP_ERROR OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PacketHeader & packetHeader, + const chip::PayloadHeader & payloadHeader, chip::System::PacketBufferHandle && payload) override; + void OnResponseTimeout(Messaging::ExchangeContext * ec) override; + + /** + * This method should be implemented to contain business-logic handling of BDX messages and other TransferSession events. + * + * @param[in] event An OutputEvent that contains output from the TransferSession object. + */ + virtual void HandleTransferSessionOutput(TransferSession::OutputEvent & event) = 0; + +protected: + /** + * The callback for when the poll timer expires. The poll timer regulates how often the TransferSession is polled. + */ + static void PollTimerHandler(chip::System::Layer * systemLayer, void * appState, CHIP_ERROR error); + + /** + * Polls the TransferSession object and calls HandleTransferSessionOutput. + */ + void PollForOutput(); + + /** + * Starts the poll timer with a very short timeout. + */ + void ScheduleImmediatePoll(); + + TransferSession mTransfer; + Messaging::ExchangeContext * mExchangeCtx; + uint32_t mPollFreqMs; + static constexpr uint32_t kDefaultPollFreqMs = 500; + static constexpr uint32_t kImmediatePollDelayMs = 1; +}; + +/** + * An Endpoint that is initialized to respond to an incoming BDX transfer request. + */ +class Responder : public Endpoint +{ +public: + /** + * Initialize the TransferSession state machine to be ready for an incoming transfer request, and start the polling timer. + * + * @param[in] role The role of this Endpoint: Sender or Receiver of BDX data + * @param[in] xferControlOpts Supported transfer modes (see TransferControlFlags) + * @param[in] maxBlockSize The supported maximum size of BDX Block data + * @param[in] timeoutMs The chosen timeout delay for the BDX transfer in milliseconds + * @param[in] pollFreqMs The period for the TransferSession poll timer in milliseconds + */ + CHIP_ERROR PrepareForTransfer(TransferRole role, BitFlags xferControlOpts, uint16_t maxBlockSize, + uint32_t timeoutMs, uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); +}; + +/** + * An Endpoint that initiates a BDX transfer. + */ +class Initiator : public Endpoint +{ +public: + /** + * Initialize the TransferSession state machine to prepare a transfer request message (does not send the message) and start the + * poll timer. + * + * @param[in] role The role of this Endpoint: Sender or Receiver of BDX data + * @param[in] initData Data needed for preparing a transfer request BDX message + * @param[in] timeoutMs The chosen timeout delay for the BDX transfer in milliseconds + * @param[in] pollFreqMs The period for the TransferSession poll timer in milliseconds + */ + CHIP_ERROR InitiateTransfer(TransferRole role, const TransferSession::TransferInitData & initData, uint32_t timeoutMs, + uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); +}; + +} // namespace bdx +} // namespace chip From 22ccd4bf9142b1c39d9296f0b5f7a84967affa56 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Tue, 20 Jul 2021 19:00:36 +0000 Subject: [PATCH 2/9] restyling and small comment change --- src/protocols/bdx/BdxEndpoint.cpp | 6 +++--- src/protocols/bdx/BdxEndpoint.h | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/protocols/bdx/BdxEndpoint.cpp b/src/protocols/bdx/BdxEndpoint.cpp index a8415c23640ead..61b73fc531783f 100644 --- a/src/protocols/bdx/BdxEndpoint.cpp +++ b/src/protocols/bdx/BdxEndpoint.cpp @@ -23,9 +23,8 @@ #include #include #include -#include -#include #include +#include namespace chip { namespace bdx { @@ -40,7 +39,8 @@ CHIP_ERROR Endpoint::OnMessageReceived(chip::Messaging::ExchangeContext * ec, co ChipLogDetail(BDX, "%s: message 0x%x protocol %u", __FUNCTION__, static_cast(payloadHeader.GetMessageType()), payloadHeader.GetProtocolID().GetProtocolId()); - CHIP_ERROR err = mTransfer.HandleMessageReceived(payloadHeader, std::move(payload), System::Platform::Clock::GetMonotonicMilliseconds()); + CHIP_ERROR err = + mTransfer.HandleMessageReceived(payloadHeader, std::move(payload), System::Platform::Clock::GetMonotonicMilliseconds()); if (err != CHIP_NO_ERROR) { ChipLogError(BDX, "failed to handle message: %s", ErrorStr(err)); diff --git a/src/protocols/bdx/BdxEndpoint.h b/src/protocols/bdx/BdxEndpoint.h index a99f8eee6b7213..fb81cd96446b5b 100644 --- a/src/protocols/bdx/BdxEndpoint.h +++ b/src/protocols/bdx/BdxEndpoint.h @@ -36,9 +36,10 @@ namespace bdx { /** * An abstract class with methods for handling BDX messages from an ExchangeContext and polling a TransferSession state machine. * - * This class contains a repeating timer which regurlaly polls the TransferSession state machine. This class does not define any - * methods for beginning a transfer or initializing the underlying TransferSession object. A CHIP node may have many Endpoints but - * only one Endpoint should be used for each BDX transfer. + * This class does not define any methods for beginning a transfer or initializing the underlying TransferSession object (see + * Initiator and Responder below). + * This class contains a repeating timer which regurlaly polls the TransferSession state machine. + * A CHIP node may have many Endpoints but only one Endpoint should be used for each BDX transfer. */ class Endpoint : public Messaging::ExchangeDelegate { From 4207c3766dd0fe23649dc01fa44dfb9be559a668 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Thu, 22 Jul 2021 21:27:03 +0000 Subject: [PATCH 3/9] Add CONFIG_DEVICE_LAYER compile guards around SystemLayer::StartTimer --- src/protocols/bdx/BdxEndpoint.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/protocols/bdx/BdxEndpoint.cpp b/src/protocols/bdx/BdxEndpoint.cpp index 61b73fc531783f..7292b5a0942e2c 100644 --- a/src/protocols/bdx/BdxEndpoint.cpp +++ b/src/protocols/bdx/BdxEndpoint.cpp @@ -67,12 +67,16 @@ void Endpoint::PollForOutput() mTransfer.PollOutput(outEvent, System::Platform::Clock::GetMonotonicMilliseconds()); HandleTransferSessionOutput(outEvent); +#if CONFIG_DEVICE_LAYER DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); +#endif } void Endpoint::ScheduleImmediatePoll() { +#if CONFIG_DEVICE_LAYER DeviceLayer::SystemLayer.StartTimer(kImmediatePollDelayMs, PollTimerHandler, this); +#endif } CHIP_ERROR Responder::PrepareForTransfer(TransferRole role, BitFlags xferControlOpts, uint16_t maxBlockSize, @@ -80,7 +84,9 @@ CHIP_ERROR Responder::PrepareForTransfer(TransferRole role, BitFlags Date: Thu, 22 Jul 2021 22:59:25 +0000 Subject: [PATCH 4/9] initialize with System::Layer pointer instead of using DeviceLayer --- src/protocols/bdx/BdxEndpoint.cpp | 40 +++++++++++++++++-------------- src/protocols/bdx/BdxEndpoint.h | 15 ++++++++---- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/protocols/bdx/BdxEndpoint.cpp b/src/protocols/bdx/BdxEndpoint.cpp index 7292b5a0942e2c..478f266432f865 100644 --- a/src/protocols/bdx/BdxEndpoint.cpp +++ b/src/protocols/bdx/BdxEndpoint.cpp @@ -67,37 +67,41 @@ void Endpoint::PollForOutput() mTransfer.PollOutput(outEvent, System::Platform::Clock::GetMonotonicMilliseconds()); HandleTransferSessionOutput(outEvent); -#if CONFIG_DEVICE_LAYER - DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); -#endif + VerifyOrReturn(mSystemLayer != nullptr, ChipLogError(BDX, "%s mSystemLayer is null", __FUNCTION__)); + mSystemLayer->StartTimer(mPollFreqMs, PollTimerHandler, this); } void Endpoint::ScheduleImmediatePoll() { -#if CONFIG_DEVICE_LAYER - DeviceLayer::SystemLayer.StartTimer(kImmediatePollDelayMs, PollTimerHandler, this); -#endif + VerifyOrReturn(mSystemLayer != nullptr, ChipLogError(BDX, "%s mSystemLayer is null", __FUNCTION__)); + mSystemLayer->StartTimer(kImmediatePollDelayMs, PollTimerHandler, this); } -CHIP_ERROR Responder::PrepareForTransfer(TransferRole role, BitFlags xferControlOpts, uint16_t maxBlockSize, - uint32_t timeoutMs, uint32_t pollFreqMs) +CHIP_ERROR Responder::PrepareForTransfer(System::Layer * layer, TransferRole role, BitFlags xferControlOpts, + uint16_t maxBlockSize, uint32_t timeoutMs, uint32_t pollFreqMs) { - mPollFreqMs = pollFreqMs; + VerifyOrReturnError(layer != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + + mPollFreqMs = pollFreqMs; + mSystemLayer = layer; + ReturnErrorOnFailure(mTransfer.WaitForTransfer(role, xferControlOpts, maxBlockSize, timeoutMs)); -#if CONFIG_DEVICE_LAYER - DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); -#endif + + mSystemLayer->StartTimer(mPollFreqMs, PollTimerHandler, this); return CHIP_NO_ERROR; } -CHIP_ERROR Initiator::InitiateTransfer(TransferRole role, const TransferSession::TransferInitData & initData, uint32_t timeoutMs, - uint32_t pollFreqMs) +CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData, + uint32_t timeoutMs, uint32_t pollFreqMs) { - mPollFreqMs = pollFreqMs; + VerifyOrReturnError(layer != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + + mPollFreqMs = pollFreqMs; + mSystemLayer = layer; + ReturnErrorOnFailure(mTransfer.StartTransfer(role, initData, timeoutMs)); -#if CONFIG_DEVICE_LAYER - DeviceLayer::SystemLayer.StartTimer(mPollFreqMs, PollTimerHandler, this); -#endif + + mSystemLayer->StartTimer(mPollFreqMs, PollTimerHandler, this); return CHIP_NO_ERROR; } diff --git a/src/protocols/bdx/BdxEndpoint.h b/src/protocols/bdx/BdxEndpoint.h index fb81cd96446b5b..5ec1e86fdfb2e6 100644 --- a/src/protocols/bdx/BdxEndpoint.h +++ b/src/protocols/bdx/BdxEndpoint.h @@ -44,7 +44,7 @@ namespace bdx { class Endpoint : public Messaging::ExchangeDelegate { public: - Endpoint() : mExchangeCtx(nullptr), mPollFreqMs(kDefaultPollFreqMs) {} + Endpoint() : mExchangeCtx(nullptr), mSystemLayer(nullptr), mPollFreqMs(kDefaultPollFreqMs) {} ~Endpoint() = default; private: @@ -78,6 +78,7 @@ class Endpoint : public Messaging::ExchangeDelegate TransferSession mTransfer; Messaging::ExchangeContext * mExchangeCtx; + System::Layer * mSystemLayer; uint32_t mPollFreqMs; static constexpr uint32_t kDefaultPollFreqMs = 500; static constexpr uint32_t kImmediatePollDelayMs = 1; @@ -85,6 +86,8 @@ class Endpoint : public Messaging::ExchangeDelegate /** * An Endpoint that is initialized to respond to an incoming BDX transfer request. + * + * It is intended that this class will be used as a delegate for handling an unsolicited BDX message. */ class Responder : public Endpoint { @@ -92,14 +95,15 @@ class Responder : public Endpoint /** * Initialize the TransferSession state machine to be ready for an incoming transfer request, and start the polling timer. * + * @param[in] layer A System::Layer pointer to use to start the polling timer * @param[in] role The role of this Endpoint: Sender or Receiver of BDX data * @param[in] xferControlOpts Supported transfer modes (see TransferControlFlags) * @param[in] maxBlockSize The supported maximum size of BDX Block data * @param[in] timeoutMs The chosen timeout delay for the BDX transfer in milliseconds * @param[in] pollFreqMs The period for the TransferSession poll timer in milliseconds */ - CHIP_ERROR PrepareForTransfer(TransferRole role, BitFlags xferControlOpts, uint16_t maxBlockSize, - uint32_t timeoutMs, uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); + CHIP_ERROR PrepareForTransfer(System::Layer * layer, TransferRole role, BitFlags xferControlOpts, + uint16_t maxBlockSize, uint32_t timeoutMs, uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); }; /** @@ -112,13 +116,14 @@ class Initiator : public Endpoint * Initialize the TransferSession state machine to prepare a transfer request message (does not send the message) and start the * poll timer. * + * @param[in] layer A System::Layer pointer to use to start the polling timer * @param[in] role The role of this Endpoint: Sender or Receiver of BDX data * @param[in] initData Data needed for preparing a transfer request BDX message * @param[in] timeoutMs The chosen timeout delay for the BDX transfer in milliseconds * @param[in] pollFreqMs The period for the TransferSession poll timer in milliseconds */ - CHIP_ERROR InitiateTransfer(TransferRole role, const TransferSession::TransferInitData & initData, uint32_t timeoutMs, - uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); + CHIP_ERROR InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData, + uint32_t timeoutMs, uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); }; } // namespace bdx From 2cad4cf37f0d633cedb6e2fed7dd4f437d92637c Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Thu, 22 Jul 2021 16:05:09 -0700 Subject: [PATCH 5/9] remove unnecessary chip::bdx:: prefix Co-authored-by: Boris Zbarsky --- src/protocols/bdx/BdxEndpoint.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocols/bdx/BdxEndpoint.cpp b/src/protocols/bdx/BdxEndpoint.cpp index 478f266432f865..88a3e36e131b2c 100644 --- a/src/protocols/bdx/BdxEndpoint.cpp +++ b/src/protocols/bdx/BdxEndpoint.cpp @@ -58,7 +58,7 @@ void Endpoint::OnResponseTimeout(Messaging::ExchangeContext * ec) void Endpoint::PollTimerHandler(chip::System::Layer * systemLayer, void * appState, CHIP_ERROR error) { VerifyOrReturn(appState != nullptr); - static_cast(appState)->PollForOutput(); + static_cast(appState)->PollForOutput(); } void Endpoint::PollForOutput() From b1e90194828d1e60f0fb24c6d2d2038528754fe9 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 23 Jul 2021 17:39:42 +0000 Subject: [PATCH 6/9] Rename Endpoint to TransferFacilitator --- src/protocols/bdx/BUILD.gn | 4 +-- ...dxEndpoint.cpp => TransferFacilitator.cpp} | 17 +++++----- .../{BdxEndpoint.h => TransferFacilitator.h} | 31 +++++++++++-------- 3 files changed, 29 insertions(+), 23 deletions(-) rename src/protocols/bdx/{BdxEndpoint.cpp => TransferFacilitator.cpp} (82%) rename src/protocols/bdx/{BdxEndpoint.h => TransferFacilitator.h} (77%) diff --git a/src/protocols/bdx/BUILD.gn b/src/protocols/bdx/BUILD.gn index 2b26e4ee9645f6..5467a3414f3e09 100644 --- a/src/protocols/bdx/BUILD.gn +++ b/src/protocols/bdx/BUILD.gn @@ -18,8 +18,8 @@ static_library("bdx") { output_name = "libBdx" sources = [ - "BdxEndpoint.cpp", - "BdxEndpoint.h", + "TransferFacilitator.cpp", + "TransferFacilitator.h", "BdxMessages.cpp", "BdxMessages.h", "BdxTransferSession.cpp", diff --git a/src/protocols/bdx/BdxEndpoint.cpp b/src/protocols/bdx/TransferFacilitator.cpp similarity index 82% rename from src/protocols/bdx/BdxEndpoint.cpp rename to src/protocols/bdx/TransferFacilitator.cpp index 88a3e36e131b2c..922ada8245c58b 100644 --- a/src/protocols/bdx/BdxEndpoint.cpp +++ b/src/protocols/bdx/TransferFacilitator.cpp @@ -15,7 +15,7 @@ * limitations under the License. */ -#include "BdxEndpoint.h" +#include "TransferFacilitator.h" #include #include @@ -29,8 +29,9 @@ namespace chip { namespace bdx { -CHIP_ERROR Endpoint::OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PacketHeader & packetHeader, - const chip::PayloadHeader & payloadHeader, chip::System::PacketBufferHandle && payload) +CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PacketHeader & packetHeader, + const chip::PayloadHeader & payloadHeader, + chip::System::PacketBufferHandle && payload) { if (mExchangeCtx == nullptr) { @@ -48,20 +49,20 @@ CHIP_ERROR Endpoint::OnMessageReceived(chip::Messaging::ExchangeContext * ec, co return err; } -void Endpoint::OnResponseTimeout(Messaging::ExchangeContext * ec) +void TransferFacilitator::OnResponseTimeout(Messaging::ExchangeContext * ec) { ChipLogError(BDX, "%s, ec: %d", __FUNCTION__, ec->GetExchangeId()); mExchangeCtx = nullptr; mTransfer.Reset(); } -void Endpoint::PollTimerHandler(chip::System::Layer * systemLayer, void * appState, CHIP_ERROR error) +void TransferFacilitator::PollTimerHandler(chip::System::Layer * systemLayer, void * appState, CHIP_ERROR error) { VerifyOrReturn(appState != nullptr); - static_cast(appState)->PollForOutput(); + static_cast(appState)->PollForOutput(); } -void Endpoint::PollForOutput() +void TransferFacilitator::PollForOutput() { TransferSession::OutputEvent outEvent; mTransfer.PollOutput(outEvent, System::Platform::Clock::GetMonotonicMilliseconds()); @@ -71,7 +72,7 @@ void Endpoint::PollForOutput() mSystemLayer->StartTimer(mPollFreqMs, PollTimerHandler, this); } -void Endpoint::ScheduleImmediatePoll() +void TransferFacilitator::ScheduleImmediatePoll() { VerifyOrReturn(mSystemLayer != nullptr, ChipLogError(BDX, "%s mSystemLayer is null", __FUNCTION__)); mSystemLayer->StartTimer(kImmediatePollDelayMs, PollTimerHandler, this); diff --git a/src/protocols/bdx/BdxEndpoint.h b/src/protocols/bdx/TransferFacilitator.h similarity index 77% rename from src/protocols/bdx/BdxEndpoint.h rename to src/protocols/bdx/TransferFacilitator.h index 5ec1e86fdfb2e6..95b11da4339bc6 100644 --- a/src/protocols/bdx/BdxEndpoint.h +++ b/src/protocols/bdx/TransferFacilitator.h @@ -39,13 +39,13 @@ namespace bdx { * This class does not define any methods for beginning a transfer or initializing the underlying TransferSession object (see * Initiator and Responder below). * This class contains a repeating timer which regurlaly polls the TransferSession state machine. - * A CHIP node may have many Endpoints but only one Endpoint should be used for each BDX transfer. + * A CHIP node may have many TransferFacilitator instances but only one TransferFacilitator should be used for each BDX transfer. */ -class Endpoint : public Messaging::ExchangeDelegate +class TransferFacilitator : public Messaging::ExchangeDelegate { public: - Endpoint() : mExchangeCtx(nullptr), mSystemLayer(nullptr), mPollFreqMs(kDefaultPollFreqMs) {} - ~Endpoint() = default; + TransferFacilitator() : mExchangeCtx(nullptr), mSystemLayer(nullptr), mPollFreqMs(kDefaultPollFreqMs) {} + ~TransferFacilitator() = default; private: // Inherited from ExchangeContext @@ -85,31 +85,36 @@ class Endpoint : public Messaging::ExchangeDelegate }; /** - * An Endpoint that is initialized to respond to an incoming BDX transfer request. + * A TransferFacilitator that is initialized to respond to an incoming BDX transfer request. * - * It is intended that this class will be used as a delegate for handling an unsolicited BDX message. + * Provides a method for initializing the TransferSession member but still needs to be extended to implement + * HandleTransferSessionOutput. It is intended that this class will be used as a delegate for handling an unsolicited BDX message. */ -class Responder : public Endpoint +class Responder : public TransferFacilitator { public: /** * Initialize the TransferSession state machine to be ready for an incoming transfer request, and start the polling timer. * * @param[in] layer A System::Layer pointer to use to start the polling timer - * @param[in] role The role of this Endpoint: Sender or Receiver of BDX data + * @param[in] role The role of the Responder: Sender or Receiver of BDX data * @param[in] xferControlOpts Supported transfer modes (see TransferControlFlags) * @param[in] maxBlockSize The supported maximum size of BDX Block data * @param[in] timeoutMs The chosen timeout delay for the BDX transfer in milliseconds * @param[in] pollFreqMs The period for the TransferSession poll timer in milliseconds */ CHIP_ERROR PrepareForTransfer(System::Layer * layer, TransferRole role, BitFlags xferControlOpts, - uint16_t maxBlockSize, uint32_t timeoutMs, uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); + uint16_t maxBlockSize, uint32_t timeoutMs, + uint32_t pollFreqMs = TransferFacilitator::kDefaultPollFreqMs); }; /** - * An Endpoint that initiates a BDX transfer. + * A TransferFacilitator that initiates a BDX transfer. + * + * Provides a method for initializing the TransferSession member (thus beginning the transfer) but still needs to be extended to + * implement HandleTransferSessionOutput. */ -class Initiator : public Endpoint +class Initiator : public TransferFacilitator { public: /** @@ -117,13 +122,13 @@ class Initiator : public Endpoint * poll timer. * * @param[in] layer A System::Layer pointer to use to start the polling timer - * @param[in] role The role of this Endpoint: Sender or Receiver of BDX data + * @param[in] role The role of the Initiator: Sender or Receiver of BDX data * @param[in] initData Data needed for preparing a transfer request BDX message * @param[in] timeoutMs The chosen timeout delay for the BDX transfer in milliseconds * @param[in] pollFreqMs The period for the TransferSession poll timer in milliseconds */ CHIP_ERROR InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData, - uint32_t timeoutMs, uint32_t pollFreqMs = Endpoint::kDefaultPollFreqMs); + uint32_t timeoutMs, uint32_t pollFreqMs = TransferFacilitator::kDefaultPollFreqMs); }; } // namespace bdx From 2d3ede9b37ad97a8b683973fcaa37c60f91696ed Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 30 Jul 2021 22:07:18 +0000 Subject: [PATCH 7/9] call WillSendMessage() for every received BDX message --- src/protocols/bdx/TransferFacilitator.cpp | 7 +++++++ src/protocols/bdx/TransferFacilitator.h | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp index 922ada8245c58b..dff66a518159d1 100644 --- a/src/protocols/bdx/TransferFacilitator.cpp +++ b/src/protocols/bdx/TransferFacilitator.cpp @@ -46,6 +46,13 @@ CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeConte { ChipLogError(BDX, "failed to handle message: %s", ErrorStr(err)); } + + // Almost every BDX message will follow up with a response on the exchange. Even messages that might signify the end of a + // transfer could necessitate a response if they are received at the wrong time. + // For this reason, it is left up to the application logic to call ExchangeContext::Close() when it has determined that the + // transfer is finished. + mExchangeCtx->WillSendMessage(); + return err; } diff --git a/src/protocols/bdx/TransferFacilitator.h b/src/protocols/bdx/TransferFacilitator.h index 95b11da4339bc6..411900d4390d4f 100644 --- a/src/protocols/bdx/TransferFacilitator.h +++ b/src/protocols/bdx/TransferFacilitator.h @@ -56,6 +56,9 @@ class TransferFacilitator : public Messaging::ExchangeDelegate /** * This method should be implemented to contain business-logic handling of BDX messages and other TransferSession events. * + * NOTE: It is the responsiblity of the implementer to Close the underlying ExchangeContext when it has determined that the + * tranfser is finished. This class assumes that a response message will be sent for all received messages. + * * @param[in] event An OutputEvent that contains output from the TransferSession object. */ virtual void HandleTransferSessionOutput(TransferSession::OutputEvent & event) = 0; From ef4b6a087ee8115756a9896fb38f9ba41c0f0a92 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 30 Jul 2021 22:15:59 +0000 Subject: [PATCH 8/9] restyle BUILD.gn --- src/protocols/bdx/BUILD.gn | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protocols/bdx/BUILD.gn b/src/protocols/bdx/BUILD.gn index 5467a3414f3e09..04bea237300fb3 100644 --- a/src/protocols/bdx/BUILD.gn +++ b/src/protocols/bdx/BUILD.gn @@ -18,12 +18,12 @@ static_library("bdx") { output_name = "libBdx" sources = [ - "TransferFacilitator.cpp", - "TransferFacilitator.h", "BdxMessages.cpp", "BdxMessages.h", "BdxTransferSession.cpp", "BdxTransferSession.h", + "TransferFacilitator.cpp", + "TransferFacilitator.h", ] cflags = [ "-Wconversion" ] From 39be8b65a22af0601f64ab16cad44e20f3c09ac9 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 30 Jul 2021 15:20:06 -0700 Subject: [PATCH 9/9] fix comment spelling Co-authored-by: Boris Zbarsky --- src/protocols/bdx/TransferFacilitator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocols/bdx/TransferFacilitator.h b/src/protocols/bdx/TransferFacilitator.h index 411900d4390d4f..bc0b3a291c27a8 100644 --- a/src/protocols/bdx/TransferFacilitator.h +++ b/src/protocols/bdx/TransferFacilitator.h @@ -57,7 +57,7 @@ class TransferFacilitator : public Messaging::ExchangeDelegate * This method should be implemented to contain business-logic handling of BDX messages and other TransferSession events. * * NOTE: It is the responsiblity of the implementer to Close the underlying ExchangeContext when it has determined that the - * tranfser is finished. This class assumes that a response message will be sent for all received messages. + * transfer is finished. This class assumes that a response message will be sent for all received messages. * * @param[in] event An OutputEvent that contains output from the TransferSession object. */