ci: add static analysis action (#137)

* ci: add static analysis action (need to figure out / make a good build target for it

* suppress cstyleCast

* fix: static analysis findings

* fix: builds

* fix sa

* ignore magic_enum.hpp in static analysis

* add maybe_unused to variable in generated states; update config in ads1x15

* update to suppress magic_enum as well?

* doc: rebuild

* WIP fixing magic_enum SA erorrs

* replace magic_enum header in state_machine include folder with magic_enum external git submodule (also updated from magic enum 0.8 to 0.9.5 (master)

* remove unused config in static analysis (esp. since with the magic_enum update the SA doesnt take more than a minute or two :) )

Author: finger563

.github/workflows/static_analysis.yml

@@ -0,0 +1,25 @@
+name: Static analysis
+
+on: [pull_request]
+
+jobs:
+  static_analysis:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout repo
+      uses: actions/checkout@v2
+      with:
+        submodules: 'recursive'
+
+    - name: Run static analysis
+      uses: esp-cpp/StaticAnalysis@master
+      with:
+        # Do not build the project and do not use cmake to generate compile_commands.json
+        use_cmake: false
+
+        # Use the 5.2 release version since it's what we build with
+        esp_idf_version: release/v5.2
+
+        # (Optional) cppcheck args
+        cppcheck_args: -i$GITHUB_WORKSPACE/lib -i$GITHUB_WORKSPACE/external -i$GITHUB_WORKSPACE/components/esp_littlefs -i$GITHUB_WORKSPACE/components/lvgl -i$GITHUB_WORKSPACE/components/esp-dsp --force --enable=all --inline-suppr --inconclusive --platform=mips32 --suppressions-list=$GITHUB_WORKSPACE/suppressions.txt

.gitmodules

@@ -25,3 +25,6 @@
 [submodule "lib/pybind11"]
 	path = lib/pybind11
 	url = git@github.com:pybind/pybind11
+[submodule "external/magic_enum"]
+	path = external/magic_enum
+	url = git@github.com:neargye/magic_enum

components/adc/include/continuous_adc.hpp

@@ -3,6 +3,7 @@
 #include <atomic>
 #include <optional>
 #include <unordered_map>
+#include <vector>
 
 #include "esp_adc/adc_cali.h"
 #include "esp_adc/adc_cali_scheme.h"
@@ -48,17 +49,15 @@ class ContinuousAdc {
    * @brief Initialize and start the continuous adc reader.
    * @param config Config used to initialize the reader.
    */
-  ContinuousAdc(const Config &config)
+  explicit ContinuousAdc(const Config &config)
       : sample_rate_hz_(config.sample_rate_hz), window_size_bytes_(config.window_size_bytes),
         num_channels_(config.channels.size()), conv_mode_(config.convert_mode),
+        result_data_(window_size_bytes_, 0xcc),
         logger_({.tag = "Continuous Adc",
                  .rate_limit = std::chrono::milliseconds(100),
                  .level = config.log_level}) {
     // initialize the adc continuous subsystem
     init(config.channels);
-    // allocate memory for the task to store result data from DMA
-    result_data_ = new uint8_t[window_size_bytes_];
-    memset(result_data_, 0xcc, window_size_bytes_);
     // and start the task
     using namespace std::placeholders;
     task_ =
@@ -78,12 +77,11 @@ class ContinuousAdc {
     vTaskNotifyGiveFromISR(task_handle_, &mustYield);
     // then stop the task
     task_->stop();
-    delete[] result_data_;
     stop();
     ESP_ERROR_CHECK(adc_continuous_deinit(handle_));
     // clean up the calibration data
-    for (auto &config : configs_) {
-      auto id = get_id(config);
+    for (const auto &config : configs_) {
+      [[maybe_unused]] auto id = get_id(config);
 #if ADC_CALI_SCHEME_CURVE_FITTING_SUPPORTED
       ESP_ERROR_CHECK(adc_cali_delete_scheme_curve_fitting(cali_handles_[id]));
 #elif ADC_CALI_SCHEME_LINE_FITTING_SUPPORTED
@@ -173,14 +171,14 @@ class ContinuousAdc {
     // wait until conversion is ready (will be notified by the registered
     ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
     auto current_timestamp = std::chrono::high_resolution_clock::now();
-    for (auto &config : configs_) {
+    for (const auto &config : configs_) {
       auto id = get_id(config);
       sums_[id] = 0;
       num_samples_[id] = 0;
     }
     esp_err_t ret;
     uint32_t ret_num = 0;
-    ret = adc_continuous_read(handle_, result_data_, window_size_bytes_, &ret_num, 0);
+    ret = adc_continuous_read(handle_, result_data_.data(), window_size_bytes_, &ret_num, 0);
     if (ret == ESP_ERR_TIMEOUT) {
       // this is ok, we read faster than the hardware could give us data
       return false;
@@ -341,21 +339,20 @@ class ContinuousAdc {
     bool calibrated = false;
 
 #if ADC_CALI_SCHEME_CURVE_FITTING_SUPPORTED
-    if (!calibrated) {
-      logger_.info("calibration scheme version is {}", "Curve Fitting");
-      adc_cali_curve_fitting_config_t cali_config = {
-          .unit_id = unit,
-          .atten = atten,
-          .bitwidth = bitwidth,
-      };
-      ret = adc_cali_create_scheme_curve_fitting(&cali_config, &handle);
-      if (ret == ESP_OK) {
-        calibrated = true;
-      }
+    logger_.info("calibration scheme version is {}", "Curve Fitting");
+    adc_cali_curve_fitting_config_t cali_config = {
+        .unit_id = unit,
+        .atten = atten,
+        .bitwidth = bitwidth,
+    };
+    ret = adc_cali_create_scheme_curve_fitting(&cali_config, &handle);
+    if (ret == ESP_OK) {
+      calibrated = true;
     }
 #endif
 
 #if ADC_CALI_SCHEME_LINE_FITTING_SUPPORTED
+    // cppcheck-suppress knownConditionTrueFalse
     if (!calibrated) {
       logger_.info("calibration scheme version is {}", "Line Fitting");
       adc_cali_line_fitting_config_t cali_config = {
@@ -373,6 +370,7 @@ class ContinuousAdc {
     *out_handle = handle;
     if (ret == ESP_OK) {
       logger_.info("Calibration Success");
+      // cppcheck-suppress knownConditionTrueFalse
     } else if (ret == ESP_ERR_NOT_SUPPORTED || !calibrated) {
       logger_.warn("eFuse not burnt, skip software calibration");
     } else {
@@ -399,11 +397,11 @@ class ContinuousAdc {
   adc_continuous_handle_t handle_;
   adc_digi_convert_mode_t conv_mode_;
   adc_digi_output_format_t output_format_;
+  std::vector<uint8_t> result_data_;
   Logger logger_;
   std::unique_ptr<Task> task_;
   TaskHandle_t task_handle_{NULL};
   std::mutex data_mutex_;
-  uint8_t *result_data_;
 
   std::atomic<bool> running_{false};
 

components/adc/include/oneshot_adc.hpp

@@ -43,7 +43,8 @@ class OneshotAdc {
    * @brief Initialize the oneshot adc reader.
    * @param config Config used to initialize the reader.
    */
-  OneshotAdc(const Config &config) : logger_({.tag = "Oneshot Adc", .level = config.log_level}) {
+  explicit OneshotAdc(const Config &config)
+      : logger_({.tag = "Oneshot Adc", .level = config.log_level}) {
     init(config);
   }
 

components/ads1x15/include/ads1x15.hpp

@@ -34,8 +34,7 @@ class Ads1x15 {
    * @param data_len Number of data bytes to read.
    * @return True if the read was successful.
    */
-  typedef std::function<bool(uint8_t dev_addr, uint8_t *data, size_t data_len)>
-      read_fn;
+  typedef std::function<bool(uint8_t dev_addr, uint8_t *data, size_t data_len)> read_fn;
 
   /**
    *  @brief Gain values for the ADC conversion.
@@ -104,7 +103,7 @@ class Ads1x15 {
    * @brief Construct Ads1x15 specficially for ADS1015.
    * @param config Configuration structure.
    */
-  Ads1x15(const Ads1015Config &config)
+  explicit Ads1x15(const Ads1015Config &config)
       : gain_(config.gain), ads1015rate_(config.sample_rate), bit_shift_(4),
         address_(config.device_address), write_(config.write), read_(config.read),
         logger_({.tag = "Ads1015", .level = config.log_level}) {}
@@ -113,7 +112,7 @@ class Ads1x15 {
    * @brief Construct Ads1x15 specficially for ADS1115.
    * @param config Configuration structure.
    */
-  Ads1x15(const Ads1115Config &config)
+  explicit Ads1x15(const Ads1115Config &config)
       : gain_(config.gain), ads1115rate_(config.sample_rate), bit_shift_(0),
         address_(config.device_address), write_(config.write), read_(config.read),
         logger_({.tag = "Ads1115", .level = config.log_level}) {}
@@ -138,7 +137,7 @@ class Ads1x15 {
 
   bool conversion_complete(std::error_code &ec);
 
-  float raw_to_mv(int16_t raw) {
+  float raw_to_mv(int16_t raw) const {
     // see data sheet Table 3
     float fsRange;
     switch (gain_) {

components/ads1x15/src/ads1x15.cpp

@@ -56,13 +56,14 @@ int16_t Ads1x15::sample_raw(int channel, std::error_code &ec) {
     return 0;
   }
   // Start with default values
-  uint16_t config = REG_CONFIG_CQUE_1CONV |   // Comparator enabled and asserts on 1
-                                              // match
-                    REG_CONFIG_CLAT_NONLAT |  // non-latching (default val)
-                    REG_CONFIG_CPOL_ACTVLOW | // Alert/Rdy active low   (default val)
-                    REG_CONFIG_CMODE_TRAD |   // Traditional comparator (default val)
-                    REG_CONFIG_MODE_SINGLE;   // Single conversion mode
-  // Set PGA/voltage range
+  uint16_t config = REG_CONFIG_MODE_SINGLE;
+  // This is equivalent to the below (since the rest are 0x0000):
+  //
+  // REG_CONFIG_CQUE_1CONV |   // Comparator enabled and asserts on 1 match
+  // REG_CONFIG_CLAT_NONLAT |  // non-latching (default val)
+  // REG_CONFIG_CPOL_ACTVLOW | // Alert/Rdy active low (default val)
+  // REG_CONFIG_CMODE_TRAD |   // Traditional comparator (default val)
+  // REG_CONFIG_MODE_SINGLE;   // Single conversion mode Set PGA/voltage range
   config |= (uint16_t)gain_;
   // Set data rate
   config |= rate_;

components/ads7138/include/ads7138.hpp

@@ -4,6 +4,7 @@
 #include <chrono>
 #include <functional>
 #include <mutex>
+#include <numeric>
 #include <thread>
 #include <unordered_map>
 
@@ -177,7 +178,7 @@ class Ads7138 {
    * @brief Construct Ads7138
    * @param config Configuration structure.
    */
-  Ads7138(const Config &config)
+  explicit Ads7138(const Config &config)
       : config_(config), mode_(config.mode), avdd_mv_(config.avdd_volts * 1000.0f) // Convert to mV
         ,
         data_format_(config.oversampling_ratio == OversamplingRatio::NONE ? DataFormat::RAW
@@ -920,16 +921,16 @@ class Ads7138 {
     logger_.info("Statistics enabled set to {}", statistics_enabled_);
   }
 
+  static uint8_t bit_pred(uint8_t value, Channel channel) {
+    return value | (1 << static_cast<uint8_t>(channel));
+  };
+
   void set_pin_configuration(std::error_code &ec) {
     logger_.info("Setting digital mode for outputs {} and inputs {}", digital_outputs_,
                  digital_inputs_);
     uint8_t data = 0;
-    for (auto channel : digital_inputs_) {
-      data |= 1 << static_cast<uint8_t>(channel);
-    }
-    for (auto channel : digital_outputs_) {
-      data |= 1 << static_cast<uint8_t>(channel);
-    }
+    std::accumulate(digital_inputs_.begin(), digital_inputs_.end(), data, bit_pred);
+    std::accumulate(digital_outputs_.begin(), digital_outputs_.end(), data, bit_pred);
     // don't have to do anything for analog inputs since they are the default
     // state (0)
     write_one_(Register::PIN_CFG, data, ec);
@@ -939,9 +940,7 @@ class Ads7138 {
     logger_.info("Setting digital output for pins {}", digital_outputs_);
     // default direction is input (0)
     uint8_t data = 0;
-    for (auto channel : digital_outputs_) {
-      data |= 1 << static_cast<uint8_t>(channel);
-    }
+    std::accumulate(digital_outputs_.begin(), digital_outputs_.end(), data, bit_pred);
     write_one_(Register::GPIO_CFG, data, ec);
   }
 
@@ -951,9 +950,7 @@ class Ads7138 {
       logger_.info("Setting analog inputs for autonomous mode");
       // configure the analog inputs for autonomous conversion sequence
       uint8_t data = 0;
-      for (auto channel : analog_inputs_) {
-        data |= 1 << static_cast<uint8_t>(channel);
-      }
+      std::accumulate(analog_inputs_.begin(), analog_inputs_.end(), data, bit_pred);
       write_one_(Register::AUTO_SEQ_CH_SEL, data, ec);
     }
   }
@@ -1204,7 +1201,7 @@ class Ads7138 {
    * @param raw Raw ADC value.
    * @return Voltage in mV.
    */
-  float raw_to_mv(uint16_t raw) {
+  float raw_to_mv(uint16_t raw) const {
     // we have a 16-bit ADC, so we can represent 2^16 = 65536 values
     // we were configured with avdd_mv_ as the reference voltage, so we
     // can represent avdd_mv_ volts with 65536 values
@@ -1218,7 +1215,7 @@ class Ads7138 {
    * @param mv Voltage in mV.
    * @return Raw ADC value.
    */
-  uint16_t mv_to_raw(float mv) {
+  uint16_t mv_to_raw(float mv) const {
     // we have a 16-bit ADC, so we can represent 2^16 = 65536 values
     // we were configured with avdd_mv_ as the reference voltage, so we
     // can represent avdd_mv_ volts with 65536 values
@@ -1512,7 +1509,7 @@ class Ads7138 {
     write_many_(reg, (uint8_t *)&value, 2, ec);
   }
 
-  void write_many_(Register reg, uint8_t *data, uint8_t len, std::error_code &ec) {
+  void write_many_(Register reg, const uint8_t *data, uint8_t len, std::error_code &ec) {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
     uint8_t total_len = len + 2;
     uint8_t data_with_header[total_len];

components/as5600/include/as5600.hpp

@@ -93,7 +93,7 @@ class As5600 {
   /**
    * @brief Construct the As5600 and start the update task.
    */
-  As5600(const Config &config)
+  explicit As5600(const Config &config)
       : address_(config.device_address), write_(config.write), read_(config.read),
         velocity_filter_(config.velocity_filter), update_period_(config.update_period),
         logger_({.tag = "As5600", .level = config.log_level}) {

components/aw9523/include/aw9523.hpp

@@ -86,7 +86,7 @@ class Aw9523 {
    * @brief Construct the Aw9523. Will call initialize() if auto_init is true.
    * @param config Config structure for configuring the AW9523
    */
-  Aw9523(const Config &config)
+  explicit Aw9523(const Config &config)
       : config_(config), address_(config.device_address), write_(config.write), read_(config.read),
         logger_({.tag = "Aw9523", .level = config.log_level}) {
     if (config.auto_init) {
@@ -305,7 +305,7 @@ class Aw9523 {
   void set_interrupt(uint8_t p0, uint8_t p1, std::error_code &ec) {
     logger_.debug("Setting interrupt on change p0:{}, p1:{}", p0, p1);
     auto addr = Registers::INTPORT0;
-    uint8_t data[] = {p0, p1};
+    const uint8_t data[] = {p0, p1};
     write_many_((uint8_t)addr, data, 2, ec);
   }
 
@@ -330,7 +330,7 @@ class Aw9523 {
   void set_direction(uint8_t p0, uint8_t p1, std::error_code &ec) {
     logger_.debug("Setting direction  p0:{}, p1:{}", p0, p1);
     auto addr = Registers::DIRPORT0;
-    uint8_t data[] = {p0, p1};
+    const uint8_t data[] = {p0, p1};
     write_many_((uint8_t)addr, data, 2, ec);
   }
 
@@ -355,7 +355,7 @@ class Aw9523 {
   void configure_led(uint8_t p0, uint8_t p1, std::error_code &ec) {
     logger_.debug("Configuring LED function p0:{}, p1:{}", p0, p1);
     auto addr = Registers::LEDMODE0;
-    uint8_t data[] = {p0, p1};
+    const uint8_t data[] = {p0, p1};
     write_many_((uint8_t)addr, data, 2, ec);
   }
 
@@ -370,7 +370,7 @@ class Aw9523 {
     uint8_t p1 = (mask >> 8) & 0xFF;
     logger_.debug("Configuring LED function p0:{}, p1:{}", p0, p1);
     auto addr = Registers::LEDMODE0;
-    uint8_t data[] = {p0, p1};
+    const uint8_t data[] = {p0, p1};
     write_many_((uint8_t)addr, data, 2, ec);
   }
 
@@ -540,7 +540,7 @@ class Aw9523 {
     write_many_(reg_addr, &data, 1, ec);
   }
 
-  void write_many_(uint8_t reg_addr, uint8_t *write_data, size_t write_data_len,
+  void write_many_(uint8_t reg_addr, const uint8_t *write_data, size_t write_data_len,
                    std::error_code &ec) {
     uint8_t total_len = 1 + write_data_len;
     uint8_t data[total_len];

components/bldc_driver/include/bldc_driver.hpp

@@ -47,7 +47,7 @@ class BldcDriver {
    * @note Enables the driver.
    * @param config Config used to initialize the driver.
    */
-  BldcDriver(const Config &config)
+  explicit BldcDriver(const Config &config)
       : gpio_ah_((gpio_num_t)config.gpio_a_h), gpio_al_((gpio_num_t)config.gpio_a_l),
         gpio_bh_((gpio_num_t)config.gpio_b_h), gpio_bl_((gpio_num_t)config.gpio_b_l),
         gpio_ch_((gpio_num_t)config.gpio_c_h), gpio_cl_((gpio_num_t)config.gpio_c_l),