MessageSet: Added more tests and fixed comments from new enum encoding

Auterion · Mar 14, 2024 · 9fd493b · 9fd493b
1 parent e215582
commit 9fd493b
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 14 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -35,7 +35,7 @@ jobs:
         run: ./build/tests/tests
 
       - name: Generate coverage report
-        run: gcovr --sonarqube -o coverage.xml build
+        run: gcovr --exclude-throw-branches --exclude-unreachable-branches --sonarqube -o coverage.xml build
 
       - name: Run sonar-scanner
         env:

diff --git a/gcovr.cfg b/gcovr.cfg
@@ -1,4 +1,5 @@
 exclude-throw-branches = yes
+exclude-unreachable-branches = yes
 filter = include/mav
 exclude = include/mav/rapidxml/*
 exclude = include/mav/picosha2/*
diff --git a/include/mav/MessageSet.h b/include/mav/MessageSet.h
@@ -37,7 +37,6 @@
 #include <utility>
 #include <memory>
 #include <optional>
-#include <bitset>
 #include <cmath>
 
 #include "MessageDefinition.h"
@@ -93,23 +92,23 @@ namespace mav {
             try {
                 uint64_t res = std::stoul(str, &pos, base);
                 if (pos != str.size()) {
-                    throw ParseError("Could not parse " + str + " as a number");
+                    throw ParseError(StringFormat() << "Could not parse " << str << " as a number" << StringFormat::end);
                 }
                 return res;
             } catch (std::exception &e) {
-                throw ParseError("Could not parse " + str + " as a number (stoul failed): " + e.what());
+                throw ParseError(StringFormat() << "Could not parse " << str << " as a number (stoul failed): " << e.what() << StringFormat::end);
             }
         }
 
         static uint64_t _parseEnumValue(const std::string &str) {
             // Check for binary format: 0b or 0B
             if (str.size() >= 2 && (str.substr(0, 2) == "0b" || str.substr(0, 2) == "0B")) {
-                return std::bitset<64>(str.substr(2)).to_ullong();
+                return _strict_stoul(str.substr(2), 2);
             }
 
             // Check for hexadecimal format: 0x or 0X
             if (str.size() >= 2 && (str.substr(0, 2) == "0x" || str.substr(0, 2) == "0X")) {
-                return _strict_stoul(str, 16);
+                return _strict_stoul(str.substr(2), 16);
             }
 
             // Check for exponential format: 2**
@@ -131,7 +130,7 @@ namespace mav {
         }
 
 
-        static bool _isPrefix(std::string_view prefix, std::string_view full) {
+        static bool _isPrefix(std::string_view prefix, std::string_view full) noexcept {
             return prefix == full.substr(0, prefix.size());
         }
 
@@ -168,7 +167,7 @@ namespace mav {
             } else if (_isPrefix("double", field_type_string)) {
                 return {FieldType::BaseType::DOUBLE, size};
             }
-            throw ParseError("Unknown field type: " + field_type_string);
+            throw ParseError(StringFormat() << "Unknown field type: " << field_type_string << StringFormat::end);
         }
 
     public:
@@ -189,7 +188,7 @@ namespace mav {
             auto doc = std::make_shared<rapidxml::xml_document<>>();
             try {
                 doc->parse<0>(file->data());
-            } catch (rapidxml::parse_error &e) {
+            } catch (const rapidxml::parse_error &e) {
                 throw ParseError(e.what());
             }
             return {file, doc, ""};
@@ -198,7 +197,7 @@ namespace mav {
 
         void parse(std::map<std::string, uint64_t> &out_enum,
                    std::map<std::string, std::shared_ptr<const MessageDefinition>> &out_messages,
-                   std::map<int, std::shared_ptr<const MessageDefinition>> &out_message_ids) {
+                   std::map<int, std::shared_ptr<const MessageDefinition>> &out_message_ids) const {
 
             auto root_node = _document->first_node("mavlink");
             if (!root_node) {

diff --git a/tests/MessageSet.cpp b/tests/MessageSet.cpp
@@ -35,6 +35,19 @@ TEST_CASE("Message set creation") {
         CHECK_THROWS_AS(message_set.addFromXMLString("<mavlink>"), mav::ParseError);
     }
 
+    SUBCASE("Can not add message with unknown field type") {
+        CHECK_THROWS_AS(message_set.addFromXMLString(R""""(
+<mavlink>
+    <messages>
+        <message id="15321" name="ONLY_MESSAGE">
+            <field type="unknown" name="field">Field</field>
+        </message>
+    </messages>
+</mavlink>
+)""""), mav::ParseError);
+
+    }
+
     SUBCASE("Can add valid, partial XML") {
         // This is a valid XML file, but it does not contain any messages or enums
         message_set.addFromXMLString("<mavlink></mavlink>");
@@ -158,10 +171,10 @@ TEST_CASE("Enum value encoding") {
             <entry value="2**4" name="BIT4" />
             <entry value="0b000100000000" name="BIT8" />
             <entry value="0x10000" name="BIT16" />
-            <entry value="0b1000000000000000000000000000000000000000000000000000000000000" name="BIT60" />
+            <entry value="0B1000000000000000000000000000000000000000000000000000000000000" name="BIT60" />
             <entry value="2305843009213693952" name="BIT61" />
             <entry value="2**62" name="BIT62" />
-            <entry value="0x8000000000000000" name="BIT63" />
+            <entry value="0X8000000000000000" name="BIT63" />
         </enum>
     </enums>
 </mavlink>
@@ -194,7 +207,7 @@ TEST_CASE("Enum value encoding") {
         CHECK_THROWS_AS(message_set.addFromXMLString(empty_value), mav::ParseError);
     }
 
-    SUBCASE("Enum with invalid value") {
+    SUBCASE("Enum with invalid value 1") {
         std::string invalid_value = R""""(
 <mavlink>
     <enums>
@@ -209,6 +222,35 @@ TEST_CASE("Enum value encoding") {
         CHECK_THROWS_AS(message_set.addFromXMLString(invalid_value), mav::ParseError);
     }
 
+    SUBCASE("Enum with invalid value 2") {
+        std::string invalid_value = R""""(
+<mavlink>
+    <enums>
+        <enum name="MY_ENUM_INVALID_VALUE">
+            <entry value="thisiswrong" name="INVALID" />
+        </enum>
+    </enums>
+</mavlink>
+)"""";
+
+        MessageSet message_set;
+        CHECK_THROWS_AS(message_set.addFromXMLString(invalid_value), mav::ParseError);
+    }
+
+    SUBCASE("Enum with invalid value 3") {
+        std::string invalid_value = R""""(
+<mavlink>
+    <enums>
+        <enum name="MY_ENUM_INVALID_VALUE">
+            <entry value="128thereismorecontent" name="INVALID" />
+        </enum>
+    </enums>
+</mavlink>
+)"""";
+
+        MessageSet message_set;
+        CHECK_THROWS_AS(message_set.addFromXMLString(invalid_value), mav::ParseError);
+    }
 
     SUBCASE("Enum with overflow value") {
         std::string invalid_value = R""""(
@@ -225,8 +267,20 @@ TEST_CASE("Enum value encoding") {
         CHECK_THROWS_AS(message_set.addFromXMLString(invalid_value), mav::ParseError);
     }
 
+    SUBCASE("Enum with non-base-2 exponential value") {
+        std::string invalid_value = R""""(
+<mavlink>
+    <enums>
+        <enum name="MY_ENUM_INVALID_VALUE">
+            <entry value="3**3" name="INVALID" />
+        </enum>
+    </enums>
+</mavlink>
+)"""";
 
-
+        MessageSet message_set;
+        CHECK_THROWS_AS(message_set.addFromXMLString(invalid_value), mav::ParseError);
+    }
 
 }