Add a basic message reader for the data model. (#3575)

The idea is to expose a semantic API where consumers can read things
like "a cluster id" or "an endpoint id", without necessarily making
assumptions about the sizes/representations of these things.

The API does still expose facilities for reading raw unsigned integers
of specific sizes.

More types will be added to this API as needed.

src/app/decoder.cpp

@@ -23,28 +23,13 @@
 //                            InterPanHeader *interPanHeader)
 
 #include <app/chip-zcl-zpro-codec.h>
-#include <core/CHIPEncoding.h>
+#include <app/message-reader.h>
+#include <lib/core/CHIPError.h>
+#include <lib/support/CodeUtils.h>
 #include <stdio.h>
 #include <string.h>
 #include <support/logging/CHIPLogging.h>
 
-template <int N>
-struct Reader
-{
-};
-
-template <>
-struct Reader<1>
-{
-    static uint8_t read(const uint8_t *& p) { return chip::Encoding::Read8(p); }
-};
-
-template <>
-struct Reader<2>
-{
-    static uint16_t read(const uint8_t *& p) { return chip::Encoding::LittleEndian::Read16(p); }
-};
-
 extern "C" {
 
 uint16_t extractApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame * outApsFrame)
@@ -55,39 +40,26 @@ uint16_t extractApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame *
         return 0;
     }
 
-    const uint8_t * read_ptr = buffer;
+    chip::DataModelReader reader(buffer, buf_length);
 
-    // Skip first byte, because that's the always-0 frame control.
-    ++read_ptr;
-    --buf_length;
-
-#define TRY_READ(fieldName, fieldSize)                                                                                             \
-    do                                                                                                                             \
-    {                                                                                                                              \
-        static_assert(sizeof(outApsFrame->fieldName) == fieldSize, "incorrect size for " #fieldName);                              \
-        if (buf_length < fieldSize)                                                                                                \
-        {                                                                                                                          \
-            ChipLogError(Zcl, "Missing " #fieldName " when extracting APS frame");                                                 \
-            return 0;                                                                                                              \
-        }                                                                                                                          \
-        outApsFrame->fieldName = Reader<fieldSize>::read(read_ptr);                                                                \
-        buf_length             = static_cast<uint16_t>(buf_length - fieldSize);                                                    \
-    } while (0)
-
-    TRY_READ(profileId, 2);
-    TRY_READ(clusterId, 2);
-    TRY_READ(sourceEndpoint, 1);
-    TRY_READ(destinationEndpoint, 1);
-    TRY_READ(options, 2);
-    TRY_READ(groupId, 2);
-    TRY_READ(sequence, 1);
-    TRY_READ(radius, 1);
+    CHIP_ERROR err = CHIP_NO_ERROR;
 
-#undef TRY_READ
-
-    // Cast is safe because buf_length is uint16_t, so we can't have moved too
-    // far along.
-    return static_cast<uint16_t>(read_ptr - buffer);
+    // Skip first byte, because that's the always-0 frame control.
+    uint8_t ignored;
+    err = reader.ReadOctet(&ignored)
+              .Read16(&outApsFrame->profileId)
+              .ReadClusterId(&outApsFrame->clusterId)
+              .ReadEndpointId(&outApsFrame->sourceEndpoint)
+              .ReadEndpointId(&outApsFrame->destinationEndpoint)
+              .Read16(&outApsFrame->options)
+              .ReadGroupId(&outApsFrame->groupId)
+              .ReadOctet(&outApsFrame->sequence)
+              .ReadOctet(&outApsFrame->radius)
+              .StatusCode();
+    SuccessOrExit(err);
+
+exit:
+    return err == CHIP_NO_ERROR ? reader.OctetsRead() : 0;
 }
 
 void printApsFrame(EmberApsFrame * frame)

src/app/message-reader.h

@@ -0,0 +1,139 @@
+/*
+ *
+ *    Copyright (c) 2020 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
+ *          Contains the API definition for a message buffer reader for the data
+ *          model.  This reader does the necessary bounds-checks before reading
+ *          and updates its own state as the buffer is read.
+ */
+
+#pragma once
+
+#include <app/util/basic-types.h>
+#include <lib/core/CHIPError.h>
+#include <lib/support/BufferReader.h>
+#include <lib/support/CodeUtils.h>
+#include <stdint.h>
+
+namespace chip {
+
+class DataModelReader
+{
+public:
+    /**
+     * Create a data model reader from a given buffer and length.
+     *
+     * @param buffer The octet buffer to read from.  The caller must ensure
+     *               (most simply by allocating the reader on the stack) that
+     *               the buffer outlives the reader.  The buffer is allowed to
+     *               be null if buf_len is 0.
+     * @param buf_len The number of octets in the buffer.
+     */
+    DataModelReader(const uint8_t * buffer, uint16_t buf_len) : mReader(buffer, buf_len) {}
+
+    /**
+     * Number of octets we have read so far.  This might be able to go away once
+     * we do less switching back and forth between DataModelReader and raw
+     * buffers.
+     */
+    uint16_t OctetsRead() const { return mReader.OctetsRead(); }
+
+    /**
+     * The reader status.
+     */
+    CHIP_ERROR StatusCode() const { return mReader.StatusCode(); }
+
+    /**
+     * Read a cluster id.
+     *
+     * @param [out] cluster_id Where the cluster id goes.
+     *
+     * @return Whether the read succeeded.  The read can fail if there are not
+     *         enough octets available.
+     */
+    CHECK_RETURN_VALUE DataModelReader & ReadClusterId(ClusterId * cluster_id)
+    {
+        mReader.RawRead(cluster_id);
+        return *this;
+    }
+
+    /**
+     * Read an endpoint id.
+     *
+     * @param [out] endpoint_id Where the endpoint id goes.
+     *
+     * @return Whether the read succeeded.  The read can fail if there are not
+     *         enough octets available.
+     */
+    CHECK_RETURN_VALUE DataModelReader & ReadEndpointId(EndpointId * endpoint_id)
+    {
+        mReader.RawRead(endpoint_id);
+        return *this;
+    }
+
+    /**
+     * Read a group id.
+     *
+     * @param [out] group_id Where the group id goes.
+     *
+     * @return Whether the read succeeded.  The read can fail if there are not
+     *         enough octets available.
+     */
+    CHECK_RETURN_VALUE DataModelReader & ReadGroupId(GroupId * group_id)
+    {
+        mReader.RawRead(group_id);
+        return *this;
+    }
+
+    /**
+     * Read a single octet.
+     *
+     * @param [out] octet Where the octet goes.
+     *
+     * @return Whether the read succeeded.  The read can fail if there are not
+     *         enough octets available.
+     *
+     * @note Use of APIs that read some semantically-meaningful type is preferred.
+     */
+    CHECK_RETURN_VALUE DataModelReader & ReadOctet(uint8_t * octet)
+    {
+        mReader.RawRead(octet);
+        return *this;
+    }
+
+    /**
+     * Read a single 16-bit unsigned integer.
+     *
+     * @param [out] dest Where the 16-bit integer goes.
+     *
+     * @return Whether the read succeeded.  The read can fail if there are not
+     *         enough octets available.
+     *
+     * @note Use of APIs that read some semantically-meaningful type is preferred.
+     */
+    CHECK_RETURN_VALUE DataModelReader & Read16(uint16_t * dest)
+    {
+        mReader.RawRead(dest);
+        return *this;
+    }
+
+private:
+    Encoding::LittleEndian::Reader mReader;
+};
+
+} // namespace chip

src/lib/support/BufferReader.cpp

@@ -44,7 +44,7 @@ void ReadHelper(const uint8_t *& p, uint64_t * dest)
 } // anonymous namespace
 
 template <typename T>
-void Reader::Read(T * retval)
+void Reader::RawRead(T * retval)
 {
     static_assert(CHAR_BIT == 8, "Our various sizeof checks rely on bytes and octets being the same thing");
 
@@ -63,10 +63,10 @@ void Reader::Read(T * retval)
 }
 
 // Explicit Read instantiations for the data types we want to support.
-template void Reader::Read(uint8_t *);
-template void Reader::Read(uint16_t *);
-template void Reader::Read(uint32_t *);
-template void Reader::Read(uint64_t *);
+template void Reader::RawRead(uint8_t *);
+template void Reader::RawRead(uint16_t *);
+template void Reader::RawRead(uint32_t *);
+template void Reader::RawRead(uint64_t *);
 
 } // namespace LittleEndian
 } // namespace Encoding

src/lib/support/BufferReader.h

@@ -79,7 +79,7 @@ class Reader
     CHECK_RETURN_VALUE
     Reader & Read8(uint8_t * dest)
     {
-        Read(dest);
+        RawRead(dest);
         return *this;
     }
 
@@ -96,7 +96,7 @@ class Reader
     CHECK_RETURN_VALUE
     Reader & Read16(uint16_t * dest)
     {
-        Read(dest);
+        RawRead(dest);
         return *this;
     }
 
@@ -113,7 +113,7 @@ class Reader
     CHECK_RETURN_VALUE
     Reader & Read32(uint32_t * dest)
     {
-        Read(dest);
+        RawRead(dest);
         return *this;
     }
 
@@ -130,17 +130,19 @@ class Reader
     CHECK_RETURN_VALUE
     Reader & Read64(uint64_t * dest)
     {
-        Read(dest);
+        RawRead(dest);
         return *this;
     }
 
-protected:
     /**
      * Helper for our various APIs so we don't have to write out various logic
-     * multiple times.
+     * multiple times.  This is public so that consumers that want to read into
+     * whatever size a logical thing they are reading into has don't have to
+     * hardcode the right API.  This is meant for other reader classes that
+     * delegate to this one.
      */
     template <typename T>
-    void Read(T * retval);
+    void RawRead(T * retval);
 
 private:
     /**