RFC: Change Stepper API to use step interval

doc/hardware/peripherals/stepper.rst

@@ -12,15 +12,15 @@ Configure Stepper Driver
   and :c:func:`stepper_get_micro_step_res`.
 - Configure **reference position** in microsteps using :c:func:`stepper_set_reference_position`
   and :c:func:`stepper_get_actual_position`.
-- Set **max velocity** in micro-steps per second using :c:func:`stepper_set_max_velocity`
+- Set **step interval** in microseconds between steps using :c:func:`stepper_set_step_interval`
 - **Enable** the stepper driver using :c:func:`stepper_enable`.
 
 Control Stepper
 ===============
 
 - **Move by** +/- micro-steps also known as **relative movement** using :c:func:`stepper_move_by`.
 - **Move to** a specific position also known as **absolute movement** using :c:func:`stepper_move_to`.
-- Run continuously with a **constant velocity** in a specific direction until
+- Run continuously with a **constant step interval** in a specific direction until
   a stop is detected using :c:func:`stepper_run`.
 - Check if the stepper is **moving** using :c:func:`stepper_is_moving`.
 - Register an **event callback** using :c:func:`stepper_set_event_callback`.

doc/releases/migration-guide-4.1.rst

@@ -237,8 +237,13 @@ Stepper
   * Renamed the ``compatible`` from ``zephyr,gpio-steppers`` to :dtcompatible:`zephyr,gpio-stepper`.
   * Renamed the ``stepper_set_actual_position`` function to :c:func:`stepper_set_reference_position`.
   * Renamed the ``stepper_enable_constant_velocity_mode`` function to :c:func:`stepper_run`.
+    The function does not take a velocity parameter anymore. Set the desired speed using the
+    :c:func:`stepper_set_step_interval` function beforehand.
   * Renamed the ``stepper_move`` function to :c:func:`stepper_move_by`.
   * Renamed the ``stepper_set_target_position`` function to :c:func:`stepper_move_to`.
+  * Renamed the ``stepper_set_max_velocity`` function to :c:func:`stepper_set_step_interval`.
+    The function now takes the step interval in microseconds. This allows for a more precise control.
+  * Changed the :c:func:`stepper_run` to take the step interval in microseconds instead of velocity.
   * The :kconfig:option:`STEPPER_ADI_TMC_RAMP_GEN` is now deprecated and is replaced with the new
     :kconfig:option:`STEPPER_ADI_TMC5041_RAMP_GEN` option.
 

drivers/stepper/adi_tmc/adi_tmc22xx_stepper_controller.c

@@ -150,7 +150,7 @@ static DEVICE_API(stepper, tmc22xx_stepper_api) = {
 	.set_reference_position = step_dir_stepper_common_set_reference_position,
 	.get_actual_position = step_dir_stepper_common_get_actual_position,
 	.move_to = step_dir_stepper_common_move_to,
-	.set_max_velocity = step_dir_stepper_common_set_max_velocity,
+	.set_step_interval = step_dir_stepper_common_set_step_interval,
 	.run = step_dir_stepper_common_run,
 	.set_event_callback = step_dir_stepper_common_set_event_callback,
 	.set_micro_step_res = tmc22xx_stepper_set_micro_step_res,

drivers/stepper/adi_tmc/adi_tmc5041_stepper_controller.c

@@ -348,15 +348,26 @@ static int tmc5041_stepper_move_by(const struct device *dev, const int32_t micro
 	return 0;
 }
 
-static int tmc5041_stepper_set_max_velocity(const struct device *dev, uint32_t velocity)
+static int tmc5041_stepper_set_step_interval(const struct device *dev, uint64_t step_interval_us)
 {
 	const struct tmc5041_stepper_config *config = dev->config;
 	const struct tmc5041_config *tmc5041_config = config->controller->config;
 	const uint32_t clock_frequency = tmc5041_config->clock_frequency;
 	uint32_t velocity_fclk;
 	int err;
 
-	velocity_fclk = tmc5xxx_calculate_velocity_from_hz_to_fclk(velocity, clock_frequency);
+	if (step_interval_us == 0) {
+		err = tmc5041_write(config->controller, TMC5041_RAMPMODE(config->index),
+				    TMC5XXX_RAMPMODE_HOLD_MODE);
+		if (err != 0) {
+			LOG_ERR("%s: Failed to stop motor", dev->name);
+			return -EIO;
+		}
+		return 0;
+	}
+
+	velocity_fclk = tmc5xxx_calculate_velocity_fclk_from_step_interval(step_interval_us,
+									   clock_frequency);
 
 	err = tmc5041_write(config->controller, TMC5041_VMAX(config->index), velocity_fclk);
 	if (err != 0) {
@@ -477,19 +488,13 @@ static int tmc5041_stepper_move_to(const struct device *dev, const int32_t micro
 	return 0;
 }
 
-static int tmc5041_stepper_run(const struct device *dev, const enum stepper_direction direction,
-			       const uint32_t velocity)
+static int tmc5041_stepper_run(const struct device *dev, const enum stepper_direction direction)
 {
-	LOG_DBG("Stepper motor controller %s run with velocity %d", dev->name, velocity);
+	LOG_DBG("Stepper motor controller %s run", dev->name);
 	const struct tmc5041_stepper_config *config = dev->config;
-	const struct tmc5041_config *tmc5041_config = config->controller->config;
 	struct tmc5041_stepper_data *data = dev->data;
-	const uint32_t clock_frequency = tmc5041_config->clock_frequency;
-	uint32_t velocity_fclk;
 	int err;
 
-	velocity_fclk = tmc5xxx_calculate_velocity_from_hz_to_fclk(velocity, clock_frequency);
-
 	if (config->is_sg_enabled) {
 		err = stallguard_enable(dev, false);
 		if (err != 0) {
@@ -504,10 +509,6 @@ static int tmc5041_stepper_run(const struct device *dev, const enum stepper_dire
 		if (err != 0) {
 			return -EIO;
 		}
-		err = tmc5041_write(config->controller, TMC5041_VMAX(config->index), velocity_fclk);
-		if (err != 0) {
-			return -EIO;
-		}
 		break;
 
 	case STEPPER_DIRECTION_NEGATIVE:
@@ -516,10 +517,6 @@ static int tmc5041_stepper_run(const struct device *dev, const enum stepper_dire
 		if (err != 0) {
 			return -EIO;
 		}
-		err = tmc5041_write(config->controller, TMC5041_VMAX(config->index), velocity_fclk);
-		if (err != 0) {
-			return -EIO;
-		}
 		break;
 	}
 
@@ -724,7 +721,7 @@ static int tmc5041_stepper_init(const struct device *dev)
 		.enable = tmc5041_stepper_enable,						\
 		.is_moving = tmc5041_stepper_is_moving,						\
 		.move_by = tmc5041_stepper_move_by,						\
-		.set_max_velocity = tmc5041_stepper_set_max_velocity,				\
+		.set_step_interval = tmc5041_stepper_set_step_interval,				\
 		.set_micro_step_res = tmc5041_stepper_set_micro_step_res,			\
 		.get_micro_step_res = tmc5041_stepper_get_micro_step_res,			\
 		.set_reference_position = tmc5041_stepper_set_reference_position,		\

drivers/stepper/adi_tmc/adi_tmc5xxx_common.h

@@ -26,17 +26,21 @@ extern "C" {
  */
 
 /**
- * @brief Calculate the velocity in full clock cycles from the velocity in Hz
+ * @brief Calculate the velocity in full clock cycles from the given step interval
  *
- * @param velocity_hz Velocity in Hz
+ * @param step_interval_us Step interval in microseconds
  * @param clock_frequency Clock frequency in Hz
  *
  * @return Calculated velocity in full clock cycles
  */
-static inline uint32_t tmc5xxx_calculate_velocity_from_hz_to_fclk(uint64_t velocity_hz,
-								  uint32_t clock_frequency)
+static inline uint32_t tmc5xxx_calculate_velocity_fclk_from_step_interval(uint64_t step_interval_us,
+									  uint32_t clock_frequency)
 {
 	__ASSERT_NO_MSG(clock_frequency);
+	__ASSERT_NO_MSG(step_interval_us);
+
+	uint32_t velocity_hz = 1e6 / step_interval_us;
+
 	return (velocity_hz << TMC5XXX_CLOCK_FREQ_SHIFT) / clock_frequency;
 }
 

drivers/stepper/fake_stepper_controller.c

@@ -25,7 +25,7 @@ DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_is_moving, const struct device *, bool
 
 DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_move_by, const struct device *, int32_t);
 
-DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_set_max_velocity, const struct device *, uint32_t);
+DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_set_step_interval, const struct device *, uint64_t);
 
 DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_set_micro_step_res, const struct device *,
 		       enum stepper_micro_step_resolution);
@@ -39,8 +39,7 @@ DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_get_actual_position, const struct devic
 
 DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_move_to, const struct device *, int32_t);
 
-DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_run, const struct device *, enum stepper_direction,
-		       uint32_t);
+DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_run, const struct device *, enum stepper_direction);
 
 DEFINE_FAKE_VALUE_FUNC(int, fake_stepper_set_event_callback, const struct device *,
 		       stepper_event_callback_t, void *);
@@ -92,7 +91,7 @@ static void fake_stepper_reset_rule_before(const struct ztest_unit_test *test, v
 	RESET_FAKE(fake_stepper_enable);
 	RESET_FAKE(fake_stepper_move_by);
 	RESET_FAKE(fake_stepper_is_moving);
-	RESET_FAKE(fake_stepper_set_max_velocity);
+	RESET_FAKE(fake_stepper_set_step_interval);
 	RESET_FAKE(fake_stepper_set_micro_step_res);
 	RESET_FAKE(fake_stepper_get_micro_step_res);
 	RESET_FAKE(fake_stepper_set_reference_position);
@@ -128,7 +127,7 @@ static DEVICE_API(stepper, fake_stepper_driver_api) = {
 	.enable = fake_stepper_enable,
 	.move_by = fake_stepper_move_by,
 	.is_moving = fake_stepper_is_moving,
-	.set_max_velocity = fake_stepper_set_max_velocity,
+	.set_step_interval = fake_stepper_set_step_interval,
 	.set_micro_step_res = fake_stepper_set_micro_step_res,
 	.get_micro_step_res = fake_stepper_get_micro_step_res,
 	.set_reference_position = fake_stepper_set_reference_position,

drivers/stepper/gpio_stepper_controller.c

@@ -37,7 +37,7 @@ struct gpio_stepper_data {
 	uint8_t coil_charge;
 	struct k_work_delayable stepper_dwork;
 	int32_t actual_position;
-	uint32_t delay_in_us;
+	uint64_t delay_in_us;
 	int32_t step_count;
 	bool is_enabled;
 	stepper_event_callback_t callback;
@@ -188,7 +188,7 @@ static int gpio_stepper_move_by(const struct device *dev, int32_t micro_steps)
 	}
 
 	if (data->delay_in_us == 0) {
-		LOG_ERR("Velocity not set or invalid velocity set");
+		LOG_ERR("Step interval not set or invalid step interval set");
 		return -EINVAL;
 	}
 	K_SPINLOCK(&data->lock) {
@@ -230,7 +230,7 @@ static int gpio_stepper_move_to(const struct device *dev, int32_t micro_steps)
 	}
 
 	if (data->delay_in_us == 0) {
-		LOG_ERR("Velocity not set or invalid velocity set");
+		LOG_ERR("Step interval not set or invalid step interval set");
 		return -EINVAL;
 	}
 	K_SPINLOCK(&data->lock) {
@@ -251,29 +251,22 @@ static int gpio_stepper_is_moving(const struct device *dev, bool *is_moving)
 	return 0;
 }
 
-static int gpio_stepper_set_max_velocity(const struct device *dev, uint32_t velocity)
+static int gpio_stepper_set_step_interval(const struct device *dev, uint64_t step_interval_us)
 {
 	struct gpio_stepper_data *data = dev->data;
 
-	if (velocity == 0) {
-		LOG_ERR("Velocity cannot be zero");
-		return -EINVAL;
-	}
-
-	if (velocity > USEC_PER_SEC) {
-		LOG_ERR("Velocity cannot be greater than %d micro_steps_per_second", USEC_PER_SEC);
-		return -EINVAL;
-	}
-
 	K_SPINLOCK(&data->lock) {
-		data->delay_in_us = USEC_PER_SEC / velocity;
+		data->delay_in_us = step_interval_us;
+
+		if (step_interval_us == 0) {
+			(void)k_work_cancel_delayable(&data->stepper_dwork);
+		}
 	}
-	LOG_DBG("Setting Motor Speed to %d", velocity);
+	LOG_DBG("Setting Motor step interval to %llu", step_interval_us);
 	return 0;
 }
 
-static int gpio_stepper_run(const struct device *dev, const enum stepper_direction direction,
-			    const uint32_t velocity)
+static int gpio_stepper_run(const struct device *dev, const enum stepper_direction direction)
 {
 	struct gpio_stepper_data *data = dev->data;
 
@@ -285,12 +278,7 @@ static int gpio_stepper_run(const struct device *dev, const enum stepper_directi
 	K_SPINLOCK(&data->lock) {
 		data->run_mode = STEPPER_RUN_MODE_VELOCITY;
 		data->direction = direction;
-		if (velocity != 0) {
-			data->delay_in_us = USEC_PER_SEC / velocity;
-			(void)k_work_reschedule(&data->stepper_dwork, K_NO_WAIT);
-		} else {
-			(void)k_work_cancel_delayable(&data->stepper_dwork);
-		}
+		(void)k_work_reschedule(&data->stepper_dwork, K_NO_WAIT);
 	}
 	return 0;
 }
@@ -377,7 +365,7 @@ static DEVICE_API(stepper, gpio_stepper_api) = {
 	.set_reference_position = gpio_stepper_set_reference_position,
 	.get_actual_position = gpio_stepper_get_actual_position,
 	.move_to = gpio_stepper_move_to,
-	.set_max_velocity = gpio_stepper_set_max_velocity,
+	.set_step_interval = gpio_stepper_set_step_interval,
 	.run = gpio_stepper_run,
 	.set_micro_step_res = gpio_stepper_set_micro_step_res,
 	.get_micro_step_res = gpio_stepper_get_micro_step_res,

drivers/stepper/step_dir/step_dir_stepper_common.c

@@ -228,40 +228,35 @@ int step_dir_stepper_common_move_by(const struct device *dev, const int32_t micr
 	struct step_dir_stepper_common_data *data = dev->data;
 	const struct step_dir_stepper_common_config *config = dev->config;
 
-	if (data->max_velocity == 0) {
-		LOG_ERR("Velocity not set or invalid velocity set");
+	if (data->step_interval_us == 0) {
+		LOG_ERR("Step interval not set or invalid step interval set");
 		return -EINVAL;
 	}
 
 	K_SPINLOCK(&data->lock) {
 		data->run_mode = STEPPER_RUN_MODE_POSITION;
 		data->step_count = micro_steps;
-		config->timing_source->update(dev, data->max_velocity);
+		config->timing_source->update(dev, data->step_interval_us);
 		update_direction_from_step_count(dev);
 		config->timing_source->start(dev);
 	}
 
 	return 0;
 }
 
-int step_dir_stepper_common_set_max_velocity(const struct device *dev, const uint32_t velocity)
+int step_dir_stepper_common_set_step_interval(const struct device *dev,
+					      const uint64_t step_interval_us)
 {
 	struct step_dir_stepper_common_data *data = dev->data;
 	const struct step_dir_stepper_common_config *config = dev->config;
 
-	if (velocity == 0) {
-		LOG_ERR("Velocity cannot be zero");
-		return -EINVAL;
-	}
-
-	if (velocity > USEC_PER_SEC) {
-		LOG_ERR("Velocity cannot be greater than %d micro steps per second", USEC_PER_SEC);
-		return -EINVAL;
-	}
-
 	K_SPINLOCK(&data->lock) {
-		data->max_velocity = velocity;
-		config->timing_source->update(dev, velocity);
+		data->step_interval_us = step_interval_us;
+		if (step_interval_us > 0) {
+			config->timing_source->update(dev, step_interval_us);
+		} else {
+			config->timing_source->stop(dev);
+		}
 	}
 
 	return 0;
@@ -294,15 +289,15 @@ int step_dir_stepper_common_move_to(const struct device *dev, const int32_t valu
 	struct step_dir_stepper_common_data *data = dev->data;
 	const struct step_dir_stepper_common_config *config = dev->config;
 
-	if (data->max_velocity == 0) {
-		LOG_ERR("Velocity not set or invalid velocity set");
+	if (data->step_interval_us == 0) {
+		LOG_ERR("Step interval not set or invalid step interval set");
 		return -EINVAL;
 	}
 
 	K_SPINLOCK(&data->lock) {
 		data->run_mode = STEPPER_RUN_MODE_POSITION;
 		data->step_count = value - data->actual_position;
-		config->timing_source->update(dev, data->max_velocity);
+		config->timing_source->update(dev, data->step_interval_us);
 		update_direction_from_step_count(dev);
 		config->timing_source->start(dev);
 	}
@@ -318,22 +313,16 @@ int step_dir_stepper_common_is_moving(const struct device *dev, bool *is_moving)
 	return 0;
 }
 
-int step_dir_stepper_common_run(const struct device *dev, const enum stepper_direction direction,
-				const uint32_t velocity)
+int step_dir_stepper_common_run(const struct device *dev, const enum stepper_direction direction)
 {
 	struct step_dir_stepper_common_data *data = dev->data;
 	const struct step_dir_stepper_common_config *config = dev->config;
 
 	K_SPINLOCK(&data->lock) {
 		data->run_mode = STEPPER_RUN_MODE_VELOCITY;
 		data->direction = direction;
-		data->max_velocity = velocity;
-		config->timing_source->update(dev, velocity);
-		if (velocity != 0) {
-			config->timing_source->start(dev);
-		} else {
-			config->timing_source->stop(dev);
-		}
+		config->timing_source->update(dev, data->step_interval_us);
+		config->timing_source->start(dev);
 	}
 
 	return 0;