Merge pull request #8 from R2D2-2019/feature-external_comm_frames

A couple of fixes before everyone uses the examples

README.md

@@ -259,7 +259,7 @@ Directly connecting the Due to these types of components will likely damage or d
 
 ### Frame structure
 A CAN frame within the project has the following structure:
-[![CAN-frame-1.png](https://i.postimg.cc/RFgPWWKV/CAN-frame-1.png)](https://postimg.cc/njQK5c36)
+[![CAN-frame-1-1.png](https://i.postimg.cc/9fMyyFJb/CAN-frame-1-1.png)](https://postimg.cc/18kgQPxV)
 
 Explanation:
  - ID: the priority (channel) of the frame. Lower value = higher priority.

code/headers/base_comm.hpp

@@ -94,7 +94,7 @@ namespace r2d2 {
          * @param buffer
          * @param prio
          */
-        virtual void send_impl(const frame_type &type, const uint8_t data[], const priority prio) = 0;
+        virtual void send_impl(const frame_type &type, const uint8_t data[], const size_t length, const priority prio) = 0;
 
     public:
         /**
@@ -116,29 +116,54 @@ namespace r2d2 {
         template<
             typename T,
             typename = std::enable_if_t<
-               is_suitable_frame_v<T> && !is_extended_frame_v<T>
+               is_suitable_frame_v<T>
             >
         >
         void send(const T &data, const priority prio = priority::NORMAL) {
-            uint8_t buffer[8] = {};
-
-            memcpy(
-                (void *) buffer,
-                (const void *) &data,
-                sizeof(T)
-            );
-
             send_impl(
-                static_cast<frame_type>(frame_type_v<T>), buffer, prio
+                static_cast<frame_type>(frame_type_v<T>),
+                reinterpret_cast<const uint8_t *>(&data),
+                sizeof(T),
+                prio
             );
         }
 
+        /**
+         * Send the given data to an external system
+         * with the given priority.
+         *
+         * @tparam T
+         * @param id
+         * @param data
+         * @param prio
+         */
+        template<
+            typename T,
+            typename = std::enable_if_t<
+                is_suitable_frame_v<T>
+            >
+        >
+        void send(const external_id_s &id, const T &data, const priority prio = priority::NORMAL) {
+            // No {} needed, since all fields are filled.
+            // Adding it will cause a call to memset
+            frame_external_s frame;
+
+            for(uint_fast8_t i = 0; i < sizeof(T); i++){
+                frame.data[i] = data[i];
+            }
+
+            frame.length = sizeof(T);
+            frame.id = id;
+
+            send(frame, prio);
+        }
+
         /**
          * Listen for the given list of packets.
          *
          * @param listen_for
          */
-        void listen_for_frames(const std::array<frame_id, 8> listen_for) {
+        void listen_for_frames(std::array<frame_id, 8> listen_for /* Copy to allow move */) {
             this->listen_for = listen_for;
 
             // Sort to enable binary search

code/headers/base_module.hpp

@@ -15,7 +15,7 @@ namespace r2d2 {
         /**
          * @param comm
          */
-        explicit module_c(base_comm_c &comm)
+        explicit base_module_c(base_comm_c &comm)
             : comm(comm) { }
 
         /**

code/headers/can.hpp

@@ -343,9 +343,9 @@ namespace r2d2::can_bus {
                 frame.mode = (id >> 25) & 0x01;
                 frame.frame_type = (id >> 10) & 0xFF;
                 frame.sequence_id = (id >> 5) & 0x1F;
-                frame.sequence_total = (id >> 5) & 0x1F;
+                frame.sequence_total = id & 0x1F;
             }
-                // Standard ID, this is an error.
+            // Standard ID, this is an error.
             else {
                 retval = mailbox_status::RX_FAILURE_STD_RECEIVED;
                 return retval;
@@ -386,12 +386,13 @@ namespace r2d2::can_bus {
         template<typename Bus>
         void _write_tx_registers(const _can_frame_s &frame, const uint8_t index) {
             // Set sequence id and total
-            port<Bus>->CAN_MB[index].CAN_MID |=
+            port<Bus>->CAN_MB[index].CAN_MID =
                 ((index + 1) << 26) |
                 ((frame.mode & 0x01) << 25) |
                 (frame.frame_type << 10) |
                 ((frame.sequence_id & 0x1F) << 5) |
-                (frame.sequence_total & 0x1F);
+                (frame.sequence_total & 0x1F) |
+                CAN_MID_MIDE;
 
             // Set the data length on the mailbox.
             port<Bus>->CAN_MB[index].CAN_MCR =

code/headers/channel.hpp

@@ -7,6 +7,7 @@
 #include "can.hpp"
 #include <queue.hpp>
 #include <ringbuffer.hpp>
+#include <cmath>
 #include "base_comm.hpp"
 
 namespace r2d2::can_bus {
@@ -98,6 +99,38 @@ namespace r2d2::can_bus {
         // send interrupt.
         inline static queue_c<detail::_can_frame_s, 32> tx_queue;
 
+        /**
+         * Volatile flag that is used to signify that there is space
+         * in the tx_queue. The tx_queue is emptied in an interrupt and the
+         * send_frame function will wait until there is space in the tx_queue before
+         * it continues execution.
+         *
+         * However "using while(tx_queue.full()) {}" will cause the compiler to
+         * optimize this to a while(true); loop. This is not the desired behaviour.
+         * We can't mark the tx_queue itself als volatile, since that would require marking
+         * all its member functions as volatile as well. So a flag that is cleared each time
+         * the size of tx_queue is reduced is the best option.
+         */
+        inline volatile static bool space_in_tx_queue = true;
+
+        /**
+         * Place a frame into the tx_queue, accounting for
+         * a full transfer queue and waiting for the interrupt
+         * to resolve.
+         *
+         * @param frame
+         */
+        static void safely_push_frame(const detail::_can_frame_s &frame) {
+            if (tx_queue.full()) {
+                space_in_tx_queue = false;
+            }
+
+            // Wait for space, space is created in the interrupt
+            while (!space_in_tx_queue);
+
+            tx_queue.push(frame);
+        }
+
     public:
         /**
          * Initialize the channel mailboxes.
@@ -132,9 +165,7 @@ namespace r2d2::can_bus {
             frame.length = 0;
             frame.frame_type = type;
 
-            while (tx_queue.full()) {}
-
-            tx_queue.push(frame);
+            safely_push_frame(frame);
 
             // Enabling the interrupt on the CAN mailbox with the TX id
             // will cause a interrupt when the CAN controller is ready.
@@ -148,22 +179,60 @@ namespace r2d2::can_bus {
          *
          * @param frame
          */
-        template<typename T>
-        static void send_frame(const T &data) {
-            detail::_can_frame_s frame{};
+        static void send_frame(const frame_type &type, const uint8_t *data, const size_t length) {
+            /*
+             * If the frame is "simple" (less than or equal to 8 bytes), it will
+             * fit on the transport in one frame. Otherwise, a sequence
+             * is created.
+             */
+            if (length <= 8) {
+                detail::_can_frame_s frame{};
+
+                for(size_t i = 0; i < length; i++){
+                    frame.data.bytes[i] = data[i];
+                }
 
-            memcpy(
-                (void *) frame.data.bytes,
-                (const void *) &data,
-                sizeof(T)
-            );
+                frame.length = length;
+                frame.frame_type = type;
 
-            frame.length = sizeof(T);
-            frame.frame_type = frame_type_v<T>;
+                safely_push_frame(frame);
+            } else {
+                const uint_fast8_t total = length / 8;
+                const uint_fast8_t remainder = length % 8;
 
-            while (tx_queue.full()) {}
+                // First, create the bulk of the frame.
+                for (uint_fast8_t i = 0; i < total; i++) {
+                    detail::_can_frame_s frame{};
 
-            tx_queue.push(frame);
+                    // Has to be 8 bytes; frame.length is copied in a lower layer
+                    for(uint_fast8_t j = 0; j < 8; j++){
+                        frame.data.bytes[j] = data[j + i];
+                    }
+
+                    frame.length = 8;
+                    frame.frame_type = type;
+                    frame.sequence_id = i;
+                    frame.sequence_total = total + (remainder > 0);
+
+                    safely_push_frame(frame);
+                }
+
+                // Handle any remaining bytes
+                if (remainder > 0) {
+                    detail::_can_frame_s frame{};
+
+                    for(uint_fast8_t i = 0; i < remainder; i++){
+                        frame.data.bytes[i] = data[i + total];
+                    }
+
+                    frame.length = remainder;
+                    frame.frame_type = type;
+                    frame.sequence_id = total;
+                    frame.sequence_total = total + 1;
+
+                    safely_push_frame(frame);
+                }
+            }
 
             // Enabling the interrupt on the CAN mailbox with the TX id
             // will cause a interrupt when the CAN controller is ready.
@@ -195,6 +264,7 @@ namespace r2d2::can_bus {
                     // Put the data on the bus
                     const auto frame = tx_queue.copy_and_pop();
                     detail::_write_tx_registers<Bus>(frame, ids::tx);
+                    space_in_tx_queue = true;
                 } else {
                     // Nothing left to send, disable interrupt on the tx mailbox
                     port<Bus>->CAN_IDR = (0x01 << ids::tx);
@@ -212,11 +282,9 @@ namespace r2d2::can_bus {
             frame.type = static_cast<frame_type>(can_frame.frame_type);
             frame.request = can_frame.mode == detail::_can_frame_mode::READ;
 
-            memcpy(
-                (void *) frame.bytes,
-                (const void *) can_frame.data.bytes,
-                8 // Has to be 8 bytes; frame.length is copied in a lower layer
-            );
+            for(uint_fast8_t i = 0; i < 8; i++){
+                frame.bytes[i] = can_frame.data.bytes[i];
+            }
 
             using regs = comm_module_register_s;
 

code/headers/comm.hpp

@@ -51,25 +51,28 @@ namespace r2d2 {
          * @param buffer
          * @param prio
          */
-        void send_impl(const frame_type &type, const uint8_t data[], const priority prio) override {
+        void send_impl(const frame_type &type, const uint8_t data[], const size_t length, const priority prio) override {
             if (prio == priority::NORMAL) {
-                channel<priority::NORMAL>::send_frame(data);
+                channel<priority::NORMAL>::send_frame(type, data, length);
             } else if (prio == priority::HIGH) {
-                channel<priority::HIGH>::send_frame(data);
+                channel<priority::HIGH>::send_frame(type, data, length);
             } else if (prio == priority::LOW) {
-                channel<priority::LOW>::send_frame(data);
+                channel<priority::LOW>::send_frame(type, data, length);
             } else {
-                channel<priority::DATA_STREAM>::send_frame(data);
+                channel<priority::DATA_STREAM>::send_frame(type, data, length);
+            }
+
+            // Only internally distribute when needed
+            if (can_bus::comm_module_register_s::count <= 1) {
+                return;
             }
 
             frame_s frame{};
             frame.type = type;
 
-            memcpy(
-                (void *) frame.bytes,
-                (const void *) &data,
-                8
-            );
+            for(uint_fast8_t i = 0; i < 8; i++){
+                frame.bytes[i] = data[i];
+            }
 
             distribute_frame_internally(frame);
         }
@@ -116,6 +119,11 @@ namespace r2d2 {
                 channel<priority::DATA_STREAM>::request_frame(type);
             }
 
+            // Only internally distribute when needed
+            if (can_bus::comm_module_register_s::count <= 1) {
+                return;
+            }
+
             frame_s frame;
             frame.type = type;
             frame.request = true;

code/headers/mock_bus.hpp

@@ -17,7 +17,8 @@ namespace r2d2 {
           * @param buffer
           * @param prio
           */
-        void send_impl(const frame_type &type, const uint8_t data[], const priority prio) override {
+        void send_impl(const frame_type &type, const uint8_t data[], const size_t length, const priority prio) override {
+            // TODO: large frame support
             frame_s frame{};
             frame.type = type;
 

code/headers/packet_types.hpp

@@ -57,6 +57,7 @@ namespace r2d2 {
         ACTIVITY_LED_STATE,
 
         // Don't touch
+        EXTERNAL,
         ALL,
         COUNT
     };
@@ -104,7 +105,31 @@ namespace r2d2 {
     template<typename T>
     constexpr frame_id frame_type_v = frame_type_s<T>::type;
 
+    /**
+    * A struct that helps to describe
+    * an external system address.
+    * Might change, depending on the external
+    * communication module.
+    */
+    struct external_id_s {
+        // 3th and 4th octet
+        uint8_t octets[2];
+    };
+
+    /**
+     * This frame describes a frame meant for external
+     * systems. Tparam T describes the actual frame being send;
+     * this structs wraps the internal frame.
+     *
+     * @tparam T
+     */
+    struct frame_external_s {
+        uint8_t data[256];
+        uint8_t length;
+        external_id_s id;
+    };
 
+    R2D2_INTERNAL_FRAME_HELPER(frame_external_s, EXTERNAL)
 
     /** USER STRUCTS */
 

examples/led_button/button/module.hpp

@@ -41,10 +41,10 @@ namespace r2d2::button {
                 // Get button state, create frame and send
                 frame_button_state_s button_state;
 
-                button_state.pressed = button.read();
+                button_state.pressed = !button.read();
 
                 comm.send(button_state);
             }
         }
-    }
+    };
 }

examples/led_button/controller/module.hpp

@@ -44,5 +44,5 @@ namespace r2d2::controller {
                 comm.send(led_state);
             }
         }
-    }
+    };
 }