Merge pull request #51 from ToolboxPlane/#50

Fixes #50

Src/Application/mode_handler.c

@@ -46,7 +46,7 @@ mode_handler_mode_t mode_handler_handle(imu_data_t *imu_data, remote_data_t *rem
     if (remote_timeout_counter >= REMOTE_TIMEOUT) {
         remote_timeout_counter = REMOTE_TIMEOUT;
         remote_active = false;
-        //error_handler_handle_warning(MODE_HANDLER, MODE_HANDLER_ERROR_NO_REMOTE_DATA);
+        // error_handler_handle_warning(MODE_HANDLER, MODE_HANDLER_ERROR_NO_REMOTE_DATA);
     }
     *remote_data = remote_get_data();
 
@@ -59,7 +59,7 @@ mode_handler_mode_t mode_handler_handle(imu_data_t *imu_data, remote_data_t *rem
     if (flightcomputer_timeout_counter >= FLIGHTCOMPUTER_TIMOUT) {
         flightcomputer_timeout_counter = FLIGHTCOMPUTER_TIMOUT;
         flightcomputer_active = false;
-        //error_handler_handle_warning(MODE_HANDLER, MODE_HANDLER_ERROR_NO_FCP_DATA);
+        // error_handler_handle_warning(MODE_HANDLER, MODE_HANDLER_ERROR_NO_FCP_DATA);
     }
     *flightcomputer_setpoint = flightcomputer_get_setpoint();
 

Src/Components/actuators.c

@@ -5,10 +5,10 @@
  * @brief Implementation of the actuators interface component.
  * @ingroup Components
  */
-#include <Drivers/ppm.h>
-
 #include "actuators.h"
 
+#include <Drivers/ppm.h>
+
 enum { SERVO_CENTRE_POSITION = 500U };
 
 void actuators_init(void) {

Src/Components/imu.c

@@ -18,6 +18,9 @@ enum { MAG_CALIB_TRESH = 2 };
 enum { GYR_CALIB_TRESH = 2 };
 enum { SYS_CALIB_TRESH = 3 };
 
+/**
+ * State of the internal sampling state machine.
+ */
 typedef enum {
     EUL,
     GYR,
@@ -26,10 +29,12 @@ typedef enum {
     CALIB_STAT,
 } bno_sampling_state_t;
 
-static volatile bno055_response_t init_response;
-static volatile bool callback_ready;
-static volatile bno_sampling_state_t bno_sampling_state;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) only read if callback ready, behaviour checked
+static volatile bno055_response_t init_response = write_fail;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) clear interaction in init, always properly set
+static volatile bool callback_ready = false;
 
+static volatile bno_sampling_state_t bno_sampling_state = EUL;
 static volatile imu_data_t imu_datas[2];
 static bno055_calib_status_t calib_status;
 static bno055_status_t bno_status;
@@ -70,7 +75,7 @@ static void bno_sample_callback(bno055_response_t response) {
             case CALIB_STAT:
                 current_sample_state_id = 1 - current_sample_state_id;
                 if (calib_status.sys_status < SYS_CALIB_TRESH || calib_status.gyr_status < GYR_CALIB_TRESH ||
-                    calib_status.acc_status < GYR_CALIB_TRESH || calib_status.mag_status < MAG_CALIB_TRESH) {
+                    calib_status.acc_status < ACC_CALIB_TRESH || calib_status.mag_status < MAG_CALIB_TRESH) {
                     imu_data->imu_ok = false;
                 }
                 sampling_complete = true;

Src/Drivers/CMakeLists.txt

@@ -1,3 +1,3 @@
 project(Drivers)
-add_library(${PROJECT_NAME} STATIC bno055.c sbus.c bno055_uart.c ppm.c protobuf.c)
+add_library(${PROJECT_NAME} STATIC bno055.c sbus.c bno055_uart.c ppm.c protobuf.c ring_buffer.c)
 target_link_libraries(${PROJECT_NAME} PUBLIC AvrHAL ToolboxPlaneMessages)

Src/Drivers/protobuf.c

@@ -12,29 +12,25 @@
 #include <Messages/MessageEncoding.h>
 #include <Messages/MessageIds.h>
 
+#include "ring_buffer.h"
+
 enum { UART_ID = 0 };
 enum { UART_BAUD = 115200U };
 
-enum { RX_BUF_SIZE = 512 };
-
-static volatile uint8_t rx_buf[RX_BUF_SIZE];
-static volatile uint8_t rx_head, rx_tail;
+static ring_buffer_data_t ring_buffer_data;
 static message_decoding_data_t message_decoding_data;
 static setpoint_message_t setpoint_message;
 
 static void uart_callback(uint8_t data) {
-    rx_buf[rx_head] = data;
-    rx_head = (rx_head + 1) % RX_BUF_SIZE;
-    if (rx_tail == rx_head) {
+    if (!ring_buffer_put(&ring_buffer_data, data)) {
         // TODO error handling
     }
 }
 
 void protobuf_init(void) {
-    uart_init(UART_ID, UART_BAUD, NONE, 1, uart_callback);
-    rx_head = 0;
-    rx_tail = 0;
     message_decoding_init(&message_decoding_data, FC_SP_ID);
+    ring_buffer_data = ring_buffer_init();
+    uart_init(UART_ID, UART_BAUD, NONE, 1, uart_callback);
 }
 
 void protobuf_send(const fc_message_t *message) {
@@ -45,10 +41,8 @@ void protobuf_send(const fc_message_t *message) {
 
 bool protobuf_setpoint_available(void) {
     bool res = false;
-    while (rx_tail != rx_head) {
-        uint8_t data = rx_buf[rx_tail];
-        rx_tail = (rx_tail + 1) % RX_BUF_SIZE;
-
+    uint8_t data = 0;
+    while (ring_buffer_get(&ring_buffer_data, &data)) {
         if (message_decoding_decode(&message_decoding_data, data, &ToolboxPlaneMessages_FlightControllerSetpoint_msg,
                                     &setpoint_message)) {
             res = true;

Src/Drivers/ring_buffer.c

@@ -0,0 +1,33 @@
+/**
+ * @file ring_buffer.c
+ * @author Paul Nykiel
+ * @date 27.12.22
+ * @brief Implemtation of the fixed-size, byte sized object ring-buffer utility module .
+ * @ingroup Drivers
+ */
+#include "ring_buffer.h"
+
+ring_buffer_data_t ring_buffer_init(void) {
+    ring_buffer_data_t uart_buffer_data = {.buf = {0}, .read_index = 0, .len = 0};
+    return uart_buffer_data;
+}
+
+bool ring_buffer_put(ring_buffer_data_t *ring_buffer_data, uint8_t data) {
+    if (ring_buffer_data->len == RING_BUFFER_SIZE) {
+        return false;
+    }
+    uint16_t index = (ring_buffer_data->read_index + ring_buffer_data->len) % RING_BUFFER_SIZE;
+    ring_buffer_data->buf[index] = data;
+    ring_buffer_data->len += 1;
+    return true;
+}
+
+bool ring_buffer_get(ring_buffer_data_t *ring_buffer_data, uint8_t *out) {
+    if (ring_buffer_data->len > 0) {
+        *out = ring_buffer_data->buf[ring_buffer_data->read_index];
+        ring_buffer_data->read_index = (ring_buffer_data->read_index + 1) % RING_BUFFER_SIZE;
+        ring_buffer_data->len -= 1;
+        return true;
+    }
+    return false;
+}

Src/Drivers/ring_buffer.h

@@ -0,0 +1,50 @@
+/**
+ * @file ring_buffer.h
+ * @author Paul Nykiel
+ * @date 27.12.22
+ * @brief Declaration of the fixed-size, byte sized object ring-buffer utility module .
+ * @ingroup Drivers
+ */
+#ifndef FLIGHTCONTROLLER_RING_BUFFER_H
+#define FLIGHTCONTROLLER_RING_BUFFER_H
+
+#include <stdbool.h>
+#include <stdint.h>
+
+/**
+ * Size of the ring-buffer, i.e. number of elements/bytes.
+ */
+enum { RING_BUFFER_SIZE = 512 };
+
+/**
+ * Internal data for the ring buffer
+ */
+typedef struct {
+    uint8_t buf[RING_BUFFER_SIZE]; ///< Buffer for the data
+    uint16_t read_index;           ///< Index of the next data to read (i.e. oldest data)
+    uint16_t len;                  ///< Number of elements
+} ring_buffer_data_t;
+
+/**
+ * Create a correctly initialized ring_buffer_data_t object.
+ * @return an ring_buffer_data_t for usage with the other functions.
+ */
+ring_buffer_data_t ring_buffer_init(void);
+
+/**
+ * Write a byte to the ring buffer.
+ * @param ring_buffer_data the internal data of the ring-buffer
+ * @param data the byte to add
+ * @return true if the object could be added, false if the buffer is already full
+ */
+bool ring_buffer_put(ring_buffer_data_t *ring_buffer_data, uint8_t data);
+
+/**
+ * Get a byte from the ring buffer if available.
+ * @param ring_buffer_data the internal data of the ring-buffer
+ * @param out the byte that got read
+ * @return true if a byte was available, otherwise false
+ */
+bool ring_buffer_get(ring_buffer_data_t *ring_buffer_data, uint8_t *out);
+
+#endif // FLIGHTCONTROLLER_RING_BUFFER_H

Src/Drivers/sbus.c

@@ -10,25 +10,22 @@
 
 #include <HAL/uart.h>
 
+#include "ring_buffer.h"
+
 enum { UART_ID = 2 };
 enum { UART_BAUD = 100000 };
 
-enum { RX_BUF_SIZE = 512 };
-
 enum { SBUS_START_BYTE = 0x0F };
 enum { SBUS_END_BYTE = 0x00 };
 enum { SBUS_BITS_PER_CHANNEL = 11U };
 
-static volatile uint8_t rx_buf[RX_BUF_SIZE];
-static volatile uint8_t rx_head, rx_tail;
-static volatile sbus_data_t latest_data;
-static volatile sbus_data_t curr_decode_data;
-static volatile uint8_t decode_byte_count;
+static ring_buffer_data_t ring_buffer_data;
+static sbus_data_t latest_data;
+static sbus_data_t curr_decode_data;
+static uint8_t decode_byte_count;
 
 static void uart_callback(uint8_t data) {
-    rx_buf[rx_head] = data;
-    rx_head = (rx_head + 1) % RX_BUF_SIZE;
-    if (rx_tail == rx_head) {
+    if (ring_buffer_put(&ring_buffer_data, data)) {
         // TODO error handling
     }
 }
@@ -72,20 +69,15 @@ static bool sbus_decode(uint8_t data) {
 }
 
 void sbus_init(void) {
-    uart_init(UART_ID, UART_BAUD, EVEN, 2, uart_callback);
-    rx_head = 0;
-    rx_tail = 0;
     decode_byte_count = 0;
-    latest_data.failsafe = true;
-    latest_data.frame_lost = true;
+    ring_buffer_data = ring_buffer_init();
+    uart_init(UART_ID, UART_BAUD, EVEN, 2, uart_callback);
 }
 
 bool sbus_data_available(void) {
     bool res = false;
-    while (rx_tail != rx_head) {
-        uint8_t data = rx_buf[rx_tail];
-        rx_tail = (rx_tail + 1) % RX_BUF_SIZE;
-
+    uint8_t data = 0;
+    while (ring_buffer_get(&ring_buffer_data, &data)) {
         if (sbus_decode(data)) {
             for (uint8_t index = 0U; index < SBUS_NUM_CHANNELS; ++index) {
                 latest_data.channel[index] = curr_decode_data.channel[index];

Src/Drivers/sbus.h

@@ -22,8 +22,8 @@ enum { SBUS_NUM_CHANNELS = 16 };
  */
 typedef struct {
     uint16_t channel[SBUS_NUM_CHANNELS]; ///< The channel data, each channel is in [0, 2047] (11 bit)
-    bool failsafe; ///< Fail-safe bit of the package, set by the receiver
-    bool frame_lost; ///< Frame lost bit of the package set by the receiver
+    bool failsafe;                       ///< Fail-safe bit of the package, set by the receiver
+    bool frame_lost;                     ///< Frame lost bit of the package set by the receiver
 } sbus_data_t;
 
 /**

Tests/LowLevel/CMakeLists.txt

@@ -6,9 +6,6 @@ target_link_libraries(Test_Drivers_bno055_uart PRIVATE Mock_HAL_uart)
 make_test(Drivers/bno055)
 target_link_libraries(Test_Drivers_bno055 PRIVATE Mock_Drivers_bno055_uart)
 
-make_test(Drivers/sbus)
-target_link_libraries(Test_Drivers_sbus PRIVATE Mock_HAL_uart)
-
 make_test(Drivers/ppm)
 target_link_libraries(Test_Drivers_ppm PRIVATE Mock_HAL_pwm16bit)
 target_link_libraries(Test_Drivers_ppm PRIVATE Mock_avr_io)
@@ -17,6 +14,13 @@ make_test(Drivers/protobuf)
 target_link_libraries(Test_Drivers_protobuf PRIVATE Mock_HAL_uart)
 target_link_libraries(Test_Drivers_protobuf PRIVATE Mock_Messages_MessageDecoding)
 target_link_libraries(Test_Drivers_protobuf PRIVATE Mock_Messages_MessageEncoding)
+target_link_libraries(Test_Drivers_protobuf PRIVATE Mock_Drivers_ring_buffer)
+
+make_test(Drivers/ring_buffer)
+
+make_test(Drivers/sbus)
+target_link_libraries(Test_Drivers_sbus PRIVATE Mock_HAL_uart)
+target_link_libraries(Test_Drivers_sbus PRIVATE Mock_Drivers_ring_buffer)
 
 make_test(Components/imu)
 target_link_libraries(Test_Components_imu PRIVATE Mock_Drivers_bno055)

Tests/LowLevel/Components/imu.cpp

@@ -54,7 +54,7 @@ TEST(TEST_NAME, init__config_mode_error) {
             [](auto /*op_mode*/, bno055_callback_t callback) { callback(write_fail); });
 
     imu_init();
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail + 1));
     EXPECT_FALSE(imu_get_latest_data().imu_ok);
     EXPECT_FALSE(imu_data_available());
 }
@@ -75,7 +75,7 @@ TEST(TEST_NAME, init__self_test_error) {
     });
 
     imu_init();
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, read_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, read_fail + 1));
     EXPECT_FALSE(imu_get_latest_data().imu_ok);
     EXPECT_FALSE(imu_data_available());
 }
@@ -185,7 +185,7 @@ TEST(TEST_NAME, init__unit_sel_error) {
                bno055_callback_t callback) { callback(write_fail); });
 
     imu_init();
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail + 1));
     EXPECT_FALSE(imu_get_latest_data().imu_ok);
     EXPECT_FALSE(imu_data_available());
 }
@@ -214,7 +214,7 @@ TEST(TEST_NAME, init__remap_fail) {
                bno055_axis_remap_axis_t /*new_z*/, bno055_callback_t callback) { callback(write_fail); });
 
     imu_init();
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail + 1));
     EXPECT_FALSE(imu_get_latest_data().imu_ok);
     EXPECT_FALSE(imu_data_available());
 }
@@ -246,7 +246,7 @@ TEST(TEST_NAME, init__remap_sign_fail) {
                bno055_axis_remap_sign_t /*new_z_sign*/, bno055_callback_t callback) { callback(write_fail); });
 
     imu_init();
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail + 1));
     EXPECT_FALSE(imu_get_latest_data().imu_ok);
 }
 
@@ -282,7 +282,7 @@ TEST(TEST_NAME, init__ndof_fail) {
                bno055_axis_remap_sign_t /*new_z_sign*/, bno055_callback_t callback) { callback(write_success); });
 
     imu_init();
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_error>(BNO055, write_fail + 1));
     EXPECT_FALSE(imu_get_latest_data().imu_ok);
     EXPECT_FALSE(imu_data_available());
 }
@@ -413,7 +413,7 @@ TEST(TEST_NAME, read__error_call_error_handler) {
     auto data = imu_get_latest_data();
 
     EXPECT_EQ(data.imu_ok, false);
-    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_warning>(BNO055, read_fail+1));
+    EXPECT_TRUE(errorHandlerHandle.functionGotCalled<error_handler_handle_warning>(BNO055, read_fail + 1));
 }
 
 TEST(TEST_NAME, read__error_restart) {

Tests/LowLevel/Components/system.cpp

@@ -111,7 +111,7 @@ TEST(TEST_NAME, timer_runtime_under_limit) {
     timer_8bit_callback_t internalCallback = nullptr;
     timerHandle.overrideFunc<timer_8bit_init>(
             [&internalCallback](timer_8bit_clock_option_t /*clock_option*/, system_timer_callback_t timerCallback) {
-              internalCallback = timerCallback;
+                internalCallback = timerCallback;
             });
     // 12/16,384*255=186.767578125
     timerHandle.overrideFunc<timer_8bit_get_count>([]() { return 186; });
@@ -134,7 +134,7 @@ TEST(TEST_NAME, timer_runtime_over_limit) {
     timer_8bit_callback_t internalCallback = nullptr;
     timerHandle.overrideFunc<timer_8bit_init>(
             [&internalCallback](timer_8bit_clock_option_t /*clock_option*/, system_timer_callback_t timerCallback) {
-              internalCallback = timerCallback;
+                internalCallback = timerCallback;
             });
     // 12/16,384*255=186.767578125
     timerHandle.overrideFunc<timer_8bit_get_count>([]() { return 187; });