From 34004c2c57fefaec7cbf01f7694adb40604c6c98 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 11 Dec 2018 11:00:26 -0800 Subject: [PATCH] [posix-app] adopt new HDLC buffer models (#3371) This commit updates the `HdlcInterface` and `RadioSpinel` to use the new HDLC buffer model. In particular, for received spinel frames the decoder uses an `Hdlc::MultiFrameBuffer` which is capable of storing multiple frames in a FIFO queue manner. With this model, the received and decoded spinel frames are directly placed in the FIFO buffer. This allows `RadioSpinel` code during `WaitResponse()` (when waiting for specific Spinel response frame) to simply save/keep a notification frame in the queue buffer (without requiring to copy the frame) so that it can read and process the saved frame later. If a received frame can be processed at the time, the frame is then simply discarded from the queue buffer. --- Android.mk | 1 - src/posix/platform/Makefile.am | 15 +- src/posix/platform/frame_queue.cpp | 245 -------------------------- src/posix/platform/frame_queue.hpp | 111 ------------ src/posix/platform/hdlc_interface.cpp | 53 +++--- src/posix/platform/hdlc_interface.hpp | 41 ++++- src/posix/platform/radio_spinel.cpp | 96 +++++++--- src/posix/platform/radio_spinel.hpp | 15 +- 8 files changed, 126 insertions(+), 451 deletions(-) delete mode 100644 src/posix/platform/frame_queue.cpp delete mode 100644 src/posix/platform/frame_queue.hpp diff --git a/Android.mk b/Android.mk index 096a36749..6f734108f 100644 --- a/Android.mk +++ b/Android.mk @@ -205,7 +205,6 @@ LOCAL_SRC_FILES := \ src/ncp/spinel_decoder.cpp \ src/ncp/spinel_encoder.cpp \ src/posix/platform/alarm.c \ - src/posix/platform/frame_queue.cpp \ src/posix/platform/hdlc_interface.cpp \ src/posix/platform/logging.c \ src/posix/platform/misc.c \ diff --git a/src/posix/platform/Makefile.am b/src/posix/platform/Makefile.am index b69f7a6b5..2a3b9e2a7 100644 --- a/src/posix/platform/Makefile.am +++ b/src/posix/platform/Makefile.am @@ -41,7 +41,6 @@ libopenthread_posix_a_CPPFLAGS = \ libopenthread_posix_a_SOURCES = \ alarm.c \ - frame_queue.cpp \ hdlc_interface.cpp \ logging.c \ misc.c \ @@ -73,7 +72,6 @@ libopenthread_posix_a_SOURCES += \ endif noinst_HEADERS = \ - frame_queue.hpp \ openthread-system.h \ platform-posix.h \ hdlc_interface.hpp \ @@ -89,17 +87,7 @@ if OPENTHREAD_BUILD_COVERAGE CLEANFILES = $(wildcard *.gcda *.gcno) endif # OPENTHREAD_BUILD_COVERAGE -check_PROGRAMS = test-frame-queue test-settings - -test_frame_queue_CPPFLAGS = \ - -I$(top_srcdir)/include \ - -I$(top_srcdir)/src/core \ - -DSELF_TEST \ - $(NULL) - -test_frame_queue_SOURCES = \ - frame_queue.cpp \ - $(NULL) +check_PROGRAMS = test-settings test_settings_CPPFLAGS = \ -I$(top_srcdir)/include \ @@ -112,7 +100,6 @@ test_settings_SOURCES = \ $(NULL) TESTS = \ - test-frame-queue \ test-settings \ $(NULL) diff --git a/src/posix/platform/frame_queue.cpp b/src/posix/platform/frame_queue.cpp deleted file mode 100644 index 783e71a50..000000000 --- a/src/posix/platform/frame_queue.cpp +++ /dev/null @@ -1,245 +0,0 @@ -/* - * Copyright (c) 2018, The OpenThread Authors. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of the copyright holder nor the - * names of its contributors may be used to endorse or promote products - * derived from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -/** - * @file - * This file includes implementation of frame queue. - */ - -#include "platform-posix.h" - -#include "frame_queue.hpp" - -#include -#include - -#include - -namespace ot { - -otError FrameQueue::Push(const uint8_t *aFrame, uint8_t aLength) -{ - otError error = OT_ERROR_NONE; - uint16_t newTail = mTail + aLength + 1; - - assert(aFrame != NULL); - VerifyOrExit(aFrame != NULL, error = OT_ERROR_INVALID_ARGS); - - if (mHead > mTail) - { - VerifyOrExit(newTail < mHead, error = OT_ERROR_NO_BUFS); - } - else if (newTail >= sizeof(mBuffer)) - { - newTail -= sizeof(mBuffer); - VerifyOrExit(newTail < mHead, error = OT_ERROR_NO_BUFS); - } - - mBuffer[mTail] = aLength; - - if (newTail > mTail) - { - memcpy(mBuffer + mTail + 1, aFrame, aLength); - } - else - { - uint16_t half = (sizeof(mBuffer) - mTail - 1); - memcpy(mBuffer + mTail + 1, aFrame, half); - memcpy(mBuffer, aFrame + half, (aLength - half)); - } - - mTail = newTail; - -exit: - return error; -} - -const uint8_t *FrameQueue::Shift(uint8_t *aFrame, uint8_t &aLength) -{ - const uint8_t *frame = NULL; - uint16_t next; - - VerifyOrExit(mHead != mTail); - - aLength = mBuffer[mHead]; - next = mHead + 1 + aLength; - if (next >= sizeof(mBuffer)) - { - uint16_t half = sizeof(mBuffer) - mHead - 1; - memcpy(aFrame, mBuffer + mHead + 1, half); - memcpy(aFrame + half, mBuffer, aLength - half); - frame = aFrame; - next -= sizeof(mBuffer); - } - else - { - frame = mBuffer + mHead + 1; - } - mHead = next; - -exit: - return frame; -} - -} // namespace ot - -#if SELF_TEST -#include -#include - -void TestSingle() -{ - otError error; - ot::FrameQueue frameQueue; - uint8_t length; - uint8_t frame[255]; - - for (size_t i = 0; i < sizeof(frame); ++i) - { - frame[i] = i; - } - - for (size_t i = 0; i < sizeof(frame); ++i) - { - uint8_t outFrame[255]; - const uint8_t *retFrame = NULL; - error = frameQueue.Push(frame, i); - assert(OT_ERROR_NONE == error); - assert(!frameQueue.IsEmpty()); - retFrame = frameQueue.Shift(outFrame, length); - assert(retFrame != NULL); - assert(length == i); - - for (size_t j = 0; j < i; ++j) - { - assert(retFrame[j] == frame[j]); - } - - assert(frameQueue.IsEmpty()); - } -} - -void TestMultiple() -{ - otError error; - ot::FrameQueue frameQueue; - uint8_t length; - uint8_t frame[255]; - int count = 0; - - for (size_t i = 0; i < sizeof(frame); ++i) - { - frame[i] = i; - } - - srand(0); - - for (size_t i = 0; i < sizeof(frame); ++i) - { - uint8_t outFrame[255]; - int action = rand(); - - if (action & 0x01) // push when odd - { - error = frameQueue.Push(frame, i); - if (error == OT_ERROR_NO_BUFS) - { - continue; - } - - assert(OT_ERROR_NONE == error); - assert(!frameQueue.IsEmpty()); - ++count; - } - else - { - const uint8_t *retFrame = NULL; - - retFrame = frameQueue.Shift(outFrame, length); - - if (count == 0) - { - assert(retFrame == NULL); - continue; - } - else - { - assert(retFrame != NULL); - } - - for (size_t j = 0; j < length; ++j) - { - assert(retFrame[j] == frame[j]); - } - - --count; - } - } -} - -void TestRing() -{ - ot::FrameQueue frameQueue; - uint8_t length; - uint8_t frame[255]; - - for (size_t i = 0; i < sizeof(frame); ++i) - { - frame[i] = i; - } - - for (size_t i = 0; i < OPENTHREAD_CONFIG_FRAME_QUEUE_SIZE + 255; i += sizeof(frame)) - { - uint8_t outFrame[255]; - const uint8_t *retFrame = NULL; - - frameQueue.Push(frame, sizeof(frame)); - - retFrame = frameQueue.Shift(outFrame, length); - - for (size_t j = 0; j < sizeof(frame); ++j) - { - assert(retFrame[j] == frame[j]); - } - }; -} - -void RunAllTests() -{ - TestSingle(); - TestMultiple(); - TestRing(); -} - -int main(void) -{ - RunAllTests(); - printf("All tests passed\n"); - return 0; -} -#endif // SELF_TEST diff --git a/src/posix/platform/frame_queue.hpp b/src/posix/platform/frame_queue.hpp deleted file mode 100644 index b77e338da..000000000 --- a/src/posix/platform/frame_queue.hpp +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright (c) 2018, The OpenThread Authors. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of the copyright holder nor the - * names of its contributors may be used to endorse or promote products - * derived from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -/** - * @file - * This file includes definitions for frame queue. - */ - -#ifndef OT_FRAME_QUEUE_HPP_ -#define OT_FRAME_QUEUE_HPP_ - -#include - -/** - * @def OPENTHREAD_CONFIG_FRAME_QUEUE_SIZE - * - * The size of a frame queue in bytes. - * - */ -#ifndef OPENTHREAD_CONFIG_FRAME_QUEUE_SIZE -#define OPENTHREAD_CONFIG_FRAME_QUEUE_SIZE 4096 -#endif - -namespace ot { - -class FrameQueue -{ -public: - /** - * This constructor initializes a frame queue based on ring buffer. - * - */ - FrameQueue(void) - : mHead(0) - , mTail(0) - { - } - - /** - * This method checks if the cache is empty. - * - * @retval true No frames are queued. - * @retval false At least one frame is queued. - * - */ - bool IsEmpty(void) const { return mHead == mTail; } - - /** - * This method pushes one frame into the queue. - * - * @param[in] aFrame A pointer to a spinel frame to be queued. - * @param[in] aLength Frame length in bytes. - * - * @retval OT_ERROR_NONE Successfully queued this frame. - * @retval OT_ERROR_NO_BUFS Insufficient memory for this frame. - * - */ - otError Push(const uint8_t *aFrame, uint8_t aLength); - - /** - * This method shifts one frame at head. - * - * @note aFrame is only used when necessary, always use the returned pointer to access frame data. - * - * @param[out] aFrame A pointer to the frame to receive the data. - * @param[out] aLength A reference to receive the frame length. - * - * @return A pointer to the frame. - * - */ - const uint8_t *Shift(uint8_t *aFrame, uint8_t &aLength); - -private: - enum - { - kQueueSize = OPENTHREAD_CONFIG_FRAME_QUEUE_SIZE, - }; - - uint8_t mBuffer[kQueueSize]; - uint16_t mHead; - uint16_t mTail; -}; - -} // namespace ot - -#endif // OT_FRAME_CACHE_HPP_ diff --git a/src/posix/platform/hdlc_interface.cpp b/src/posix/platform/hdlc_interface.cpp index c1c94ac07..d531d965b 100644 --- a/src/posix/platform/hdlc_interface.cpp +++ b/src/posix/platform/hdlc_interface.cpp @@ -66,27 +66,12 @@ namespace ot { namespace PosixApp { -class EncoderBuffer : public Hdlc::Encoder::BufferWriteIterator -{ -public: - EncoderBuffer(void) - { - mWritePointer = mBuffer; - mRemainingLength = sizeof(mBuffer); - } - - uint16_t GetLength(void) const { return static_cast(mWritePointer - mBuffer); } - const uint8_t *GetBuffer(void) const { return mBuffer; } - -private: - uint8_t mBuffer[HdlcInterface::kMaxFrameSize]; -}; - HdlcInterface::HdlcInterface(Callbacks &aCallbacks) : mCallbacks(aCallbacks) , mSockFd(-1) , mIsDecoding(false) - , mHdlcDecoder(mDecoderBuffer, sizeof(mDecoderBuffer), HandleHdlcFrame, HandleHdlcError, this) + , mRxFrameBuffer() + , mHdlcDecoder(mRxFrameBuffer, HandleHdlcFrame, this) { } @@ -166,15 +151,15 @@ void HdlcInterface::Decode(const uint8_t *aBuffer, uint16_t aLength) otError HdlcInterface::SendFrame(const uint8_t *aFrame, uint16_t aLength) { - otError error = OT_ERROR_NONE; - Hdlc::Encoder hdlcEncoder; - EncoderBuffer encoderBuffer; + otError error = OT_ERROR_NONE; + Hdlc::FrameBuffer encoderBuffer; + Hdlc::Encoder hdlcEncoder(encoderBuffer); - SuccessOrExit(error = hdlcEncoder.Init(encoderBuffer)); - SuccessOrExit(error = hdlcEncoder.Encode(aFrame, aLength, encoderBuffer)); - SuccessOrExit(error = hdlcEncoder.Finalize(encoderBuffer)); + SuccessOrExit(error = hdlcEncoder.BeginFrame()); + SuccessOrExit(error = hdlcEncoder.Encode(aFrame, aLength)); + SuccessOrExit(error = hdlcEncoder.EndFrame()); - error = Write(encoderBuffer.GetBuffer(), encoderBuffer.GetLength()); + error = Write(encoderBuffer.GetFrame(), encoderBuffer.GetLength()); exit: return error; @@ -501,19 +486,21 @@ exit: } #endif // OPENTHREAD_CONFIG_POSIX_APP_ENABLE_PTY_DEVICE -void HdlcInterface::HandleHdlcFrame(void *aContext, uint8_t *aFrame, uint16_t aFrameLength) +void HdlcInterface::HandleHdlcFrame(void *aContext, otError aError) { - static_cast(aContext)->mCallbacks.HandleReceivedFrame(aFrame, aFrameLength); + static_cast(aContext)->HandleHdlcFrame(aError); } -void HdlcInterface::HandleHdlcError(void *aContext, otError aError, uint8_t *aFrame, uint16_t aFrameLength) +void HdlcInterface::HandleHdlcFrame(otError aError) { - OT_UNUSED_VARIABLE(aContext); - OT_UNUSED_VARIABLE(aError); - OT_UNUSED_VARIABLE(aFrame); - OT_UNUSED_VARIABLE(aFrameLength); - - otLogWarnPlat("Error decoding hdlc frame: %s", otThreadErrorToString(aError)); + if (aError == OT_ERROR_NONE) + { + mCallbacks.HandleReceivedFrame(*this); + } + else + { + otLogWarnPlat("Error decoding hdlc frame: %s", otThreadErrorToString(aError)); + } } } // namespace PosixApp diff --git a/src/posix/platform/hdlc_interface.hpp b/src/posix/platform/hdlc_interface.hpp index bd39370e4..04a090bea 100644 --- a/src/posix/platform/hdlc_interface.hpp +++ b/src/posix/platform/hdlc_interface.hpp @@ -54,6 +54,15 @@ public: kMaxWaitTime = 2000, ///< Maximum wait time in Milliseconds for socket to become writable (see `SendFrame`). }; + /** + * This type defines a receive frame buffer to store received (and decoded) frame(s). + * + * @note The receive frame buffer is an `Hdlc::MultiFrameBuffer` and therefore it is capable of storing multiple + * frames in a FIFO queue manner. + * + */ + typedef Hdlc::MultiFrameBuffer RxFrameBuffer; + /** * This class defines the callbacks provided by `HdlcInterfac` to its owner/user. * @@ -64,11 +73,15 @@ public: /** * This callback is invoked to notify owner/user of `HdlcInterface` of a received (and decoded) frame. * - * @param[in] aFrame A pointer to buffer containing the received frame. - * @param[in] aLength The length (number of bytes) of the received frame. + * The newly received frame is available in `RxFrameBuffer` from `HdclInterface::GetRxFrameBuffer()`. The + * user can read and process the frame. The callback is expected to either discard the new frame using + * `RxFrameBuffer::DiscardFrame()` or save the frame using `RxFrameBuffer::SaveFrame()` to be read and + * processed later. + * + * @param[in] aHdlcInterface A reference to the `HdlcInterface` object. * */ - void HandleReceivedFrame(const uint8_t *aFrame, uint16_t aLength); + void HandleReceivedFrame(HdlcInterface &aHdlcInterface); }; /** @@ -133,6 +146,22 @@ public: */ void Read(void); + /** + * This method gets the `RxFrameBuffer`. + * + * The receive frame buffer is an `Hdlc::MultiFrameBuffer` and therefore it is capable of storing multiple + * frames in a FIFO queue manner. The `RxFrameBuffer` contains the decoded received frames. + * + * Wen during `Read()` the `Callbacks::HandleReceivedFrame()` is invoked, the newly received decoded frame is + * available in the receive frame buffer. The callback is expected to either process and then discard the frame + * (using `RxFrameBuffer::DiscardFrame()` method) or save the frame (using `RxFrameBuffer::SaveFrame()` so that + * it can be read later. + * + * @returns A reference to receive frame buffer containing newly received frame or previously saved frames. + * + */ + RxFrameBuffer &GetRxFrameBuffer(void) { return mRxFrameBuffer; } + /** * This method encodes and sends a frame to Radio Co-processor (RCP) over the socket. * @@ -201,8 +230,8 @@ private: */ void Decode(const uint8_t *aBuffer, uint16_t aLength); - static void HandleHdlcFrame(void *aContext, uint8_t *aFrame, uint16_t aFrameLength); - static void HandleHdlcError(void *aContext, otError aError, uint8_t *aFrame, uint16_t aFrameLength); + static void HandleHdlcFrame(void *aContext, otError aError); + void HandleHdlcFrame(otError aError); static int OpenFile(const char *aFile, const char *aConfig); #if OPENTHREAD_CONFIG_POSIX_APP_ENABLE_PTY_DEVICE @@ -212,8 +241,8 @@ private: Callbacks & mCallbacks; int mSockFd; bool mIsDecoding; + RxFrameBuffer mRxFrameBuffer; Hdlc::Decoder mHdlcDecoder; - uint8_t mDecoderBuffer[kMaxFrameSize]; }; } // namespace PosixApp diff --git a/src/posix/platform/radio_spinel.cpp b/src/posix/platform/radio_spinel.cpp index e274eb767..1aacfd8da 100644 --- a/src/posix/platform/radio_spinel.cpp +++ b/src/posix/platform/radio_spinel.cpp @@ -144,9 +144,9 @@ static void LogIfFail(const char *aText, otError aError) } } -void HdlcInterface::Callbacks::HandleReceivedFrame(const uint8_t *aBuffer, uint16_t aLength) +void HdlcInterface::Callbacks::HandleReceivedFrame(HdlcInterface &aInterface) { - static_cast(this)->HandleSpinelFrame(aBuffer, aLength); + static_cast(this)->HandleSpinelFrame(aInterface.GetRxFrameBuffer()); } RadioSpinel::RadioSpinel(void) @@ -300,31 +300,37 @@ void RadioSpinel::Deinit(void) mHdlcInterface.Deinit(); } -void RadioSpinel::HandleSpinelFrame(const uint8_t *aBuffer, uint16_t aLength) +void RadioSpinel::HandleSpinelFrame(HdlcInterface::RxFrameBuffer &aFrameBuffer) { otError error = OT_ERROR_NONE; uint8_t header; - spinel_ssize_t rval; + spinel_ssize_t unpacked; - rval = spinel_datatype_unpack(aBuffer, aLength, "C", &header); + unpacked = spinel_datatype_unpack(aFrameBuffer.GetFrame(), aFrameBuffer.GetLength(), "C", &header); - VerifyOrExit(rval > 0 && (header & SPINEL_HEADER_FLAG) == SPINEL_HEADER_FLAG && SPINEL_HEADER_GET_IID(header) == 0, + VerifyOrExit(unpacked > 0 && (header & SPINEL_HEADER_FLAG) == SPINEL_HEADER_FLAG && + SPINEL_HEADER_GET_IID(header) == 0, error = OT_ERROR_PARSE); if (SPINEL_HEADER_GET_TID(header) == 0) { - HandleNotification(aBuffer, aLength); + HandleNotification(aFrameBuffer); } else { - HandleResponse(aBuffer, aLength); + HandleResponse(aFrameBuffer.GetFrame(), aFrameBuffer.GetLength()); + aFrameBuffer.DiscardFrame(); } exit: - LogIfFail("Error handling hdlc frame", error); + if (error != OT_ERROR_NONE) + { + aFrameBuffer.DiscardFrame(); + otLogWarnPlat("Error handling hdlc frame: %s", otThreadErrorToString(error)); + } } -void RadioSpinel::HandleNotification(const uint8_t *aBuffer, uint16_t aLength) +void RadioSpinel::HandleNotification(HdlcInterface::RxFrameBuffer &aFrameBuffer) { spinel_prop_key_t key; spinel_size_t len = 0; @@ -332,9 +338,11 @@ void RadioSpinel::HandleNotification(const uint8_t *aBuffer, uint16_t aLength) uint8_t * data = NULL; uint32_t cmd; uint8_t header; - otError error = OT_ERROR_NONE; + otError error = OT_ERROR_NONE; + bool shouldSaveFrame = false; - unpacked = spinel_datatype_unpack(aBuffer, aLength, "CiiD", &header, &cmd, &key, &data, &len); + unpacked = spinel_datatype_unpack(aFrameBuffer.GetFrame(), aFrameBuffer.GetLength(), "CiiD", &header, &cmd, &key, + &data, &len); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(SPINEL_HEADER_GET_TID(header) == 0, error = OT_ERROR_PARSE); @@ -347,9 +355,7 @@ void RadioSpinel::HandleNotification(const uint8_t *aBuffer, uint16_t aLength) if (!IsSafeToHandleNow(key)) { - assert(aLength <= 255); - error = mFrameQueue.Push(aBuffer, static_cast(aLength)); - ExitNow(); + ExitNow(shouldSaveFrame = true); } HandleValueIs(key, data, static_cast(len)); @@ -365,9 +371,38 @@ void RadioSpinel::HandleNotification(const uint8_t *aBuffer, uint16_t aLength) } exit: + if (shouldSaveFrame) + { + aFrameBuffer.SaveFrame(); + } + else + { + aFrameBuffer.DiscardFrame(); + } + LogIfFail("Error processing notification", error); } +void RadioSpinel::HandleNotification(const uint8_t *aFrame, uint16_t aLength) +{ + spinel_prop_key_t key; + spinel_size_t len = 0; + spinel_ssize_t unpacked; + uint8_t * data = NULL; + uint32_t cmd; + uint8_t header; + otError error = OT_ERROR_NONE; + + unpacked = spinel_datatype_unpack(aFrame, aLength, "CiiD", &header, &cmd, &key, &data, &len); + VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); + VerifyOrExit(SPINEL_HEADER_GET_TID(header) == 0, error = OT_ERROR_PARSE); + VerifyOrExit(cmd == SPINEL_CMD_PROP_VALUE_IS); + HandleValueIs(key, data, static_cast(len)); + +exit: + LogIfFail("Error processing saved notification", error); +} + void RadioSpinel::HandleResponse(const uint8_t *aBuffer, uint16_t aLength) { spinel_prop_key_t key; @@ -593,11 +628,10 @@ exit: void RadioSpinel::ProcessFrameQueue(void) { - uint8_t length; - uint8_t buffer[kMaxSpinelFrame]; - const uint8_t *frame; + uint8_t *frame; + uint16_t length; - while ((frame = mFrameQueue.Shift(buffer, length)) != NULL) + while (mHdlcInterface.GetRxFrameBuffer().ReadSavedFrame(frame, length) == OT_ERROR_NONE) { HandleNotification(frame, length); } @@ -652,7 +686,7 @@ void RadioSpinel::UpdateFdSet(fd_set &aReadFdSet, fd_set &aWriteFdSet, int &aMax FD_SET(sockFd, &aWriteFdSet); } - if (!mFrameQueue.IsEmpty() || (mState == kStateTransmitDone)) + if (mHdlcInterface.GetRxFrameBuffer().HasSavedFrame() || (mState == kStateTransmitDone)) { aTimeout.tv_sec = 0; aTimeout.tv_usec = 0; @@ -661,18 +695,20 @@ void RadioSpinel::UpdateFdSet(fd_set &aReadFdSet, fd_set &aWriteFdSet, int &aMax void RadioSpinel::Process(const fd_set &aReadFdSet, const fd_set &aWriteFdSet) { - if (FD_ISSET(mHdlcInterface.GetSocket(), &aReadFdSet) || !mFrameQueue.IsEmpty()) + if (mHdlcInterface.GetRxFrameBuffer().HasSavedFrame()) { - // Handle frames received during WaitResponse() + // Handle frames received and saved during `WaitResponse()` ProcessFrameQueue(); - - if (FD_ISSET(mHdlcInterface.GetSocket(), &aReadFdSet)) - { - mHdlcInterface.Read(); - ProcessFrameQueue(); - } } + if (FD_ISSET(mHdlcInterface.GetSocket(), &aReadFdSet)) + { + mHdlcInterface.Read(); + ProcessFrameQueue(); + } + + mHdlcInterface.GetRxFrameBuffer().ClearReadFrames(); + if (mState == kStateTransmitDone) { mState = kStateReceive; @@ -1509,7 +1545,7 @@ int8_t otPlatRadioGetReceiveSensitivity(otInstance *aInstance) #if OPENTHREAD_POSIX_VIRTUAL_TIME void ot::PosixApp::RadioSpinel::Process(const Event &aEvent) { - if (!mFrameQueue.IsEmpty()) + if (mHdlcInterface.GetRxFrameBuffer().HasSavedFrame()) { ProcessFrameQueue(); } @@ -1521,6 +1557,8 @@ void ot::PosixApp::RadioSpinel::Process(const Event &aEvent) ProcessFrameQueue(); } + mHdlcInterface.GetRxFrameBuffer().ClearReadFrames(); + if (mState == kStateTransmitDone) { mState = kStateReceive; diff --git a/src/posix/platform/radio_spinel.hpp b/src/posix/platform/radio_spinel.hpp index f25ada405..eeb1435d0 100644 --- a/src/posix/platform/radio_spinel.hpp +++ b/src/posix/platform/radio_spinel.hpp @@ -36,7 +36,6 @@ #include -#include "frame_queue.hpp" #include "hdlc_interface.hpp" #include "spinel.h" @@ -443,11 +442,9 @@ public: /** * This method processes a received Spinel frame. * - * @param[in] aBuffer A pointer to buffer containing the frame. - * @param[in] aLength Length (number of bytes) in the received frame. - * + * @param[in] aFrameBuffer The frame buffer constaining the newly received frame. */ - void HandleSpinelFrame(const uint8_t *aFrame, uint16_t aLength); + void HandleSpinelFrame(HdlcInterface::RxFrameBuffer &aFrameBuffer); private: enum @@ -543,11 +540,6 @@ private: va_list args); otError ParseRadioFrame(otRadioFrame &aFrame, const uint8_t *aBuffer, uint16_t aLength); - static void HandleSpinelFrame(void *aContext, uint8_t *aBuffer, uint16_t aLength) - { - static_cast(aContext)->HandleSpinelFrame(aBuffer, aLength); - } - /** * This method returns if the property changed event is safe to be handled now. * @@ -565,6 +557,7 @@ private: (aKey == SPINEL_PROP_STREAM_RAW || aKey == SPINEL_PROP_MAC_ENERGY_SCAN_RESULT)); } + void HandleNotification(HdlcInterface::RxFrameBuffer &aFrameBuffer); void HandleNotification(const uint8_t *aBuffer, uint16_t aLength); void HandleValueIs(spinel_prop_key_t aKey, const uint8_t *aBuffer, uint16_t aLength); @@ -589,8 +582,6 @@ private: uint32_t mExpectedCommand; ///< Expected response command of current transaction. otError mError; ///< The result of current transaction. - FrameQueue mFrameQueue; - uint8_t mRxPsdu[OT_RADIO_FRAME_MAX_SIZE]; uint8_t mTxPsdu[OT_RADIO_FRAME_MAX_SIZE]; uint8_t mAckPsdu[OT_RADIO_FRAME_MAX_SIZE];