[Linter Addin] Add in Cppcheck linter and fix bugs

.github/workflows/format.yml

@@ -10,4 +10,10 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       - run: sudo apt install clang-format
-      - run: bash scripts/test_clang_format.sh
+      - run: bash scripts/test_clang_format.sh
+  linter:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - run: sudo apt install cppcheck
+      - run: bash scripts/linter.sh

.gitignore

@@ -32,6 +32,7 @@
 *.app
 
 build/
+cppcheckbuild/
 
 # Bazel files
 bazel-*

control_loop/bldc/brushless_control_loop.cpp

@@ -86,7 +86,7 @@ ControlLoop::ControlLoopStatus BrushlessControlLoop::run_current_control(float i
         // Update the id reference
         i_d_reference_ = i_d_reference;
         // Now, run the FOC control loop with the speed divided by the speed to iq gain
-        UNUSED(run(i_q_reference / params_->foc_params.speed_to_iq_gain));
+        IGNORE(run(i_q_reference / params_->foc_params.speed_to_iq_gain));
     }
 
     return status_;

control_loop/bldc/brushless_control_loop.hpp

@@ -136,7 +136,8 @@ class BrushlessControlLoop : public ControlLoop {
     utime_t time_at_start_ = 0;
     utime_t last_run_time_ = 0;
     float rotor_position_open_loop_start_ = 0.0f;
-    float motor_speed_, rotor_position_ = 0;
+    float motor_speed_ = 0.0f;
+    float rotor_position_ = 0;
 
     BrushlessControlLoopType control_loop_type_ = BrushlessControlLoopType::OPEN_LOOP;
 
@@ -146,10 +147,10 @@ class BrushlessControlLoop : public ControlLoop {
     float desired_rotor_angle_open_loop_ = 0.0f;
 
     // duty cycles
-    float duty_cycle_u_h_, duty_cycle_v_h_, duty_cycle_w_h_ = 0.0f;
+    float duty_cycle_u_h_ = 0.0f, duty_cycle_v_h_ = 0.0f, duty_cycle_w_h_ = 0.0f;
 
     // FOC variables
-    float i_quadrature_, i_direct_, V_quadrature_, V_direct_, V_alpha_, V_beta_ = 0.0f;
+    float i_quadrature_ = 0.0f, i_direct_ = 0.0f, V_quadrature_ = 0.0f, V_direct_ = 0.0f, V_alpha_ = 0.0f, V_beta_ = 0.0f;
     float i_d_reference_ = 0.0f;
 
     /**

control_loop/bldc/rotor_estimator.cpp

@@ -72,7 +72,7 @@ bool BldcSensorlessRotorSectorSensor::zero_crossing_detected(
         control_loop::Bldc6StepCommutationTypes::CommutationSignal::Z_RISING;
     float undriven_phase_voltage = 0.0f;
 
-    float bemf_voltages[hwbridge::Bridge3Phase::NUM_PHASES] = {bemf_voltage.u, bemf_voltage.v, bemf_voltage.w};
+    const float bemf_voltages[hwbridge::Bridge3Phase::NUM_PHASES] = {bemf_voltage.u, bemf_voltage.v, bemf_voltage.w};
 
     for (uint8_t i = 0; i < hwbridge::Bridge3Phase::NUM_PHASES; i++) {
         if ((current_commutation_step.signals[i] != control_loop::Bldc6StepCommutationTypes::CommutationSignal::Z_FALLING) &&
@@ -202,15 +202,15 @@ app_hal_status_E BldcElectricalRotorPositionEstimatorFromHall::update(utime_t ti
             math::wraparound(raw_hall_angle_diff, -math::M_PI_FLOAT, math::M_PI_FLOAT);
             const float velocity_previous = velocity_;
 
+            velocity_ = raw_hall_angle_diff / time_delta_since_hall_update;
+
             // Calculate the acceleration if the time delta is greater than 0
             if (time_delta_since_hall_update > 0.0f) {
                 acceleration_ = (velocity_ - velocity_previous) / time_delta_since_hall_update;
             } else {
                 acceleration_ = 0.0f;
             }
 
-            velocity_ = raw_hall_angle_diff / time_delta_since_hall_update;
-
             compensated_velocity_ = velocity_;
 
             rotor_position_ = raw_hall_angle;

control_loop/stepper/stepper_control_loop.hpp

@@ -63,7 +63,7 @@ class StepperControlLoop : public ControlLoop {
 
     void init(StepperControlLoopParams* params);
 
-    ControlLoopStatus run(float speed);
+    ControlLoopStatus run(float speed) override;
 
    protected:
     /**
@@ -81,7 +81,7 @@ class StepperControlLoop : public ControlLoop {
     StepperControlLoopParams* params_ = nullptr;
 
     // Create 2 variables to store the current setpoints for the A and B motors
-    float current_setpoint_a, current_setpoint_b = 0;
+    float current_setpoint_a = 0.0f, current_setpoint_b = 0.0f;
 
     // Create a status object
     StepperControlLoopStatus status_;

hal/hal_common.hpp

@@ -13,9 +13,6 @@ typedef enum {
     APP_HAL_BUFFER_DIMENSION_MISSMATCH,
 } app_hal_status_E;
 
-template <class T>
-void IGNORE(const T&) {}
-
 typedef int64_t utime_t;
 
 #endif

hwbridge/3phase/bridge_3phase_drv8323.hpp

@@ -9,6 +9,7 @@
 #include "hal_common.hpp"
 #include "hal_gpio.hpp"
 #include "hal_timer.hpp"
+#include "util.hpp"
 
 namespace hwbridge {
 
@@ -28,14 +29,8 @@ class Bridge3PhaseDRV8323 : public Bridge3Phase {
     } drv8323_phase_config_info_t;
 
     Bridge3PhaseDRV8323(basilisk_hal::HAL_ComplementaryPWM_Timer& timer, drv8323_phase_config_info_t& u,
-                        drv8323_phase_config_info_t& v, drv8323_phase_config_info_t& w, uint32_t frequency) {
-        // Register the timer and pin
-        pwm_timer_ = &timer;
-        u_ = u;
-        v_ = v;
-        w_ = w;
-        frequency_ = frequency;
-    };
+                        drv8323_phase_config_info_t& v, drv8323_phase_config_info_t& w, uint32_t frequency)
+        : pwm_timer_(&timer), u_(u), v_(v), w_(w), frequency_(frequency){};
     ~Bridge3PhaseDRV8323() = default;
 
     app_hal_status_E init() override {
@@ -122,7 +117,7 @@ class Bridge3PhaseDRV8323 : public Bridge3Phase {
     }
 
     // Define a function to read the current
-    app_hal_status_E read_current(phase_current_t& current) {
+    app_hal_status_E read_current(const phase_current_t& current) {
         IGNORE(current);
         return app_hal_status_E::APP_HAL_NOT_IMPLEMENTED;
     }

scripts/linter.sh

@@ -0,0 +1,22 @@
+mkdir -p cppcheckbuild
+
+# Find all C, C++, and header files in the current directory
+files=$(find . -name "*.c" -o -name "*.cpp" -o -name "*.h" -o -name "*.hpp")
+
+# Remove any files located in build/
+files=$(echo "$files" | grep -v "build/")
+# Remove any files located in libs/
+files=$(echo "$files" | grep -v "libs/")
+# Remove any files located in tests/
+files=$(echo "$files" | grep -v "test/")
+
+# Store a variable to hold the suppressions
+suppressions="--suppress=missingIncludeSystem --suppress=missingInclude --suppress=unusedFunction --suppress=unusedStructMember --suppress=unmatchedSuppression"
+
+# Of the files glob, find only the hpp files
+hpp_files=$(echo "$files" | grep -E "\.hpp$")
+
+# Find the directories of the hpp files. Prepend the -I flag to each directory
+hpp_dirs=$(echo "$hpp_files" | xargs -n1 dirname | sort | uniq | sed -e 's/^/-I/')
+
+cppcheck $files --enable=all $suppressions $hpp_dirs --template=gcc -q --error-exitcode=1 --cppcheck-build-dir=cppcheckbuild

test/rotor_estimator_test.cpp

@@ -56,16 +56,11 @@ TEST(RotorEstimatorTest, test_angle_one_for_one) {
     rotor_estimator.get_rotor_velocity(rotor_velocity);
     EXPECT_FLOAT_EQ(rotor_velocity, compensated_velocity);
 
-    // Expect a call to get the sector position and ensure the reference is updated to return
-    EXPECT_CALL(sector_sensor, get_electrical_angle(_))  // _ allowing any param
-        .WillOnce(DoAll(SetArgReferee<0>(1 * math::M_PI_FLOAT / 3.0), Return(APP_HAL_OK)));
-
-    // Update the rotor position with a utime of 1000
-    rotor_estimator.update(1000);
-
-    // Expect the rotor position to be 2.0f * M_PI / 6.0f
-    rotor_estimator.get_rotor_position(rotor_position);
-    EXPECT_FLOAT_EQ(rotor_position, 1.5f * 2.0f * M_PI / 6.0f);
+    // Expect the acceleration to be (2094.395 - 0) / (500 us) = 4.18879e6 rad/s^2
+    const float actual_acceleration = (actual_velocity - 0.0f) / (500.0f / mock_clock.kMicrosecondsPerSecond);
+    float rotor_acceleration = 0.0f;
+    rotor_estimator.get_rotor_acceleration(rotor_acceleration);
+    EXPECT_FLOAT_EQ(rotor_acceleration, actual_acceleration);
 }
 
 TEST(RotorEstimatorTest, test_angle_underflow) {

util/util.hpp

@@ -1,7 +1,7 @@
 #ifndef UTIL_HPP
 #define UTIL_HPP
 
-// Define an unused macro to silence compiler warnings
-#define UNUSED(x) (void)(x)
+template <class T>
+void IGNORE(const T&) {}
 
 #endif  // UTIL_HPP