Merge commit '903a3f9' into qgcgov-changes

.github/workflows/test.yml

@@ -8,12 +8,40 @@ jobs:
     strategy:
         fail-fast: false
         matrix:
-            platform: [ubuntu-latest, macos-latest]
-    runs-on: ${{ matrix.platform }}
+            platform:
+              - runner: ubuntu-latest
+                sonarcloud-build-wrapper : build-wrapper-linux-x86-64
+              - runner: ubuntu-20.04
+                sonarcloud-build-wrapper : build-wrapper-linux-x86-64
+              - runner: macos-latest
+                sonarcloud-build-wrapper : build-wrapper-macosx-x86
+    runs-on: ${{ matrix.platform.runner }}
     steps:
       - uses: actions/checkout@v3
+
+      - name: Install gcovr
+        run: pip3 install gcovr
+
+      - name: Setup sonar cloud
+        uses: SonarSource/[email protected]
+
+      - name: Build tests
+        run: cmake -B build -S .
+
       - name: Build tests
-        run: mkdir build && cd build && cmake .. && make tests
-      - name: Run tests
-        run: ./tests/tests
-        working-directory: build
+        run: ${{ matrix.platform.sonarcloud-build-wrapper }} --out-dir bw-output cmake --build build
+
+      - name: Run tests and coverage
+        run: ./build/tests/tests
+
+      - name: Generate coverage report
+        run: gcovr --sonarqube -o coverage.xml build
+
+      - name: Run sonar-scanner
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          SONAR_TOKEN: ${{ secrets.AUTERION_CI_SONAR_TOKEN }}
+        run: |
+          sonar-scanner \
+            --define sonar.cfamily.build-wrapper-output="bw-output" \
+            --define sonar.coverageReportPaths=coverage.xml

.gitignore

@@ -2,3 +2,4 @@
 cmake-build-debug
 build
 Testing
+html

.vscode/c_cpp_properties.json

@@ -0,0 +1,17 @@
+{
+    "configurations": [
+        {
+            "name": "Linux",
+            "includePath": [
+                "${workspaceFolder}/**"
+            ],
+            "defines": [],
+            "compilerPath": "/usr/bin/gcc",
+            "cStandard": "c11",
+            "cppStandard": "c++17",
+            "intelliSenseMode": "gcc-x64",
+            "compileCommands": "${workspaceFolder}/build/compile_commands.json"
+        }
+    ],
+    "version": 4
+}

CMakeLists.txt

@@ -6,6 +6,13 @@ include(GNUInstallDirs)
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
 
+find_program(CCACHE_PROGRAM ccache)
+if(CCACHE_PROGRAM)
+    set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE "${CCACHE_PROGRAM}")
+endif()
+
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+
 set(CMAKE_CXX_STANDARD 17)
 
 add_library(mav INTERFACE)

README.md

@@ -29,6 +29,22 @@ Since the library is header only, you only need the library on the build system.
 
 You can also include the library as a submodule in your project.
 
+### Running the tests
+
+Libmav uses [doctest](https://github.com/doctest/doctest/) and [gcovr](https://github.com/gcovr/gcovr/).
+
+To run the tests, build the library, then run the test executable. Test results will be output to console.
+
+```bash
+mkdir build && cd build && cmake .. && make tests
+./tests/tests
+```
+
+To test coverage, simple invoke the coverage tool from the root directory.
+```bash
+gcovr
+```
+
 ## Getting started
 
 ### Loading a message set

gcovr.cfg

@@ -1,3 +1,4 @@
 exclude-throw-branches = yes
 filter = include/mav
-exclude = include/mav/rapidxml/*
+exclude = include/mav/rapidxml/*
+exclude = include/mav/picosha2/*

include/mav/Connection.h

@@ -63,9 +63,7 @@ namespace mav {
 
         struct PromiseCallback {
             Expectation promise;
-            int message_id;
-            int system_id;
-            int component_id;
+            std::function<bool(const Message &message)> selector;
         };
 
         using Callback = std::variant<FunctionCallback, PromiseCallback>;
@@ -123,9 +121,7 @@ namespace mav {
                             }
                             it++;
                         } else if constexpr (std::is_same_v<T, PromiseCallback>) {
-                            if (message.id() == arg.message_id &&
-                                    (arg.system_id == mav::ANY_ID || message.header().systemId() == arg.system_id) &&
-                                    (arg.component_id == mav::ANY_ID || message.header().componentId() == arg.component_id)) {
+                            if (arg.selector(message)) {
                                 arg.promise->set_value(message);
                                 it = _message_callbacks.erase(it);
                             } else {
@@ -199,20 +195,24 @@ namespace mav {
             _message_callbacks.erase(handle);
         }
 
-
-        [[nodiscard]] Expectation expect(int message_id, int source_id=mav::ANY_ID,
-                                         int component_id=mav::ANY_ID) {
-
+        [[nodiscard]] Expectation expect(std::function<bool(const mav::Message&)> selector) {
             auto promise = std::make_shared<std::promise<Message>>();
             std::scoped_lock<std::mutex> lock(_message_callback_mtx);
             CallbackHandle handle = _next_handle;
-            _message_callbacks[handle] = PromiseCallback{promise, message_id, source_id, component_id};
+            _message_callbacks[handle] = PromiseCallback{promise, std::move(selector)};
             _next_handle++;
-
-            auto prom = std::make_shared<std::promise<Message>>();
             return promise;
         }
 
+        [[nodiscard]] Expectation expect(int message_id, int source_id=mav::ANY_ID,
+                                         int component_id=mav::ANY_ID) {
+            return expect([message_id, source_id, component_id](const Message &message) {
+                    return message.id() == message_id &&
+                           (source_id == mav::ANY_ID || message.header().systemId() == source_id) &&
+                           (component_id == mav::ANY_ID || message.header().componentId() == component_id);
+            });
+        }
+
         [[nodiscard]] inline Expectation expect(const std::string &message_name, int source_id=mav::ANY_ID,
                                          int component_id=mav::ANY_ID) {
             return expect(_message_set.idForMessage(message_name), source_id, component_id);
@@ -250,6 +250,10 @@ namespace mav {
         Message inline receive(int message_id, int timeout_ms=-1) {
             return receive(message_id, mav::ANY_ID, mav::ANY_ID, timeout_ms);
         }
+
+        Message inline receive(std::function<bool(const mav::Message&)> selector, int timeout_ms=-1) {
+            return receive(expect(std::move(selector)), timeout_ms);
+        }
     };
 
 };

include/mav/Message.h

@@ -77,10 +77,10 @@ namespace mav {
     class Message {
         friend MessageSet;
     private:
-        ConnectionPartner _source_partner;
+        ConnectionPartner _source_partner{};
         std::array<uint8_t, MessageDefinition::MAX_MESSAGE_SIZE> _backing_memory{};
-        const MessageDefinition* _message_definition;
-        int _crc_offset = -1;
+        const MessageDefinition* _message_definition{nullptr};
+        int _crc_offset{-1};
 
         explicit Message(const MessageDefinition &message_definition) :
             _message_definition(&message_definition) {
@@ -154,33 +154,27 @@ namespace mav {
             throw std::runtime_error("Unknown base type"); // should never happen
         }
 
-        uint64_t _computeSignatureHash48(const std::array<uint8_t, 32>& key) const {
+        uint64_t _computeSignatureHash48(const std::array<uint8_t, MessageDefinition::KEY_SIZE>& key) const {
             // signature = sha256_48(secret_key + header + payload + CRC + link-ID + timestamp)
-            constexpr size_t maxSize = 32 + MessageDefinition::HEADER_SIZE +
-                                       MessageDefinition::MAX_PAYLOAD_SIZE +
-                                       MessageDefinition::CHECKSUM_SIZE + 1 + 6;
-            std::array<uint8_t, maxSize> data;
-            size_t actualSize = 0;
+            picosha2::hash256_one_by_one hasher;
             // secret_key
-            std::copy_n(key.begin(), 32, data.begin() + actualSize);
-            actualSize += 32;
+            hasher.process(key.begin(), key.begin() + MessageDefinition::KEY_SIZE);
             // header + payload + CRC
-            const size_t dataSize =
-                MessageDefinition::HEADER_SIZE + header().len() + MessageDefinition::CHECKSUM_SIZE;
-            std::copy_n(_backing_memory.begin(), dataSize, data.begin() + actualSize);
-            actualSize += dataSize;
+            hasher.process(_backing_memory.begin(), _backing_memory.begin() + 
+                    MessageDefinition::HEADER_SIZE + header().len() + MessageDefinition::CHECKSUM_SIZE);
             // link-ID
             const uint8_t linkId = signature().linkId();
-            serialize(linkId, data.begin() + actualSize);
-            actualSize += 1;
+            hasher.process(&linkId, &linkId + MessageDefinition::SIGNATURE_LINK_ID_SIZE);
             // timestamp
             const uint64_t timestamp = signature().timestamp();
-            serialize(timestamp, data.begin() + actualSize);
-            actualSize += 6;
+            std::array<uint8_t, sizeof(timestamp)> timestampSerialized;
+            serialize(timestamp, timestampSerialized.begin());
+            hasher.process(timestampSerialized.begin(), timestampSerialized.begin() + MessageDefinition::SIGNATURE_TIMESTAMP_SIZE);
 
+            hasher.finish();
             std::vector<unsigned char> hash(picosha2::k_digest_size);
-            picosha2::hash256(data.begin(), data.begin() + actualSize, hash.begin(), hash.end());
-            return deserialize<uint64_t>(hash.data(), 6);
+            hasher.get_hash_bytes(hash.begin(), hash.end());
+            return deserialize<uint64_t>(hash.data(), MessageDefinition::SIGNATURE_SIGNATURE_SIZE);
         }
 
     public:
@@ -253,10 +247,16 @@ namespace mav {
         }
 
         [[nodiscard]] const Signature<const uint8_t*> signature() const {
+            if (!isFinalized()) {
+                throw std::runtime_error("Unable to parse unfinalized message.");
+            }
             return Signature<const uint8_t*>(&_backing_memory[MessageDefinition::HEADER_SIZE + header().len() + MessageDefinition::CHECKSUM_SIZE]);
         }
 
         [[nodiscard]] Signature<uint8_t*> signature() {
+            if (!isFinalized()) {
+                throw std::runtime_error("Unable to parse unfinalized message.");
+            }
             return Signature<uint8_t*>(&_backing_memory[MessageDefinition::HEADER_SIZE + header().len() + MessageDefinition::CHECKSUM_SIZE]);
         }
 
@@ -478,21 +478,23 @@ namespace mav {
             return ss.str();
         }
 
-        void sign(const std::array<uint8_t, 32>& key, const uint64_t& timestamp) {
-            signature().linkId() = 0;
-            signature().timestamp() = timestamp;
-            signature().signature() = _computeSignatureHash48(key);
+        [[nodiscard]] bool validate(const std::array<uint8_t, MessageDefinition::KEY_SIZE>& key) const {
+            return signature().signature() == _computeSignatureHash48(key);
         }
 
-        [[nodiscard]] bool validate(const std::array<uint8_t, 32>& key) const {
-            return signature().signature() == _computeSignatureHash48(key);
+        [[nodiscard]] uint32_t finalize(uint8_t seq, const Identifier &sender) {
+            static const std::array<uint8_t, MessageDefinition::KEY_SIZE> null_key = {};
+            return finalize(seq, sender, null_key, 0, 0);
         }
 
-        [[nodiscard]] uint32_t finalize(uint8_t seq, const Identifier &sender, const bool sign = false) {
+        [[nodiscard]] uint32_t finalize(uint8_t seq, const Identifier &sender,
+                                        const std::array<uint8_t, MessageDefinition::KEY_SIZE>& key,
+                                        const uint64_t& timestamp, const uint8_t linkId = 0) {
             if (isFinalized()) {
                 _unFinalize();
             }
 
+            bool sign = (timestamp > 0);
             auto last_nonzero = std::find_if(_backing_memory.rend() -
                     MessageDefinition::HEADER_SIZE - _message_definition->maxPayloadSize(),
                     _backing_memory.rend(), [](const auto &v) {
@@ -504,15 +506,15 @@ namespace mav {
                             - MessageDefinition::HEADER_SIZE, 1);
 
             header().magic() = 0xFD;
-            header().len() = payload_size;
+            header().len() = static_cast<uint8_t>(payload_size);
             header().incompatFlags() = sign ? 0x01 : 0x00;
             header().compatFlags() = 0;
             header().seq() = seq;
             if (header().systemId() == 0) {
-                header().systemId() = sender.system_id;
+                header().systemId() = static_cast<uint8_t>(sender.system_id);
             }
             if (header().componentId() == 0) {
-                header().componentId() = sender.component_id;
+                header().componentId() = static_cast<uint8_t>(sender.component_id);
             }
             header().msgId() = _message_definition->id();
 
@@ -523,7 +525,15 @@ namespace mav {
             _crc_offset = MessageDefinition::HEADER_SIZE + payload_size;
             serialize(crc.crc16(), _backing_memory.data() + _crc_offset);
 
-            return MessageDefinition::HEADER_SIZE + payload_size + MessageDefinition::CHECKSUM_SIZE;
+            int signature_size = 0;
+            if (sign) {
+                signature().linkId() = linkId;
+                signature().timestamp() = timestamp;
+                signature().signature() = _computeSignatureHash48(key);
+                signature_size = MessageDefinition::SIGNATURE_SIZE;
+            }
+
+            return MessageDefinition::HEADER_SIZE + payload_size + MessageDefinition::CHECKSUM_SIZE + signature_size;
         }
 
         [[nodiscard]] const uint8_t* data() const {
@@ -533,4 +543,4 @@ namespace mav {
 
 } // namespace libmavlink`
 
-#endif //MAV_DYNAMICMESSAGE_H
+#endif //MAV_DYNAMICMESSAGE_H

include/mav/MessageDefinition.h

@@ -367,8 +367,12 @@ namespace mav {
         static constexpr int MAX_PAYLOAD_SIZE = 255;
         static constexpr int HEADER_SIZE = 10;
         static constexpr int CHECKSUM_SIZE = 2;
-        static constexpr int SIGNATURE_SIZE = 13;
+        static constexpr int SIGNATURE_LINK_ID_SIZE = 1;
+        static constexpr int SIGNATURE_TIMESTAMP_SIZE = 6;
+        static constexpr int SIGNATURE_SIGNATURE_SIZE = 6;
+        static constexpr int SIGNATURE_SIZE = SIGNATURE_LINK_ID_SIZE + SIGNATURE_TIMESTAMP_SIZE + SIGNATURE_SIGNATURE_SIZE;
         static constexpr int MAX_MESSAGE_SIZE = MAX_PAYLOAD_SIZE + HEADER_SIZE + CHECKSUM_SIZE + SIGNATURE_SIZE;
+        static constexpr int KEY_SIZE = 32;
 
         [[nodiscard]] inline const std::string& name() const {
             return _name;