diff --git a/drivers/stepper/h_bridge_stepper.c b/drivers/stepper/h_bridge_stepper.c index 6cd866317856b..a2f31afe8805b 100644 --- a/drivers/stepper/h_bridge_stepper.c +++ b/drivers/stepper/h_bridge_stepper.c @@ -189,6 +189,15 @@ static int h_bridge_stepper_move_by(const struct device *dev, int32_t micro_step LOG_ERR("Step interval not set or invalid step interval set"); return -EINVAL; } + + if (micro_steps == 0) { + (void)k_work_cancel_delayable(&data->stepper_dwork); + if (data->callback) { + data->callback(data->dev, STEPPER_EVENT_STEPS_COMPLETED, + data->event_cb_user_data); + } + return 0; + } K_SPINLOCK(&data->lock) { data->run_mode = STEPPER_RUN_MODE_POSITION; data->step_count = micro_steps; diff --git a/drivers/stepper/step_dir/step_dir_stepper_common.c b/drivers/stepper/step_dir/step_dir_stepper_common.c index 25a72916f58a1..30c76577fd9d3 100644 --- a/drivers/stepper/step_dir/step_dir_stepper_common.c +++ b/drivers/stepper/step_dir/step_dir_stepper_common.c @@ -11,7 +11,6 @@ LOG_MODULE_REGISTER(step_dir_stepper, CONFIG_STEPPER_LOG_LEVEL); static inline int step_dir_stepper_perform_step(const struct device *dev) { const struct step_dir_stepper_common_config *config = dev->config; - struct step_dir_stepper_common_data *data = dev->data; int ret; ret = gpio_pin_toggle_dt(&config->step_pin); @@ -28,12 +27,6 @@ static inline int step_dir_stepper_perform_step(const struct device *dev) } } - if (data->direction == STEPPER_DIRECTION_POSITIVE) { - data->actual_position++; - } else { - data->actual_position--; - } - return 0; } @@ -120,15 +113,10 @@ static void stepper_work_event_handler(struct k_work *work) static void update_remaining_steps(struct step_dir_stepper_common_data *data) { - const struct step_dir_stepper_common_config *config = data->dev->config; - - if (data->step_count > 0) { - data->step_count--; - } else if (data->step_count < 0) { - data->step_count++; - } else { - stepper_trigger_callback(data->dev, STEPPER_EVENT_STEPS_COMPLETED); - config->timing_source->stop(data->dev); + if (atomic_get(&data->step_count) > 0) { + atomic_dec(&data->step_count); + } else if (atomic_get(&data->step_count) < 0) { + atomic_inc(&data->step_count); } } @@ -136,9 +124,9 @@ static void update_direction_from_step_count(const struct device *dev) { struct step_dir_stepper_common_data *data = dev->data; - if (data->step_count > 0) { + if (atomic_get(&data->step_count) > 0) { data->direction = STEPPER_DIRECTION_POSITIVE; - } else if (data->step_count < 0) { + } else if (atomic_get(&data->step_count) < 0) { data->direction = STEPPER_DIRECTION_NEGATIVE; } else { LOG_ERR("Step count is zero"); @@ -150,23 +138,20 @@ static void position_mode_task(const struct device *dev) struct step_dir_stepper_common_data *data = dev->data; const struct step_dir_stepper_common_config *config = dev->config; - if (data->step_count) { - (void)step_dir_stepper_perform_step(dev); - } + update_remaining_steps(dev->data); - if (config->timing_source->needs_reschedule(dev) && data->step_count != 0) { + if (config->timing_source->needs_reschedule(dev) && atomic_get(&data->step_count) != 0) { (void)config->timing_source->start(dev); + } else if (atomic_get(&data->step_count) == 0) { + stepper_trigger_callback(data->dev, STEPPER_EVENT_STEPS_COMPLETED); + config->timing_source->stop(data->dev); } - - update_remaining_steps(dev->data); } static void velocity_mode_task(const struct device *dev) { const struct step_dir_stepper_common_config *config = dev->config; - (void)step_dir_stepper_perform_step(dev); - if (config->timing_source->needs_reschedule(dev)) { (void)config->timing_source->start(dev); } @@ -176,18 +161,23 @@ void stepper_handle_timing_signal(const struct device *dev) { struct step_dir_stepper_common_data *data = dev->data; - K_SPINLOCK(&data->lock) { - switch (data->run_mode) { - case STEPPER_RUN_MODE_POSITION: - position_mode_task(dev); - break; - case STEPPER_RUN_MODE_VELOCITY: - velocity_mode_task(dev); - break; - default: - LOG_WRN("Unsupported run mode: %d", data->run_mode); - break; - } + (void)step_dir_stepper_perform_step(dev); + if (data->direction == STEPPER_DIRECTION_POSITIVE) { + atomic_inc(&data->actual_position); + } else { + atomic_dec(&data->actual_position); + } + + switch (data->run_mode) { + case STEPPER_RUN_MODE_POSITION: + position_mode_task(dev); + break; + case STEPPER_RUN_MODE_VELOCITY: + velocity_mode_task(dev); + break; + default: + LOG_WRN("Unsupported run mode: %d", data->run_mode); + break; } } @@ -243,9 +233,15 @@ int step_dir_stepper_common_move_by(const struct device *dev, const int32_t micr return -EINVAL; } + if (micro_steps == 0) { + stepper_trigger_callback(data->dev, STEPPER_EVENT_STEPS_COMPLETED); + config->timing_source->stop(dev); + return 0; + } + K_SPINLOCK(&data->lock) { data->run_mode = STEPPER_RUN_MODE_POSITION; - data->step_count = micro_steps; + atomic_set(&data->step_count, micro_steps); update_direction_from_step_count(dev); ret = update_dir_pin(dev); if (ret < 0) { @@ -259,7 +255,7 @@ int step_dir_stepper_common_move_by(const struct device *dev, const int32_t micr } int step_dir_stepper_common_set_microstep_interval(const struct device *dev, - const uint64_t microstep_interval_ns) + const uint64_t microstep_interval_ns) { struct step_dir_stepper_common_data *data = dev->data; const struct step_dir_stepper_common_config *config = dev->config; @@ -292,9 +288,7 @@ int step_dir_stepper_common_get_actual_position(const struct device *dev, int32_ { struct step_dir_stepper_common_data *data = dev->data; - K_SPINLOCK(&data->lock) { - *value = data->actual_position; - } + *value = atomic_get(&data->actual_position); return 0; } @@ -305,9 +299,7 @@ int step_dir_stepper_common_move_to(const struct device *dev, const int32_t valu int32_t steps_to_move; /* Calculate the relative movement required */ - K_SPINLOCK(&data->lock) { - steps_to_move = value - data->actual_position; - } + steps_to_move = value - atomic_get(&data->actual_position); return step_dir_stepper_common_move_by(dev, steps_to_move); } diff --git a/drivers/stepper/step_dir/step_dir_stepper_common.h b/drivers/stepper/step_dir/step_dir_stepper_common.h index b345cf5d68a5d..0bcea64487988 100644 --- a/drivers/stepper/step_dir/step_dir_stepper_common.h +++ b/drivers/stepper/step_dir/step_dir_stepper_common.h @@ -71,9 +71,9 @@ struct step_dir_stepper_common_data { struct k_spinlock lock; enum stepper_direction direction; enum stepper_run_mode run_mode; - int32_t actual_position; uint64_t microstep_interval_ns; - int32_t step_count; + atomic_t actual_position; + atomic_t step_count; stepper_event_callback_t callback; void *event_cb_user_data; diff --git a/tests/drivers/stepper/stepper_api/src/main.c b/tests/drivers/stepper/stepper_api/src/main.c index 9a2a1179b50c2..5c058f0e101f3 100644 --- a/tests/drivers/stepper/stepper_api/src/main.c +++ b/tests/drivers/stepper/stepper_api/src/main.c @@ -72,6 +72,9 @@ static void *stepper_setup(void) &stepper_signal); zassert_not_null(fixture.dev); + zassert_equal( + stepper_set_event_callback(fixture.dev, fixture.callback, (void *)fixture.dev), 0, + "Failed to set event callback"); (void)stepper_enable(fixture.dev); return &fixture; } @@ -131,12 +134,9 @@ ZTEST_F(stepper, test_target_position_w_fixed_step_interval) int ret; ret = stepper_set_microstep_interval(fixture->dev, 100 * USEC_PER_SEC); - if (ret == -ENOSYS) { ztest_test_skip(); } - /* Pass the function name as user data */ - (void)stepper_set_event_callback(fixture->dev, fixture->callback, (void *)fixture->dev); (void)stepper_move_to(fixture->dev, pos); @@ -154,7 +154,6 @@ ZTEST_F(stepper, test_move_by_positive_step_count) int32_t steps = 20; (void)stepper_set_microstep_interval(fixture->dev, 100 * USEC_PER_SEC); - (void)stepper_set_event_callback(fixture->dev, fixture->callback, (void *)fixture->dev); (void)stepper_move_by(fixture->dev, steps); POLL_AND_CHECK_SIGNAL( @@ -169,7 +168,6 @@ ZTEST_F(stepper, test_move_by_negative_step_count) int32_t steps = -20; (void)stepper_set_microstep_interval(fixture->dev, 100 * USEC_PER_SEC); - (void)stepper_set_event_callback(fixture->dev, fixture->callback, (void *)fixture->dev); (void)stepper_move_by(fixture->dev, steps); POLL_AND_CHECK_SIGNAL( @@ -181,8 +179,6 @@ ZTEST_F(stepper, test_move_by_negative_step_count) ZTEST_F(stepper, test_stop) { - (void)stepper_set_event_callback(fixture->dev, fixture->callback, (void *)fixture->dev); - /* Run the stepper in positive direction */ (void)stepper_run(fixture->dev, STEPPER_DIRECTION_POSITIVE); @@ -206,3 +202,22 @@ ZTEST_F(stepper, test_stop) zassert_unreachable("Stepper stop failed"); } } + +ZTEST_F(stepper, test_move_by_zero_steps) +{ + bool is_moving; + int err; + + err = stepper_set_microstep_interval(fixture->dev, 100 * USEC_PER_SEC); + if (err == -ENOSYS) { + ztest_test_skip(); + } + zassert_equal(err, 0, "Failed to set microstep interval"); + + zassert_equal(stepper_move_by(fixture->dev, 0), 0, "Failed to move by zero steps"); + POLL_AND_CHECK_SIGNAL(stepper_signal, stepper_event, STEPPER_EVENT_STEPS_COMPLETED, + K_NO_WAIT); + zassert_equal(stepper_is_moving(fixture->dev, &is_moving), 0, + "Failed to check if stepper is moving"); + zassert_equal(is_moving, false, "Stepper is still moving"); +}