Curiously Recurring Template Pattern (#88)

* CRTP encoder * CRTP framer * redundant template arg * use new pointers in benchmark * simplify json reading * separate threads for messages and enums * Update message_database.hpp * Update python.yml * remove clone --------- Co-authored-by: Jonathan McDermid <[email protected]>
novatel · Jan 29, 2025 · 7e00a47 · 7e00a47
1 parent 5c3a04d
commit 7e00a47
Show file tree

Hide file tree

Showing 15 changed files with 664 additions and 732 deletions.
diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml
@@ -5,11 +5,6 @@ on:
       - '**.md'
       - '.gitignore'
       - 'resources/**'
-  pull_request:
-    paths-ignore:
-      - '**.md'
-      - '.gitignore'
-      - 'resources/**'
   workflow_dispatch:
 jobs:
   python:

diff --git a/include/novatel_edie/decoders/common/encoder.hpp b/include/novatel_edie/decoders/common/encoder.hpp
diff --git a/include/novatel_edie/decoders/common/framer.hpp b/include/novatel_edie/decoders/common/framer.hpp
@@ -37,7 +37,7 @@
 //! \brief Base class for all framers. Contains necessary buffers and member
 //! variables, defining generic framer operations.
 //============================================================================
-class FramerBase
+template <typename Derived> class FramerBase
 {
   protected:
     std::shared_ptr<spdlog::logger> pclMyLogger;
@@ -52,7 +52,7 @@ class FramerBase
     bool bMyPayloadOnly{false};
     bool bMyFrameJson{false};
 
-    virtual void ResetState() = 0;
+    void ResetState() { return static_cast<Derived*>(this)->ResetStateImpl(); }
 
     [[nodiscard]] bool IsCrlf(const uint32_t uiPosition_) const
     {

diff --git a/include/novatel_edie/decoders/common/message_database.hpp b/include/novatel_edie/decoders/common/message_database.hpp
@@ -232,8 +232,6 @@ struct BaseField
 
     virtual ~BaseField() = default;
 
-    virtual BaseField* Clone() { return new BaseField(*this); }
-
     void SetConversion(const std::string& sConversion_)
     {
         conversion = sConversion_;
@@ -293,8 +291,6 @@ struct EnumField : BaseField
 
     ~EnumField() override = default;
 
-    EnumField* Clone() override { return new EnumField(*this); }
-
     using Ptr = std::shared_ptr<EnumField>;
     using ConstPtr = std::shared_ptr<const EnumField>;
 };
@@ -311,8 +307,6 @@ struct ArrayField : BaseField
 
     ~ArrayField() override = default;
 
-    ArrayField* Clone() override { return new ArrayField(*this); }
-
     using Ptr = std::shared_ptr<ArrayField>;
     using ConstPtr = std::shared_ptr<const ArrayField>;
 };
@@ -330,7 +324,8 @@ struct FieldArrayField : BaseField
 
     FieldArrayField(const FieldArrayField& that_) : BaseField(that_)
     {
-        for (const auto& field : that_.fields) { fields.push_back(std::shared_ptr<BaseField>(field->Clone())); }
+        fields.reserve(that_.fields.size());
+        for (const auto& field : that_.fields) { fields.emplace_back(field); }
 
         arrayLength = that_.arrayLength;
         fieldSize = that_.fieldSize;
@@ -343,7 +338,8 @@ struct FieldArrayField : BaseField
             BaseField::operator=(that_);
 
             fields.clear();
-            for (const auto& field : that_.fields) { fields.push_back(std::shared_ptr<BaseField>(field->Clone())); }
+            fields.reserve(that_.fields.size());
+            for (const auto& field : that_.fields) { fields.emplace_back(field); }
 
             arrayLength = that_.arrayLength;
             fieldSize = that_.fieldSize;
@@ -352,8 +348,6 @@ struct FieldArrayField : BaseField
         return *this;
     }
 
-    FieldArrayField* Clone() override { return new FieldArrayField(*this); }
-
     using Ptr = std::shared_ptr<FieldArrayField>;
     using ConstPtr = std::shared_ptr<const FieldArrayField>;
 };
@@ -378,10 +372,11 @@ struct MessageDefinition
     {
         for (const auto& fieldDefinition : that_.fields)
         {
-            uint32_t key = fieldDefinition.first;
+            const uint32_t key = fieldDefinition.first;
             // Ensure a 0-length vector exists for this key in the case the message has no fields.
             fields[key].clear();
-            for (const auto& field : fieldDefinition.second) { fields[key].emplace_back(field->Clone()); }
+            fields[key].reserve(fieldDefinition.second.size());
+            for (const auto& field : fieldDefinition.second) { fields[key].emplace_back(field); }
         }
 
         _id = that_._id;
@@ -398,10 +393,11 @@ struct MessageDefinition
             fields.clear();
             for (const auto& fieldDefinition : that_.fields)
             {
-                uint32_t key = fieldDefinition.first;
+                const uint32_t key = fieldDefinition.first;
                 // Ensure a 0-length vector exists for this key in the case the message has no fields.
                 fields[key].clear();
-                for (const auto& field : fieldDefinition.second) { fields[key].emplace_back(field->Clone()); }
+                fields[key].reserve(fieldDefinition.second.size());
+                for (const auto& field : fieldDefinition.second) { fields[key].emplace_back(field); }
             }
 
             _id = that_._id;

diff --git a/include/novatel_edie/decoders/oem/commander.hpp b/include/novatel_edie/decoders/oem/commander.hpp
@@ -53,7 +53,7 @@ class Commander
     EnumDefinition::ConstPtr vMyPortAddressDefinitions{nullptr};
     EnumDefinition::ConstPtr vMyGpsTimeStatusDefinitions{nullptr};
 
-    MessageDefinition stMyRespDef;
+    MessageDefinition::Ptr stMyRespDef{nullptr};
 
     // Enum util functions
     void InitEnumDefinitions();

diff --git a/include/novatel_edie/decoders/oem/encoder.hpp b/include/novatel_edie/decoders/oem/encoder.hpp
@@ -40,7 +40,7 @@ namespace novatel::edie::oem {
 //! \class Encoder
 //! \brief Class to encode OEM messages.
 //============================================================================
-class Encoder : public EncoderBase
+class Encoder : public EncoderBase<Encoder>
 {
   private:
     // Enum util functions
@@ -49,9 +49,9 @@ class Encoder : public EncoderBase
     [[nodiscard]] std::string JsonHeaderToMsgName(const IntermediateHeader& stInterHeader_) const;
 
   protected:
-    [[nodiscard]] char SeparatorAscii() const override { return OEM4_ASCII_FIELD_SEPARATOR; }
-    [[nodiscard]] char SeparatorAbbAscii() const override { return OEM4_ABBREV_ASCII_SEPARATOR; }
-    [[nodiscard]] uint32_t IndentationLengthAbbAscii() const override { return OEM4_ABBREV_ASCII_INDENTATION_LENGTH; }
+    static constexpr char separatorAscii = OEM4_ASCII_FIELD_SEPARATOR;
+    static constexpr char separatorAbbAscii = OEM4_ABBREV_ASCII_SEPARATOR;
+    static constexpr uint32_t indentationLengthAbbAscii = OEM4_ABBREV_ASCII_INDENTATION_LENGTH;
 
     // Encode binary
     [[nodiscard]] static bool EncodeBinaryHeader(const IntermediateHeader& stInterHeader_, unsigned char** ppcOutBuf_, uint32_t& uiBytesLeft_);
@@ -173,6 +173,8 @@ class Encoder : public EncoderBase
     //----------------------------------------------------------------------------
     [[nodiscard]] STATUS EncodeBody(unsigned char** ppucBuffer_, uint32_t uiBufferSize_, const std::vector<FieldContainer>& stMessage_,
                                     MessageDataStruct& stMessageData_, const MetaDataStruct& stMetaData_, ENCODE_FORMAT eFormat_) const;
+
+    friend class EncoderBase<Encoder>;
 };
 
 } // namespace novatel::edie::oem

diff --git a/include/novatel_edie/decoders/oem/framer.hpp b/include/novatel_edie/decoders/oem/framer.hpp
@@ -36,15 +36,15 @@ namespace novatel::edie::oem {
 //! \class Framer
 //! \brief Search bytes for patterns that could be OEM message.
 //============================================================================
-class Framer : public FramerBase
+class Framer : public FramerBase<Framer>
 {
   private:
     NovAtelFrameState eMyFrameState{NovAtelFrameState::WAITING_FOR_SYNC};
 
     uint32_t uiMyJsonObjectOpenBraces{0};
     uint32_t uiMyAbbrevAsciiHeaderPosition{0};
 
-    void ResetState() override;
+    void ResetStateImpl();
 
     //----------------------------------------------------------------------------
     //! \brief Check if the characters following an '*' fit the CRC format.
@@ -82,6 +82,8 @@ class Framer : public FramerBase
     //! to the size specified by uiFrameBufferSize_.
     //----------------------------------------------------------------------------
     [[nodiscard]] STATUS GetFrame(unsigned char* pucFrameBuffer_, uint32_t uiFrameBufferSize_, MetaDataStruct& stMetaData_);
+
+    friend class FramerBase<Framer>;
 };
 
 } // namespace novatel::edie::oem

diff --git a/python/bindings/message_database.cpp b/python/bindings/message_database.cpp
@@ -91,7 +91,6 @@ void init_common_message_database(nb::module_& m)
         .def_rw("conversion_before_point", &BaseField::conversionBeforePoint)
         .def_rw("conversion_after_point", &BaseField::conversionAfterPoint)
         .def_rw("data_type", &BaseField::dataType)
-        .def("clone", &BaseField::Clone)
         .def("set_conversion", &BaseField::SetConversion, "conversion"_a)
         .def("__repr__", [](const BaseField& field) {
             const std::string& desc = field.description == "[Brief Description]" ? "" : field.description;
@@ -120,7 +119,6 @@ void init_common_message_database(nb::module_& m)
         .def_rw("enum_id", &EnumField::enumId)
         .def_rw("enum_def", &EnumField::enumDef)
         .def_rw("length", &EnumField::length)
-        .def("clone", &EnumField::Clone)
         .def("__repr__", [](const EnumField& field) {
             const std::string& desc = field.description == "[Brief Description]" ? "" : field.description;
             return nb::str("EnumField(name={!r}, type={}, data_type={}, description={!r}, conversion={!r}, enum_id={!r}, length={!r})")
@@ -130,7 +128,6 @@ void init_common_message_database(nb::module_& m)
     nb::class_<ArrayField, BaseField>(m, "ArrayField", "Struct containing elements of array fields in the UI DB")
         .def(nb::init())
         .def_rw("array_length", &ArrayField::arrayLength)
-        .def("clone", &ArrayField::Clone)
         .def("__repr__", [](const ArrayField& field) {
             const std::string& desc = field.description == "[Brief Description]" ? "" : field.description;
             return nb::str("ArrayField(name={!r}, type={}, data_type={}, description={!r}, conversion={!r}, array_length={!r})")
@@ -142,7 +139,6 @@ void init_common_message_database(nb::module_& m)
         .def_rw("array_length", &FieldArrayField::arrayLength)
         .def_rw("field_size", &FieldArrayField::fieldSize)
         .def_rw("fields", &FieldArrayField::fields, nb::rv_policy::reference_internal)
-        .def("clone", &FieldArrayField::Clone)
         .def("__repr__", [](const FieldArrayField& field) {
             const std::string& desc = field.description == "[Brief Description]" ? "" : field.description;
             return nb::str("FieldArrayField(name={!r}, type={}, data_type={}, description={!r}, conversion={!r}, array_length={!r}, field_size={!r}, "