Update stepper_motor module to work on esp32s3 and migrate to newer pcnt driver (#191)

* add changes for s3

* fixed stepper motor on esp32s3 & migrate to newer pcnt driver

* refactor

* fix auto-formatting for reference table

* add missing error checks

* add missing include

* wrap lines for readability and smaller diffs

* use designated initializers for PCNT config structs

* add defaults to prevent warnings

---------

Co-authored-by: Falko Schindler <[email protected]>

Author: JensOgorek

docs/module_reference.md

@@ -604,12 +604,13 @@ When the wheels are disabled, they will stop and ignore movement commands.
 The stepper motor module controls a stepper motor via "step" and "direction" pins.
 It uses the ESP LED Control API to generate pulses with sufficiently high frequencies and the Pulse Counter API to count steps.
 
-| Constructor                                               | Description             | Arguments |
-| --------------------------------------------------------- | ----------------------- | --------- |
-| `motor = StepperMotor(step, dir[, pu[, cp[, lt[, lc]]]])` | Step and direction pins | 6x `int`  |
+| Constructor                                   | Description             | Arguments  |
+| --------------------------------------------- | ----------------------- | ---------- |
+| `motor = StepperMotor(step, dir[, lt[, lc]])` | Step and direction pins | 2–4x `int` |
 
-The constructor arguments `pu` (pulse counter unit), `pc` (pulse counter channel), `lt` (LED timer) and `lc` (LED channel) are optional and default to 0.
-When using multiple stepper motors, they can be set to different values to avoid conflicts.
+The constructor arguments `lt` (LEDC timer) and `lc` (LEDC channel) are optional and default to 0.
+When using multiple stepper motors, set different timer/channel pairs to avoid conflicts.
+The pulse counter used for position feedback is created internally.
 
 | Properties       | Description                    | Data type |
 | ---------------- | ------------------------------ | --------- |

main/CMakeLists.txt

@@ -9,7 +9,24 @@ file(GLOB SRC_FILES
 idf_component_register(
     SRCS ${SRC_FILES}
     INCLUDE_DIRS "." "compilation" "modules" "utils" "${CMAKE_BINARY_DIR}/generated"
-    REQUIRES driver esp_wifi esp_timer efuse lwip esp_event esp_netif esp_adc esp32-zeug esp32-serial-flasher app_update esp_driver_uart esp_https_ota spi_flash bt nvs_flash
+    REQUIRES
+        app_update
+        bt
+        driver
+        efuse
+        esp32-serial-flasher
+        esp32-zeug
+        esp_adc
+        esp_driver_pcnt
+        esp_driver_uart
+        esp_event
+        esp_https_ota
+        esp_netif
+        esp_timer
+        esp_wifi
+        lwip
+        nvs_flash
+        spi_flash
 )
 add_compile_definitions(OWL_TOKEN_RUN_LENGTH=256)
 component_compile_options(-std=gnu++17 -Wsuggest-override)

main/modules/module.cpp

@@ -11,7 +11,6 @@
 #include "d1_motor.h"
 #include "driver/gpio.h"
 #include "driver/ledc.h"
-#include "driver/pcnt.h"
 #include "dunker_motor.h"
 #include "dunker_wheels.h"
 #include "expander.h"
@@ -288,17 +287,15 @@ Module_ptr Module::create(const std::string type,
         const RoboClawMotor_ptr right_motor = get_module_paramter<RoboClawMotor>(arguments[1], roboclaw_motor, "roboclaw motor");
         return std::make_shared<RoboClawWheels>(name, left_motor, right_motor);
     } else if (type == "StepperMotor") {
-        if (arguments.size() < 2 || arguments.size() > 6) {
+        if (arguments.size() < 2 || arguments.size() > 4) {
             throw std::runtime_error("unexpected number of arguments");
         }
-        Module::expect(arguments, -1, integer, integer, integer, integer, integer, integer);
+        Module::expect(arguments, -1, integer, integer, integer, integer);
         gpio_num_t step_pin = (gpio_num_t)arguments[0]->evaluate_integer();
         gpio_num_t dir_pin = (gpio_num_t)arguments[1]->evaluate_integer();
-        pcnt_unit_t pcnt_unit = arguments.size() > 2 ? (pcnt_unit_t)arguments[2]->evaluate_integer() : PCNT_UNIT_0;
-        pcnt_channel_t pcnt_channel = arguments.size() > 3 ? (pcnt_channel_t)arguments[3]->evaluate_integer() : PCNT_CHANNEL_0;
-        ledc_timer_t ledc_timer = arguments.size() > 4 ? (ledc_timer_t)arguments[4]->evaluate_integer() : LEDC_TIMER_0;
-        ledc_channel_t ledc_channel = arguments.size() > 5 ? (ledc_channel_t)arguments[5]->evaluate_integer() : LEDC_CHANNEL_0;
-        return std::make_shared<StepperMotor>(name, step_pin, dir_pin, pcnt_unit, pcnt_channel, ledc_timer, ledc_channel);
+        ledc_timer_t ledc_timer = arguments.size() > 2 ? (ledc_timer_t)arguments[2]->evaluate_integer() : LEDC_TIMER_0;
+        ledc_channel_t ledc_channel = arguments.size() > 3 ? (ledc_channel_t)arguments[3]->evaluate_integer() : LEDC_CHANNEL_0;
+        return std::make_shared<StepperMotor>(name, step_pin, dir_pin, ledc_timer, ledc_channel);
     } else if (type == "MotorAxis") {
         Module::expect(arguments, 3, identifier, identifier, identifier);
         const std::string name = arguments[0]->evaluate_identifier();

main/modules/stepper_motor.cpp

@@ -1,24 +1,31 @@
 #include "stepper_motor.h"
 #include "../utils/timing.h"
 #include "../utils/uart.h"
-#include "esp32/rom/gpio.h"
+#include "rom/gpio.h"
 #include "soc/gpio_sig_map.h"
 #include <algorithm>
 #include <driver/ledc.h>
-#include <driver/pcnt.h>
-#include <esp_rom_gpio.h>
-#include <math.h>
-#include <memory>
+#include <driver/pulse_cnt.h>
 #include <stdexcept>
 
 #define MIN_SPEED 490
 
+static void check_error(esp_err_t err, const char *msg) {
+    if (err != ESP_OK) {
+        throw std::runtime_error(std::string(msg) + ": " + esp_err_to_name(err));
+    }
+}
+
 #ifdef CONFIG_IDF_TARGET_ESP32S3
 #define SPEED_MODE LEDC_LOW_SPEED_MODE
 #define SPEED_OUT_IDX LEDC_LS_SIG_OUT0_IDX
+#define DUTY_RESOLUTION LEDC_TIMER_8_BIT
+#define DUTY_VALUE 128 // 50% of 256 with 8-bit resolution
 #else
 #define SPEED_MODE LEDC_HIGH_SPEED_MODE
 #define SPEED_OUT_IDX LEDC_HS_SIG_OUT0_IDX
+#define DUTY_RESOLUTION LEDC_TIMER_1_BIT
+#define DUTY_VALUE 1 // 50% of max 1 with 1-bit resolution
 #endif
 
 REGISTER_MODULE_DEFAULTS(StepperMotor)
@@ -35,48 +42,56 @@ const std::map<std::string, Variable_ptr> StepperMotor::get_defaults() {
 StepperMotor::StepperMotor(const std::string name,
                            const gpio_num_t step_pin,
                            const gpio_num_t dir_pin,
-                           const pcnt_unit_t pcnt_unit,
-                           const pcnt_channel_t pcnt_channel,
                            const ledc_timer_t ledc_timer,
                            const ledc_channel_t ledc_channel)
     : Module(stepper_motor, name),
       step_pin(step_pin),
       dir_pin(dir_pin),
-      pcnt_unit(pcnt_unit),
-      pcnt_channel(pcnt_channel),
       ledc_timer(ledc_timer),
       ledc_channel(ledc_channel) {
     gpio_reset_pin(step_pin);
     gpio_reset_pin(dir_pin);
 
     this->properties = StepperMotor::get_defaults();
 
-    pcnt_config_t pcnt_config = {
-        .pulse_gpio_num = step_pin,
-        .ctrl_gpio_num = dir_pin,
-        .lctrl_mode = PCNT_MODE_REVERSE,
-        .hctrl_mode = PCNT_MODE_KEEP,
-        .pos_mode = PCNT_COUNT_INC,
-        .neg_mode = PCNT_COUNT_DIS,
-        .counter_h_lim = 30000,
-        .counter_l_lim = -30000,
-        .unit = this->pcnt_unit,
-        .channel = this->pcnt_channel,
+    pcnt_unit_config_t unit_config = {
+        .low_limit = -30000,
+        .high_limit = 30000,
+        .intr_priority = 0,
+        .flags = {},
+    };
+    check_error(pcnt_new_unit(&unit_config, &this->pcnt_unit), "failed to create PCNT unit");
+
+    pcnt_chan_config_t chan_config = {
+        .edge_gpio_num = step_pin,
+        .level_gpio_num = dir_pin,
+        .flags = {},
     };
-    pcnt_unit_config(&pcnt_config);
-    pcnt_counter_pause(this->pcnt_unit);
-    pcnt_counter_clear(this->pcnt_unit);
-    pcnt_counter_resume(this->pcnt_unit);
+    check_error(pcnt_new_channel(this->pcnt_unit, &chan_config, &this->pcnt_channel), "failed to create PCNT channel");
+
+    check_error(pcnt_channel_set_edge_action(this->pcnt_channel,
+                                             PCNT_CHANNEL_EDGE_ACTION_INCREASE,
+                                             PCNT_CHANNEL_EDGE_ACTION_HOLD),
+                "failed to set PCNT edge action");
+
+    check_error(pcnt_channel_set_level_action(this->pcnt_channel,
+                                              PCNT_CHANNEL_LEVEL_ACTION_KEEP,
+                                              PCNT_CHANNEL_LEVEL_ACTION_INVERSE),
+                "failed to set PCNT level action");
+
+    check_error(pcnt_unit_enable(this->pcnt_unit), "failed to enable PCNT unit");
+    check_error(pcnt_unit_clear_count(this->pcnt_unit), "failed to clear PCNT count");
+    check_error(pcnt_unit_start(this->pcnt_unit), "failed to start PCNT unit");
 
     ledc_timer_config_t timer_config = {
         .speed_mode = SPEED_MODE,
-        .duty_resolution = LEDC_TIMER_1_BIT,
+        .duty_resolution = DUTY_RESOLUTION,
         .timer_num = this->ledc_timer,
         .freq_hz = 1000,
         .clk_cfg = LEDC_AUTO_CLK,
         .deconfigure = false,
     };
-    ledc_timer_config(&timer_config);
+    check_error(ledc_timer_config(&timer_config), "failed to configure LEDC timer");
 
     ledc_channel_config_t channel_config = {
         .gpio_num = step_pin,
@@ -88,16 +103,16 @@ StepperMotor::StepperMotor(const std::string name,
         .hpoint = 0,
         .flags = {},
     };
-    ledc_channel_config(&channel_config);
+    check_error(ledc_channel_config(&channel_config), "failed to configure LEDC channel");
 
     gpio_set_direction(step_pin, GPIO_MODE_INPUT_OUTPUT);
     gpio_set_direction(dir_pin, GPIO_MODE_INPUT_OUTPUT);
 }
 
 void StepperMotor::read_position() {
-    int16_t count;
-    pcnt_get_counter_value(this->pcnt_unit, &count);
-    int16_t d_count = count - this->last_count;
+    int count;
+    pcnt_unit_get_count(this->pcnt_unit, &count);
+    int d_count = count - this->last_count;
     if (d_count > 15000) {
         d_count -= 30000;
     }
@@ -113,7 +128,7 @@ void StepperMotor::set_state(StepperState new_state) {
     this->properties.at("idle")->boolean_value = (new_state == Idle);
 
     gpio_matrix_out(this->step_pin, new_state == Idle ? SIG_GPIO_OUT_IDX : SPEED_OUT_IDX + this->ledc_channel, 0, 0);
-    ledc_set_duty(SPEED_MODE, this->ledc_channel, new_state == Idle ? 0 : 1);
+    ledc_set_duty(SPEED_MODE, this->ledc_channel, new_state == Idle ? 0 : DUTY_VALUE);
     ledc_update_duty(SPEED_MODE, this->ledc_channel);
 }
 

main/modules/stepper_motor.h

@@ -2,7 +2,7 @@
 
 #include "driver/gpio.h"
 #include "driver/ledc.h"
-#include "driver/pcnt.h"
+#include "driver/pulse_cnt.h"
 #include "module.h"
 #include "motor.h"
 
@@ -19,13 +19,14 @@ class StepperMotor : public Module, virtual public Motor {
 private:
     const gpio_num_t step_pin;
     const gpio_num_t dir_pin;
-    const pcnt_unit_t pcnt_unit;
-    const pcnt_channel_t pcnt_channel;
     const ledc_timer_t ledc_timer;
     const ledc_channel_t ledc_channel;
 
+    pcnt_unit_handle_t pcnt_unit = nullptr;
+    pcnt_channel_handle_t pcnt_channel = nullptr;
+
     uint32_t last_micros = 0;
-    int16_t last_count = 0;
+    int last_count = 0;
 
     StepperState state = Idle;
     int32_t target_position = 0;
@@ -40,8 +41,6 @@ class StepperMotor : public Module, virtual public Motor {
     StepperMotor(const std::string name,
                  const gpio_num_t step_pin,
                  const gpio_num_t dir_pin,
-                 const pcnt_unit_t pcnt_unit,
-                 const pcnt_channel_t pcnt_channel,
                  const ledc_timer_t ledc_timer,
                  const ledc_channel_t ledc_channel);
     void step() override;