Merge remote-tracking branch 'origin/main' into HEAD

Auterion · Dec 1, 2023 · ea665f2 · ea665f2
2 parents b85c387 + 4c612dc
commit ea665f2
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 23 deletions.
diff --git a/README.md b/README.md
@@ -98,7 +98,7 @@ auto physical = mav::TCPClient("<ip>", 4560);
 
 auto heartbeat_message = message_set.create("HEARTBEAT");
 heartbeat_message.set({
-    {"type", message_set.e("MAV_TYPE_GCS")},
+    {"type", message_set.e("MAV_TYPE_ONBOARD_CONTROLLER")},
     {"autopilot", message_set.e("MAV_AUTOPILOT_GENERIC")},
     {"base_mode", 0},
     {"custom_mode", 0},

diff --git a/include/mav/Message.h b/include/mav/Message.h
@@ -88,10 +88,10 @@ namespace mav {
 
         Message(const MessageDefinition &message_definition, ConnectionPartner source_partner, int crc_offset,
                 std::array<uint8_t, MessageDefinition::MAX_MESSAGE_SIZE> &&backing_memory) :
-                _message_definition(&message_definition),
                 _source_partner(source_partner),
-                _crc_offset(crc_offset),
-                _backing_memory(std::move(backing_memory)) {}
+                _backing_memory(std::move(backing_memory)),
+                _message_definition(&message_definition),
+                _crc_offset(crc_offset) {}
 
         inline bool isFinalized() const {
             return _crc_offset >= 0;
@@ -333,7 +333,7 @@ namespace mav {
                 throw std::invalid_argument(StringFormat() << "Field " << field_key <<
                                                         " is not of type char" << StringFormat::end);
             }
-            if (v.size() > field.type.size) {
+            if (static_cast<int>(v.size()) > field.type.size) {
                 throw std::out_of_range(StringFormat() << "String of length " << v.size() <<
                                                        " does not fit in field with size " << field.type.size <<
                                                        StringFormat::end);
@@ -365,7 +365,7 @@ namespace mav {
                     ret_value.resize(field.type.size);
                 }
 
-                if (ret_value.size() < field.type.size) {
+                if (static_cast<int>(ret_value.size()) < field.type.size) {
                     throw std::out_of_range(StringFormat() << "Array of size " << field.type.size <<
                         " can not fit in return type of size " << ret_value.size() << StringFormat::end);
                 }

diff --git a/include/mav/Network.h b/include/mav/Network.h
@@ -80,7 +80,7 @@ namespace mav {
         virtual ConnectionPartner receive(uint8_t* destination, uint32_t size) = 0;
         virtual void markMessageBoundary() {};
         virtual void markSyncing() {};
-        virtual bool isConnectionOriented() const {
+        [[nodiscard]] virtual bool isConnectionOriented() const {
             return false;
         };
     };
@@ -331,8 +331,8 @@ namespace mav {
         NetworkRuntime(const Identifier &own_id, const MessageSet &message_set, NetworkInterface &interface,
                        std::function<void(const std::shared_ptr<Connection>&)> on_connection = {},
                        std::function<void(const std::shared_ptr<Connection>&)> on_connection_lost = {}) :
-                _own_id(own_id), _message_set(message_set),
-                _interface(interface), _parser(_message_set, _interface),
+                _interface(interface), _message_set(message_set),
+                _parser(_message_set, _interface), _own_id(own_id),
                 _on_connection(std::move(on_connection)), _on_connection_lost(std::move(on_connection_lost)) {
 
             _receive_thread = std::thread{
@@ -379,6 +379,7 @@ namespace mav {
         }
 
         std::shared_ptr<Connection> awaitConnection(int timeout_ms = -1) {
+            std::future<std::shared_ptr<Connection>> future;
             {
                 std::lock_guard<std::mutex> lock(_connections_mutex);
                 if (!_connections.empty()) {
@@ -389,15 +390,15 @@ namespace mav {
                     }
                 }
                 _first_connection_promise = std::make_unique<std::promise<std::shared_ptr<Connection>>>();
+                future = _first_connection_promise->get_future();
             }
 
-            auto fut = _first_connection_promise->get_future();
             if (timeout_ms >= 0) {
-                if (fut.wait_for(std::chrono::milliseconds(timeout_ms)) == std::future_status::timeout) {
+                if (future.wait_for(std::chrono::milliseconds(timeout_ms)) == std::future_status::timeout) {
                     throw TimeoutException("Timeout while waiting for first connection");
                 }
             }
-            return fut.get();
+            return future.get();
         }
 
         void setHeartbeatMessage(const Message &message) {

diff --git a/include/mav/UDPClient.h b/include/mav/UDPClient.h
@@ -57,7 +57,7 @@ namespace mav {
         struct sockaddr_in _server_address{};
 
         std::array<uint8_t, RX_BUFFER_SIZE> _rx_buffer;
-        int _bytes_available = 0;
+        uint32_t _bytes_available = 0;
         ConnectionPartner _partner;
 
     public:
@@ -119,7 +119,7 @@ namespace mav {
             return _partner;
         }
 
-        void send(const uint8_t *data, uint32_t size, ConnectionPartner target) override {
+        void send(const uint8_t *data, uint32_t size, ConnectionPartner) override {
             // no need to specify target here, as we called the udp connect function in constructor
             if (sendto(_socket, data, size, 0, (struct sockaddr *) nullptr, 0) < 0) {
                 throw NetworkError("Could not send to socket", errno);

diff --git a/include/mav/UDPServer.h b/include/mav/UDPServer.h
@@ -56,7 +56,7 @@ namespace mav {
         int _socket = -1;
 
         std::array<uint8_t, RX_BUFFER_SIZE> _rx_buffer;
-        int _bytes_available = 0;
+        uint32_t _bytes_available = 0;
         ConnectionPartner _current_partner;
 
     public:

diff --git a/include/mav/utils.h b/include/mav/utils.h
@@ -84,12 +84,16 @@ namespace mav {
 
 
     template <typename T>
-    inline T deserialize(const uint8_t* source, uint32_t deserialize_size) {
+    inline T deserialize(const uint8_t* source, int deserialize_size) {
+        // in case we do not have any bytes to read, we return 0
+        if (deserialize_size <= 0) {
+            return T{0};
+        }
         if (deserialize_size == sizeof(T)) {
             return *static_cast<const T*>(static_cast<const void*>(source));
         } else {
             uint8_t deserialize_buff[sizeof(T)]{};
-            std::copy(source, source + std::min(deserialize_size, static_cast<uint32_t>(sizeof(T))), deserialize_buff);
+            std::copy(source, source + std::min(static_cast<uint32_t>(deserialize_size), static_cast<uint32_t>(sizeof(T))), deserialize_buff);
             return *static_cast<const T*>(static_cast<const void*>(deserialize_buff));
         }
     }

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -10,7 +10,7 @@ add_executable(tests
 
 add_test(NAME tests COMMAND tests)
 
-target_compile_options(tests PRIVATE -O0 -g --coverage)
+target_compile_options(tests PRIVATE -O0 -g --coverage -Wall -Wextra -pedantic)
 target_link_options(tests PRIVATE --coverage)
 target_include_directories(tests PRIVATE ../include)
 target_link_libraries(tests PRIVATE Threads::Threads)
diff --git a/tests/Message.cpp b/tests/Message.cpp
@@ -28,12 +28,18 @@ TEST_CASE("Message set creation") {
             <field type="float[3]" name="float_arr_field">description</field>
             <field type="int32_t[3]" name="int32_arr_field">description</field>
         </message>
+        <message id="9916" name="UINT8_ONLY_MESSAGE">
+            <field type="uint8_t" name="field1">description</field>
+            <field type="uint8_t" name="field2">description</field>
+            <field type="uint8_t" name="field3">description</field>
+            <field type="uint8_t" name="field4">description</field>
+        </message>
     </messages>
 </mavlink>
 )"""");
 
     REQUIRE(message_set.contains("BIG_MESSAGE"));
-    REQUIRE_EQ(message_set.size(), 1);
+    REQUIRE_EQ(message_set.size(), 2);
 
     auto message = message_set.create("BIG_MESSAGE");
     CHECK_EQ(message.id(), message_set.idForMessage("BIG_MESSAGE"));
@@ -212,8 +218,7 @@ TEST_CASE("Message set creation") {
     }
 
     SUBCASE("String at the end of message") {
-        message.set("uint32_field", 0x0);
-        message.set("int8_field", 0x0);
+        message.set("int8_field", 0x00);
         message.set("uint16_field", 0x0);
         message.set("int16_field", 0x0);
         message.set("uint32_field", 0x0);
@@ -236,4 +241,23 @@ TEST_CASE("Message set creation") {
         message.set("char_arr_field", "Hello Worldo!");
         CHECK_EQ(message.get<std::string>("char_arr_field"), "Hello Worldo!");
     }
+
+    SUBCASE("Zero elision works on single byte at end of array") {
+
+        auto this_test_message = message_set.create("UINT8_ONLY_MESSAGE");
+        this_test_message.set("field1", 111);
+        this_test_message.set("field2", 0);
+        this_test_message.set("field3", 0);
+        this_test_message.set("field4", 0);
+
+        uint32_t wire_size = this_test_message.finalize(5, {6, 7});
+        CHECK_EQ(wire_size, 13);
+        CHECK_EQ(this_test_message.get<uint8_t>("field1"), 111);
+        CHECK_EQ(this_test_message.get<uint8_t>("field2"), 0);
+        CHECK_EQ(this_test_message.get<uint8_t>("field3"), 0);
+        CHECK_EQ(this_test_message.get<uint8_t>("field4"), 0);
+    }
+
+
+
 }
diff --git a/tests/TCP.cpp b/tests/TCP.cpp
@@ -48,7 +48,7 @@ TEST_CASE("TCP server client") {
 
         std::promise<void> connection_called_promise;
         auto connection_called_future = connection_called_promise.get_future();
-        server_runtime.onConnection([&connection_called_promise](const std::shared_ptr<mav::Connection> &connection) {
+        server_runtime.onConnection([&connection_called_promise](const std::shared_ptr<mav::Connection>&) {
             connection_called_promise.set_value();
         });
 

diff --git a/tests/UDP.cpp b/tests/UDP.cpp
@@ -61,7 +61,7 @@ TEST_CASE("UDP server client") {
 
         std::promise<void> connection_called_promise;
         auto connection_called_future = connection_called_promise.get_future();
-        server_runtime.onConnection([&connection_called_promise](const std::shared_ptr<mav::Connection> &connection) {
+        server_runtime.onConnection([&connection_called_promise](const std::shared_ptr<mav::Connection>&) {
             connection_called_promise.set_value();
         });