drivers: stepper: tests: Test Generalization #85658

jbehrensnx · 2025-02-12T10:21:56Z

This PR includes the test generalization discussed in PR #82843.

This PR has 3 objectives, first to offer a set of test suites for stepper drivers that can be used to help develop new drivers. The second objective is to offer a specification of the expected API behavior, something that is currently missing. And thirdly, to offer a tool to evaluate new drivers before approval.

While several stepper drivers have been developed, testing was often done only haphazardly, primarily using the stepper shell, such as PR #79120. Testing via the shell is not only inconvenient and time consuming, it is also unreliable and non-repeatable, leading to many bugs and other issues to slip through. This set of test suites counteracts this by offering a set of repeatable tests that cover a wide range of expected behavior, making development easier and improving driver quality.

The stepper API currently lacks a specification for expected behavior. This not only makes driver development more difficult, as it is unclear how the new driver should behave in certain situations, it also leads to differences in behavior between drivers. This is in stark contrast to the goals of an API, where the user shouldn't care about the specific implementation and instead each api call should lead to the same result. These tests can alleviate this by defining expected behavior. Drivers that successfully pass them behave the same in the covered areas, improving their interchangeability from the user perspective.

Thirdly, does these test suites offer a way to evaluate new drivers before approval and thus ensure a certain level quality. Ideally, a new stepper should pass all applicable tests before it is approved.

Test Suites

stepper_api

This is a complete replacement of the existing stepper_api test suite. It checks general behavior of the driver and the results of incorrect input.

stepper_const_velocity

This test suite checks that drivers that don't use acceleration and thus run at a constant velocity, move as expected. This includes that move_to and move_by finish in the expected time, and that all three movement commands result in the expected position after a given time.

Current Test Results

All Tests were executed on a Nucleo F767ZI board with the CONFIG_ZTEST_SHUFFLE flag set, ensuring both random order and repetition. For the GPIO Stepper Controller, incorrect spinlock usage results in test_micro_step_res_set_incorrect_value_detection to crash the system. For this driver the test is skipped and considered to fail. The following table gives the test pass rate for both test suites and all tested drivers.

Driver	stepper_api	stepper_const_velocity
GPIO Stepper Controller	83.33%	100%
DRV8424 Work Queue	88.89%	60%
DRV8424 Counter	100%	100%
TMC2209 Work Queue	50%	60%
TMC2209 Counter	55.56%	100%

Limitations

Like all tests, this set of test suites has its limitations. While the new stepper_api test suite should be applicable to all stepper driver, the stepper_const_velocity, as the name suggests, is only meant for drivers that move at a constant velocity. Drivers that use acceleration, such as tmc50xx or the acceleration step-dir implementation currently under development require their own test suite to evaluate movement.

Another limitation of this set of test suites is that they are limited to evaluating api return values. They can not check the physical behavior. In the case of a step-dir driver for example, they can not check, if the order is given for 100 steps, that the step signal actually send 100 pulses and that they occurred with the right frequency. Twisters support for these kinds of tests is currently limited. In addition, they require a far greater degree of customization for the target driver, going beyond the scope of this PR.

Possible Future Work

Fixing existing stepper drivers so that they pass all tests.
Development of tests that evaluate actual outputs. Likely to be fairly board/driver specific, but some generalization for step-dir might be possible.
Development of tests for drivers that use acceleration. Very high likelihood of being driver specific.
Discussion (possibly in this PR) about acceptance criteria for new stepper drivers.

faxe1008

First rough pass, thanks @jbehrensnx amazing effort!

faxe1008 · 2025-02-12T10:28:49Z

tests/drivers/stepper/stepper_const_velocity/Kconfig

@@ -6,4 +6,7 @@ config ENTROPY_GENERATOR
 	bool "Shuffle the order of tests and suites"
 	default y

+config SPEED_STEPS
+	int "Test speed in microsteps/s, also used as number of steps to make for certain tests"


a bit long and a grammar error, move this to a help text maybe?

faxe1008 · 2025-02-12T10:30:40Z

tests/drivers/stepper/stepper_api/Kconfig

+config ENTROPY_GENERATOR
+	depends on ZTEST_SHUFFLE
+	bool "Shuffle the order of tests and suites"
+	default y


This looks like its reverse - shuffling would depend on the entropy generator no?

Technically yes, but it is in that order to ensure that ENTROPY_GENERATOR is automatically selected when shuffling. ZTEST_SHUFFLE having select ENTROPY_GENERATOR is probably the better option

tests/drivers/stepper/stepper_api/Kconfig

tests/drivers/stepper/stepper_api/boards/mimxrt1060_evk_mimxrt1062_qspi_B.overlay

jbehrensnx · 2025-02-17T08:03:47Z

@jilaypandya For some reason you were not included automatically

jilaypandya · 2025-02-17T17:32:15Z

Thanks @jbehrensnx great work in unifying the tests :)

The stepper API currently lacks a specification for expected behavior.

Should we not fix the stepper api specification before writing tests, i mean it's probably a change to the API documentation, or?

jbehrensnx · 2025-02-19T09:01:48Z

Thanks @jbehrensnx great work in unifying the tests :)

The stepper API currently lacks a specification for expected behavior.

Should we not fix the stepper api specification before writing tests, i mean it's probably a change to the API documentation, or?

On some level you could say yes. The tests can however help us find cases where the current api specification is insufficient. That said, I would agree that we should complete the specification discussion before merging/approving this PR.

The other question is, should we discuss the api specification in this PR or in a RFC?

jbehrensnx · 2025-02-19T09:11:23Z

Fixed and added copyright information
Temporarily removed qemu_x86_64 from testcases (overlays remain) as the stepper_api tests currently fail for this platform, thus failing some github checks
Fixed wrong expected return value in one test (setting incorrect microstep resolution). This causes 1 test for tmc2209 and drv8424 to fail that previously passed and the inverse to happen for the fixed gpio driver

jilaypandya · 2025-02-19T18:33:16Z

Thanks @jbehrensnx great work in unifying the tests :)

The stepper API currently lacks a specification for expected behavior.

Should we not fix the stepper api specification before writing tests, i mean it's probably a change to the API documentation, or?

On some level you could say yes. The tests can however help us find cases where the current api specification is insufficient. That said, I would agree that we should complete the specification discussion before merging/approving this PR.

The other question is, should we discuss the api specification in this PR or in a RFC?

Yes a RFC sounds good 👍

jilaypandya · 2025-03-16T07:45:00Z

Please rebase :)

jbehrensnx · 2025-03-19T15:00:32Z

Significant rework, incorporating the changes that have occurred since. Replaced test_config.yaml with tags. Usage of alias instead of chosen, as that is what the stepper_api test suite was using.

tests/drivers/stepper/stepper_api/README.rst

tests/drivers/stepper/stepper_api/boards/mimxrt1060_evk_mimxrt1062_qspi_B/allegro_a4979.overlay

jilaypandya · 2025-03-19T17:54:10Z

tests/drivers/stepper/stepper_api/README.rst

+Mimxrt1060-EVKB
+---------------
+
+NOTE: For this test no real a4979 was used and thus the following pin configuration is


In overlays via comments we can and do mention the pinout. Please drop this and other tables

Suggested change

NOTE: For this test no real a4979 was used and thus the following pin configuration is

tests/drivers/stepper/stepper_const_velocity/README.rst

jilaypandya

While i do appreciate this extensive testing, for an api that is still experimental this would mean each time a change is introduced, quite a bit of refactoring will have to be undetaken

jilaypandya · 2025-03-19T18:12:18Z

tests/drivers/stepper/stepper_api/Kconfig

+	int "Test speed in microsteps/s"
+	default 50
+	help
+	  Besides being the test speed, this value is also used in several tests as the number of


Description says otherwise

tests/drivers/stepper/stepper_api/Kconfig

...rs/stepper/stepper_api/boards/mimxrt1060_evk_mimxrt1062_qspi_B/adi_tmc2209_workqueue.overlay

jilaypandya · 2025-03-19T18:15:50Z

tests/drivers/stepper/stepper_api/boards/native_sim/adi_tmc2209_workqueue.overlay

Can we have the moving of files in a separate commit?

I can see what I can do, but as squashing everything into one commit was not the last thing I did, it might be difficult.

Right now lot of different things are squashed into one commit. Let's bring the changes step by step in.
In the first step probably just transfer the existing drv8424_api tests to stepper_api. That would also make reviewing easier and give more context about what is happening.

tests/drivers/stepper/stepper_api/boards/nucleo_f767zi/adi_tmc2209.overlay

tests/drivers/stepper/stepper_api/boards/nucleo_f767zi/ti_drv8424.overlay

jilaypandya

I think there are some overlays which you have not actually tested on the hardware, can we please not include them? I stick to the example of nucleo_g071rb since i test few drivers that i have with that board.

Also this leaves room for further contributors who want to get their first PR badge :-)

Nucleo F767ZI
-------------

NOTE: For this test no real tmc2209 was used and thus the following pin configuration is
theoretical.

jilaypandya · 2025-03-19T18:24:44Z

...rs/stepper/stepper_api/boards/mimxrt1060_evk_mimxrt1062_qspi_B/adi_tmc2209_workqueue.overlay

Is this actually tested on the hardware or is it just suggestive

The drv8424, tmc2209 and a4979 drivers dont actually need hardware at this point. One can simply define and configure a node for them and then run the tests successfully. Which is what I did for the tmc and allegro.
I defined devicetree nodes for the drivers and tested them on the two boards. The wording is admittedly maybe a bit misleading, as I tested them on actual boards, just without an actual stepper controller.

It is basically the same as the native_sim tests, just on a real board and not a simulated one.

Updated the readme so that this is hopefully clearer.

jilaypandya · 2025-03-19T18:35:12Z

tests/drivers/stepper/stepper_const_velocity/src/main.c

Is this some pre-work for the upcoming ramp drivers?

More or less, plus the tmc50xx driver already exists. As we have drivers with differing movement behavior, separating these tests seemed like a good idea.

so set_micro_step_interval is the only way of setting speed in stepper api. tmc50xx in its current form does not support this, the timing of the steps is decided internally by the IC. One can set the ramp parameters and the rest of the work is taken over by the IC.

so stepper api inherently does not have any functions which would suggest anything other than a fixed step interval ticking i.e. set_micro_step_interval. Hence there is no need to diverge into two different test-suites as of now.

Actually there is. stepper_api is intended to include tests that all stepper drivers need to be able to pass. This includes tmc50xx. The remaining functionality needs to be tested in other testsuites. This is where stepper_const_velocity comes into play. Another test suite concerning the specific behavior of tmc50xx should also be developed at some point, but that is beyond the scope of this PR.

jilaypandya · 2025-03-19T18:36:29Z

tests/drivers/stepper/stepper_const_velocity/testcase.yaml

+    extra_args:
+      - platform:native_sim/native/64:DTC_OVERLAY_FILE="boards/native_sim/allegro_a4979.overlay"
+      - platform:nucleo_f767zi/stm32f767xx:DTC_OVERLAY_FILE="boards/nucleo_f767zi/allegro_a4979.overlay"
+      - platform:mimxrt1060_evk@B/mimxrt1062/qspi:DTC_OVERLAY_FILE="boards/mimxrt1060_evk_mimxrt1062_qspi_B/allegro_a4979.overlay"


please only add the overlays if they are actually tested on the hardware

NOTE: For this test no real a4979 was used and thus the following pin configuration is theoretical.

jilaypandya · 2025-03-19T19:03:33Z

tests/drivers/stepper/stepper_api/src/main.c

-		.callback = stepper_print_event_callback,
+		.callback = stepper_api_print_event_callback,
+		.base_ms_resolution = CONFIG_BASE_MS_RESOLUTION,
+		.test_ms_resolution = CONFIG_TEST_MS_RESOLUTION,


where exactly is this getting used?

During the rework I forgot to add the corresponding test.
Edit: Added missing test.

jilaypandya · 2025-03-19T19:04:28Z

tests/drivers/stepper/stepper_api/src/main.c

+	int32_t steps = 100;
+	bool moving = true;
+
+	(void)stepper_set_microstep_interval(fixture->dev, fixture->test_interval);


Suggested change

(void)stepper_set_microstep_interval(fixture->dev, fixture->test_interval);

(void)stepper_set_microstep_interval(fixture->dev, CONFIG_TEST_MS_RESOLUTION);

That constant is intended to be used to test that a new microstep resolution can be successfully set. I forgot to add the test during the rework, which will be fixed. Managing the interval via a constant is however something I will do.

jilaypandya · 2025-03-19T19:05:39Z

tests/drivers/stepper/stepper_api/src/main.c

+	(void)stepper_move_by(fixture->dev, steps);
+	(void)stepper_enable(fixture->dev, false);
+	(void)stepper_is_moving(fixture->dev, &moving);
+	zassert_false(moving, "Driver should not be in state is_moving after being disabled");


While this statement is indeed correct, its just a bit handful to read :)

Suggested change

zassert_false(moving, "Driver should not be in state is_moving after being disabled");

zassert_false(moving, "stepper should not be moving once disabled");

Your recommendation is somewhat imprecise. There is a difference between the stepper being in state is_moving versus actually moving (i.e. position counter changing). The new error message could indicate the latter, which is however tested in another test.

jilaypandya · 2025-03-19T19:06:19Z

tests/drivers/stepper/stepper_api/src/main.c

+	(void)stepper_enable(fixture->dev, true);
+	(void)k_msleep(100);
+	(void)stepper_is_moving(fixture->dev, &moving);
+	zassert_false(moving, "Driver should not be in state is_moving after being reenabled");


Suggested change

zassert_false(moving, "Driver should not be in state is_moving after being reenabled");

zassert_false(moving, "Stepper should not start moving upon being just enabled");

Being reenabled is actually an important part of the test and thus should occur in the error message.

jbehrensnx · 2025-03-20T08:22:48Z

I will drop the qemu and native sim platforms from the const_velocity testsuite, as they will always fail, and this causes issues with the automated tests.
For stepper_api I will remove the native_sim and qemu platforms from some tests for the same reason, but as in this case the failures are a result of the drivers themself and not timer issues in the simulation, the overlays and everything else will remain, so that they can be reenabled once the issues of the drivers are fixed.

jbehrensnx · 2025-03-20T08:25:36Z

While i do appreciate this extensive testing, for an api that is still experimental this would mean each time a change is introduced, quite a bit of refactoring will have to be undetaken

That is actually a very good reason to write all these tests, as they will help to ensure, that after these changes the api still behaves as expected. It is not quite Test Driven Development, but it borrows at least a few ideas from it.

jilaypandya · 2025-03-28T16:44:17Z

tests/drivers/stepper/stepper_api/src/main.c

+{
+	int32_t steps = 1000;
+	int32_t position_1;
+	int32_t position_2;


int32_t position_before_reenabled;
int32_t position_after_reenabled;

jilaypandya · 2025-03-28T16:47:55Z

tests/drivers/stepper/stepper_api/README.rst

+#################
+
+Tests the general functionality and behaviour of stepper drivers to ensure conformity to the stepper
+api specification. All new stepper drivers should pass this test suite. It should be noted, that this


All new stepper drivers should pass this test suite.

This is actually a process decision, tbh i have not seen this in any other driver subsystem till now.

I am aware of this. That said, having a test suite that every new driver should pass (excluding exceptional circumstances with a good reason, I am not ruling these out) would improve the quality and consistency of new stepper drivers.

There are multiple possible reasons why other driver subsystems have not done this. One of them is finding a common core of functionality to set as a criteria (which can be seen even with stepper drivers, considering I split it in 2 test suites). The counter drivers for example have an extensive test suite, but it is missing tests for some functions while having tests not every driver can pass.
Another reason is that it might be very difficult to test these drivers in a meaningful way.
Then there the large number of existing drivers that might fail, which would result in many changes to these. In that regard, the low number of stepper drivers is a boon, as it makes this quite manageable.
And finally, people might not have want/been able to spend the required resources on this.

Another thing to consider is that with the growing commercial importance of Zephyr, testing will become more important and the stepper api could get a headstart on this.

Also, the goal of these tests is to test that a driver fulfills the api specification. If a new driver fails these tests, it does not fulfill the specification, so why would we want to accept it?

If we do it this way, there will be followup discussions about issues we find and about clarifications, but I think it is an important first step.

And finally, people might not have want/been able to spend the required resources on this.

This is a very good reason to not increase the entry bar for new drivers.

And finally, people might not have want/been able to spend the required resources on this.

This is a very good reason to not increase the entry bar for new drivers.

This was in reference to spending the resources creating such a test suite.
Making the test suite mandatory doesn't really increase the entry bar for new drivers noticeably. In fact, one could argue that it lowers it. When developing a new driver you want to use the test suite during development anyway, as it is an easy way to check that your driver is doing the correct things. One will still need to use the shell on occasion, but the tests make the development process much easier. And with this background, succeeding the test suite becomes a normal part of developing a driver. Thus not raising the entry bar.
In the end, we have something akin to Test Driven Development, but with the tests already existing.

I am aware of this. That said, having a test suite that every new driver should pass (excluding exceptional circumstances with a good reason, I am not ruling these out)

What would be these exceptional circumstances?

I have mentioned this in one of the earlier comments as well, testing the purely gpio-based drivers is easy and as far as i can see can be very well covered by such test suites. But drivers like tmc5xxx or for that matter even tmc2209(only uart based) would need some sort of emulator. So what would we do in this case?

What would be these exceptional circumstances?

I'm not sure until it happens. As these tests are not perfect, there is always the possibility that there is some edge case or another reason that I did not consider In that case, these will need to be discussed. That is what I meant to say with this.

I have mentioned this in one of the earlier comments as well, testing the purely gpio-based drivers is easy and as far as i can see can be very well covered by such test suites. But drivers like tmc5xxx or for that matter even tmc2209(only uart based) would need some sort of emulator. So what would we do in this case?

Those that can be tested via native_sim/qemu are, the rest can't be tested this way. That is a limitation of Zephyr, that some things can only be tested on real hardware. If however, someone develops a new driver, they have the means to test it. In that case we either have to trust their word or ask for the test log. Its one of the reasons, why only new tests should need to pass the test suite, as at that point there is somebody that can test the driver, later on that is however no longer the case.
The ideal solution in these cases would be for someone to develop an emulator, or at least stubbs for testing under native_sim/qemu but that is a lot of work and would, as you mentioned, raise the bar for entry, which is why I consider that nice to have but not required (and there is an argument to be made, that these emulators should be developed by the hardware developers, but that is a whole other discussion).

jilaypandya · 2025-03-28T16:55:58Z

...rs/stepper/stepper_api/boards/mimxrt1060_evk_mimxrt1062_qspi_B/adi_tmc2209_workqueue.overlay

+		step-gpios = <&arduino_header 10 0>; /* D4 */
+		en-gpios = <&arduino_header 11 0>; /* D5 */
+		msx-gpios = <&arduino_header 12 GPIO_ACTIVE_HIGH>, /* D6 */
+			<&arduino_header 13 GPIO_ACTIVE_HIGH>; /* D7 */


msx-gpios = <&arduino_header 12 GPIO_ACTIVE_HIGH>, /* D6 */ <&arduino_header 13 GPIO_ACTIVE_HIGH>; /* D7 */

jilaypandya · 2025-03-28T17:01:00Z

...vers/stepper/stepper_api/boards/mimxrt1060_evk_mimxrt1062_qspi_B/zephyr_gpio_stepper.overlay

What's the upside of adding these overlays for so many different boards?

Partly they are the result of me testing these drivers on these boards too see how they perform. The other reason is that counters and possibly the working queue can behave slightly different between boards. And the step-dir drivers sometimes use the step-dir implementation in slightly different ways. Thus, testing drivers on many different boards can improve the chance of finding errors.

github-actions · 2025-06-03T00:39:50Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Adds a check for 0 steps to take for move_by in both step_dir_stepper_common and the h-bridge driver. Signed-off-by: Jan Behrens <[email protected]>

jbehrensnx · 2025-06-27T12:54:57Z

Next attempt. Moved all tests from drv84xx/api to stepper_api. I also had to make some fixes to existing drivers to fix some bugs.

This is still work in progress.

One addition is the drivers.stepper.stepper_api.generic test scenario. It is intended as an entry point for tests on physical hardware. The existing test scenarios don't work, as all of them load additional overlay files, which causes issues. That said, I am not really happy with my solution (and it has its own issues), so I am open to other ideas.

Also, before somebody asks, I removed the LOG_DBG in stepper_print_event_callback because it would cause native_sim to crash.

dipakgmx · 2025-06-27T16:42:09Z

tests/drivers/stepper/stepper_api/src/main.c

 {
-	int32_t pos = 10u;
-	int ret;
+	(void)stepper_enable(fixture->dev);


I think, a better check of these calls is necessary, rather than void-ing out the returns. You can do something like:

zassert_equal(stepper_enable(fixture->dev), 0, "Failed to enable device");

I don't think it is still necessary. Enable only charges the coils and has no impact on the stepper behavior otherwise. That said, a dedicated test for checking enable/disable would be an option. Checking it in every test case would be overkill imo.

I don't think it is still necessary.

Could you justify, why asserting the function call is not necessary? You would never know if the function call was erroneous if you void the return.

And this is not a unique pattern. Here are some examples from the ADC, RTC or EEPROM api tests.

Could you justify, why asserting the function call is not necessary? You would never know if the function call was erroneous if you void the return.

As written above, since one of the recent changes to the stepper api, the enable/disable functions no longer influence the step-logic. Thus their success or failure is irrelevant. Enable/disable is currently used because it was previously necessary and they would appear in a workflow on physical hardware, but I am currently wondering if it would be better to just remove them entirely.

@jilaypandya What is your opinion, should the enable/disable function calls be just removed? They no longer fulfill the functionality they previously did.

One should always assert the return values of function calls in tests.
you can do something like this
zassert_not_equal(stepper_enable(fixture->dev), -EIO, "Failed to enable device");, coz if the enable returns -ENOTSUP or 0, the test-suite shall pass.

you can move enable in before, so that you don't have to repeat it in each and every test.

@dipakgmx thanks for pointing this out, i will make sure i do this as well.

tests/drivers/stepper/stepper_api/Kconfig

tests/drivers/stepper/stepper_api/src/main.c

jbehrensnx · 2025-07-03T08:53:41Z

The drivers.stepper.stepper_api.generic issues can be seen in the failed tests as I would have to exclude all boards the ci is run on, which is not an option. I am thinking about instead allowing (and adding an overlay for) a single board for this test scenario. This should fix the current issue with .generic and allow developers to use it with the -K twister flag.

If a physical board is still a no-go, then allowing a single emulation platform (maybe qemu_x86_64/atom, as it is already used for the h-bridge driver) with a conventional overlay file is also an option.

EDIT: Edited

jilaypandya · 2025-07-03T09:17:05Z

The drivers.stepper.stepper_api.generic issues can be seen in the failed tests as I would have to blacklist all boards the ci is run on, which is not an option. I am thinking about instead whitelisting (and adding an overlay for) a single board for this test scenario. This should fix the current issue with .generic and allow developers to use it with the -K twister flag.

If a physical board is still a no-go, then whitelisting a single emulation platform (maybe qemu_x86_64/atom, as it is already used for the h-bridge driver) with a conventional overlay file is also an option.

https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#rule-a-2-inclusive-language

jbehrensnx · 2025-07-03T11:08:14Z

Removed the test filtering and improved the range check for the run tests.

I also removed the .generic scenario. I somehow forgot, when creating it, that the additional overlay files are platform filtered. The error that made me believe otherwise was something else on my side.

jilaypandya · 2025-07-03T12:58:51Z

tests/drivers/stepper/stepper_api/src/main.c

@@ -18,6 +19,8 @@ struct k_poll_signal stepper_signal;
 struct k_poll_event stepper_event;
 void *user_data_received;

+#define TEST_STEP_INTERVAL (20 * USEC_PER_SEC)


This has to be a Kconfig Configuration Option

jilaypandya · 2025-07-05T09:26:21Z

tests/drivers/stepper/stepper_api/src/main.c

 {
-	int32_t pos = 10u;
-	int ret;
+	(void)stepper_enable(fixture->dev);


One should always assert the return values of function calls in tests.
you can do something like this
zassert_not_equal(stepper_enable(fixture->dev), -EIO, "Failed to enable device");, coz if the enable returns -ENOTSUP or 0, the test-suite shall pass.

you can move enable in before, so that you don't have to repeat it in each and every test.

@dipakgmx thanks for pointing this out, i will make sure i do this as well.

jilaypandya · 2025-07-11T17:02:13Z

tests/drivers/stepper/stepper_api/src/main.c

+ZTEST_F(stepper, test_set_micro_step_res_valid)
+{
+	int ret =
+		stepper_set_micro_step_res(fixture->dev, CONFIG_STEPPER_TEST_MICROSTEP_RESOLUTION);


This looks a bit strange. Most probably happened because of clang-format.
Adjust the code as following.

int ret; ret = stepper_set_micro_step_res(fixture->dev, CONFIG_STEPPER_TEST_MICROSTEP_RESOLUTION);

jilaypandya · 2025-07-11T17:07:42Z

tests/drivers/stepper/stepper_api/src/main.c

 }

-ZTEST_F(stepper, test_target_position_w_fixed_step_interval)


Was this test similiar to a test in drv84xx test suite, and hence been deleted?

Please add the tests from drv84xx to stepper_api in a commit and then delete the duplicate test cases and do corresponding adjustments to stepper_api test suite in another commit. Thanks.

Moved the removal to the drv84xx removal commit and renamed it to clarify that it removes superfluous tests.

Separating modifications to the tests into a separate commit would be very difficult at this point, especially, as some changes are needed to run the tests. That said, I have adjusted the commit message to indicate that there also some modifications to tests.

jilaypandya · 2025-07-11T17:08:53Z

tests/drivers/stepper/stepper_api/src/main.c

 {
-	int32_t pos = 10u;
-	int ret;
+	(void)stepper_set_microstep_interval(fixture->dev, CONFIG_STEPPER_TEST_MICROSTEP_INTERVAL);


One of the previous review comments about asserting the return values of stepper_api functions needs to be addressed. Thanks.

Considering how often this function is called, I moved it into the before function and added a separate test for this. Because these tests are only for non-acceleration drivers, this should suffice.

tests/drivers/stepper/stepper_api/src/main.c

Moves the tests of the drv84xx/api test suite into the stepper_api test suite. To facilitate this and for cleanup purposes, several tests are modified. Signed-off-by: Jan Behrens <[email protected]>

Removes the drv84xx/api test suite as well as a duplicate test in stepper_api. Signed-off-by: Jan Behrens <[email protected]>

sonarqubecloud · 2025-07-14T13:38:49Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

faxe1008 reviewed Feb 12, 2025

View reviewed changes

jbehrensnx force-pushed the step-dir-tests branch from 6e98d0a to a2acc50 Compare February 17, 2025 09:18

jilaypandya added the area: Stepper label Feb 17, 2025

jilaypandya requested review from fabiobaltieri, dipakgmx and bjarki-andreasen February 17, 2025 18:12

jilaypandya self-assigned this Feb 17, 2025

jbehrensnx force-pushed the step-dir-tests branch from a2acc50 to d80586e Compare February 19, 2025 09:10

jbehrensnx mentioned this pull request Feb 19, 2025

drivers: stepper: tests Various Driver Fixes #85987

Closed

jbehrensnx mentioned this pull request Feb 20, 2025

RFC: drivers: stepper: API Specification Improvements #86052

Closed

jbehrensnx mentioned this pull request Mar 5, 2025

drivers: stepper: add stepper driver for allegro a4979 #86620

Merged

jilaypandya mentioned this pull request Mar 14, 2025

Nominate Jan Behrens (jbehrensnx) as Collaborator #87128

Closed

jbehrensnx mentioned this pull request Mar 18, 2025

tests: stepper_api: add a4979 to common stepper api tests #87231

Merged

jbehrensnx force-pushed the step-dir-tests branch from d80586e to fb71476 Compare March 19, 2025 15:00

zephyrbot requested a review from jilaypandya March 19, 2025 15:00

jilaypandya reviewed Mar 19, 2025

View reviewed changes

jbehrensnx force-pushed the step-dir-tests branch from fb71476 to 60dfb91 Compare March 20, 2025 13:13

jbehrensnx force-pushed the step-dir-tests branch from 0b73f17 to b43788e Compare March 28, 2025 14:22

jilaypandya reviewed Mar 28, 2025

View reviewed changes

github-actions bot added the Stale label Jun 3, 2025

github-actions bot closed this Jun 18, 2025

jbehrensnx linked an issue Jun 25, 2025 that may be closed by this pull request

Deprecate/Remove Driver Specific API tests in stepper area. #92103

Open

2 tasks

jbehrensnx mentioned this pull request Jun 25, 2025

Deprecate/Remove Driver Specific API tests in stepper area. #92103

Open

2 tasks

drivers: stepper: Adds input data check for move_by

90c6dbc

Adds a check for 0 steps to take for move_by in both step_dir_stepper_common and the h-bridge driver. Signed-off-by: Jan Behrens <[email protected]>

jbehrensnx reopened this Jun 27, 2025

jbehrensnx force-pushed the step-dir-tests branch from b43788e to 4e55a8e Compare June 27, 2025 12:55

dipakgmx reviewed Jun 27, 2025

View reviewed changes

github-actions bot removed the Stale label Jun 28, 2025

jilaypandya requested changes Jul 2, 2025

View reviewed changes

tests/drivers/stepper/stepper_api/Kconfig Outdated Show resolved Hide resolved

tests/drivers/stepper/stepper_api/src/main.c Outdated Show resolved Hide resolved

tests/drivers/stepper/stepper_api/src/main.c Show resolved Hide resolved

jbehrensnx force-pushed the step-dir-tests branch from 4e55a8e to af9bf00 Compare July 3, 2025 07:51

jbehrensnx force-pushed the step-dir-tests branch from af9bf00 to 431afe0 Compare July 3, 2025 11:04

jilaypandya reviewed Jul 5, 2025

View reviewed changes

jbehrensnx force-pushed the step-dir-tests branch from 431afe0 to 977160c Compare July 7, 2025 10:29

jilaypandya reviewed Jul 11, 2025

View reviewed changes

jilaypandya reviewed Jul 12, 2025

View reviewed changes

tests/drivers/stepper/stepper_api/src/main.c Outdated Show resolved Hide resolved

tests/drivers/stepper/stepper_api/src/main.c Outdated Show resolved Hide resolved

tests/drivers/stepper/stepper_api/src/main.c Show resolved Hide resolved

jbehrensnx force-pushed the step-dir-tests branch from 977160c to 559b675 Compare July 14, 2025 12:05

drivers: stepper: tests: Transfers drv84xx api tests to stepper_api

a214e4b

Moves the tests of the drv84xx/api test suite into the stepper_api test suite. To facilitate this and for cleanup purposes, several tests are modified. Signed-off-by: Jan Behrens <[email protected]>

jbehrensnx force-pushed the step-dir-tests branch from 559b675 to 35ea2df Compare July 14, 2025 12:47

drivers: stepper: tests: Removes superfluous Tests

7ead59e

Removes the drv84xx/api test suite as well as a duplicate test in stepper_api. Signed-off-by: Jan Behrens <[email protected]>

jbehrensnx force-pushed the step-dir-tests branch from 35ea2df to 7ead59e Compare July 14, 2025 13:11

	(void)stepper_set_microstep_interval(fixture->dev, fixture->test_interval);
	(void)stepper_set_microstep_interval(fixture->dev, CONFIG_TEST_MS_RESOLUTION);

	zassert_false(moving, "Driver should not be in state is_moving after being disabled");
	zassert_false(moving, "stepper should not be moving once disabled");

		}

		ZTEST_F(stepper, test_target_position_w_fixed_step_interval)

drivers: stepper: tests: Test Generalization #85658

Are you sure you want to change the base?

drivers: stepper: tests: Test Generalization #85658

Conversation

jbehrensnx commented Feb 12, 2025

Test Suites

stepper_api

stepper_const_velocity

Current Test Results

Limitations

Possible Future Work

Uh oh!

faxe1008 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jbehrensnx commented Feb 17, 2025

Uh oh!

jilaypandya commented Feb 17, 2025

Uh oh!

jbehrensnx commented Feb 19, 2025

Uh oh!

jbehrensnx commented Feb 19, 2025

Uh oh!

jilaypandya commented Feb 19, 2025

Uh oh!

jilaypandya commented Mar 16, 2025

Uh oh!

jbehrensnx commented Mar 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jilaypandya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jilaypandya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jbehrensnx Mar 20, 2025 •

edited

Loading