From d393f08b6f61bd496069affacb6ce38c7604d00a Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Fri, 18 Jan 2019 06:05:13 -0800
Subject: [PATCH] [ncp-base] define ResponseType for queued spinel response
 entries (#3471)

---
 src/ncp/ncp_base.cpp | 14 +++++++-------
 src/ncp/ncp_base.hpp | 33 +++++++++++++++++++++------------
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/src/ncp/ncp_base.cpp b/src/ncp/ncp_base.cpp
index 508ad8366..837a7557f 100644
--- a/src/ncp/ncp_base.cpp
+++ b/src/ncp/ncp_base.cpp
@@ -649,7 +649,7 @@ uint8_t NcpBase::GetWrappedResponseQueueIndex(uint8_t aPosition)
     return aPosition;
 }
 
-otError NcpBase::PrepareResponse(uint8_t aHeader, bool aIsLastStatus, bool aIsGetResponse, unsigned int aKeyOrStatus)
+otError NcpBase::EnqueueResponse(uint8_t aHeader, ResponseType aType, unsigned int aKeyOrStatus)
 {
     otError        error = OT_ERROR_NONE;
     spinel_tid_t   tid   = SPINEL_HEADER_GET_TID(aHeader);
@@ -661,7 +661,7 @@ otError NcpBase::PrepareResponse(uint8_t aHeader, bool aIsLastStatus, bool aIsGe
         // `LAST_STATUS` error status (if not filtered) for TID
         // zero (e.g., for a dropped `STREAM_NET` set command).
 
-        if (aIsLastStatus)
+        if (aType == kResponseTypeLastStatus)
         {
             mChangedPropsSet.AddLastStatus(static_cast<spinel_status_t>(aKeyOrStatus));
         }
@@ -707,8 +707,7 @@ otError NcpBase::PrepareResponse(uint8_t aHeader, bool aIsLastStatus, bool aIsGe
 
     entry->mTid             = tid;
     entry->mIsInUse         = true;
-    entry->mIsLastStatus    = aIsLastStatus;
-    entry->mIsGetResponse   = aIsGetResponse;
+    entry->mType            = aType;
     entry->mPropKeyOrStatus = aKeyOrStatus;
 
     mResponseQueueTail++;
@@ -731,7 +730,7 @@ otError NcpBase::SendQueuedResponses(void)
 
             header |= static_cast<uint8_t>(entry.mTid << SPINEL_HEADER_TID_SHIFT);
 
-            if (entry.mIsLastStatus)
+            if (entry.mType == kResponseTypeLastStatus)
             {
                 spinel_status_t status = static_cast<spinel_status_t>(entry.mPropKeyOrStatus);
 
@@ -739,9 +738,10 @@ otError NcpBase::SendQueuedResponses(void)
             }
             else
             {
-                spinel_prop_key_t propKey = static_cast<spinel_prop_key_t>(entry.mPropKeyOrStatus);
+                spinel_prop_key_t propKey       = static_cast<spinel_prop_key_t>(entry.mPropKeyOrStatus);
+                bool              isGetResponse = (entry.mType == kResponseTypeGet);
 
-                SuccessOrExit(error = WritePropertyValueIsFrame(header, propKey, entry.mIsGetResponse));
+                SuccessOrExit(error = WritePropertyValueIsFrame(header, propKey, isGetResponse));
             }
         }
 
diff --git a/src/ncp/ncp_base.hpp b/src/ncp/ncp_base.hpp
index 801ef0739..7656564a9 100644
--- a/src/ncp/ncp_base.hpp
+++ b/src/ncp/ncp_base.hpp
@@ -174,19 +174,26 @@ protected:
     typedef otError (NcpBase::*PropertyHandler)(void);
 
     /**
-     * This struct represents a spinel response entry.
+     * This enumeration represents the `ResponseEntry` type.
      *
-     * It can be either a `VALUE_IS` property update for a specific property, or a `VALUE_IS(LAST_STATUS)` with a given
-     * spinel status error.
+     */
+    enum ResponseType
+    {
+        kResponseTypeGet = 0,    ///< Response entry is for a `VALUE_GET` command.
+        kResponseTypeSet,        ///< Response entry is for a `VALUE_SET` command.
+        kResponseTypeLastStatus, ///< Response entry is a `VALUE_IS(LAST_STATUS)`.
+    };
+
+    /**
+     * This struct represents a spinel response entry.
      *
      */
     struct ResponseEntry
     {
-        uint8_t  mTid : 4;              ///< Spinel transaction id.
-        bool     mIsInUse : 1;          ///< `true` if this entry is in use, `false` otherwise.
-        bool     mIsLastStatus : 1;     ///< `true` if entry is a `LAST_STATUS`, otherwise it is a property update.
-        bool     mIsGetResponse : 1;    ///< `true` if response is for `VALUE_GET`, 'false` otherwise.
-        uint32_t mPropKeyOrStatus : 24; ///< 3 bytes for either property key or spinel status.
+        uint8_t      mTid : 4;              ///< Spinel transaction id.
+        bool         mIsInUse : 1;          ///< `true` if this entry is in use, `false` otherwise.
+        ResponseType mType : 2;             ///< Response type.
+        uint32_t     mPropKeyOrStatus : 24; ///< 3 bytes for either property key or spinel status.
     };
 
     NcpFrameBuffer::FrameTag GetLastOutboundFrameTag(void);
@@ -212,19 +219,21 @@ protected:
 
     otError SendQueuedResponses(void);
     bool    IsResponseQueueEmpty(void) { return (mResponseQueueHead == mResponseQueueTail); }
-    otError PrepareResponse(uint8_t aHeader, bool aIsLastStatus, bool aIsGetResponse, unsigned int aPropKeyOrStatus);
+    otError EnqueueResponse(uint8_t aHeader, ResponseType aType, unsigned int aPropKeyOrStatus);
+
     otError PrepareGetResponse(uint8_t aHeader, spinel_prop_key_t aPropKey)
     {
-        return PrepareResponse(aHeader, false /* aIsLastStatus */, true /* aIsGetResponse*/, aPropKey);
+        return EnqueueResponse(aHeader, kResponseTypeGet, aPropKey);
     }
     otError PrepareSetResponse(uint8_t aHeader, spinel_prop_key_t aPropKey)
     {
-        return PrepareResponse(aHeader, false /* aIsLastStatus */, false /* aIsGetResponse*/, aPropKey);
+        return EnqueueResponse(aHeader, kResponseTypeSet, aPropKey);
     }
     otError PrepareLastStatusResponse(uint8_t aHeader, spinel_status_t aStatus)
     {
-        return PrepareResponse(aHeader, true /*aIsLastStatus */, false, aStatus);
+        return EnqueueResponse(aHeader, kResponseTypeLastStatus, aStatus);
     }
+
     static uint8_t GetWrappedResponseQueueIndex(uint8_t aPosition);
 
     static void UpdateChangedProps(Tasklet &aTasklet);