-
Notifications
You must be signed in to change notification settings - Fork 7.3k
drivers: stepper: Fix stepper callbacks when using work_q #88835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
drivers: stepper: Fix stepper callbacks when using work_q #88835
Conversation
tests/drivers/stepper/stepper_api/boards/native_sim_adi_tmc2209_work_q.overlay
Outdated
Show resolved
Hide resolved
- platform:native_sim/native/64:DTC_OVERLAY_FILE="boards/native_sim_adi_tmc2209_work_q.overlay" | ||
extra_configs: | ||
- CONFIG_GPIO=y | ||
- CONFIG_COUNTER=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- CONFIG_COUNTER=y |
- platform:native_sim/native/64:DTC_OVERLAY_FILE="boards/native_sim_ti_drv84xx_work_q.overlay" | ||
extra_configs: | ||
- CONFIG_GPIO=y | ||
- CONFIG_COUNTER=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- CONFIG_COUNTER=y |
tests/drivers/stepper/stepper_api/boards/native_sim_allegro_a4979_work_q.overlay
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this Patch :)
can you run this script locally in order to fix compliance checks ./scripts/ci/check_compliance.py
.
POLL_AND_CHECK_SIGNAL(stepper_signal, stepper_event, STEPPER_EVENT_STEPS_COMPLETED, | ||
K_MSEC(pos * 120)); | ||
K_MSEC(pos * 130)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well make the 130 a define or a config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would adding Kconfig file with the parameter to the test and then adding the parameter to the testcase.yaml be a good solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that would be great actually. Thanks.
988f785
to
dfeb19e
Compare
Fix issue where stepper callbacks were not being called when using work_q. This was due to the steps being counted down before the work_q was rescheduled. Added additional unit tests to stepper_api to verify the fix. Signed-off-by: Josselin Bunt <josselin@sensible.health
dfeb19e
to
5d6dced
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be seperated into two commits.
How? Unit test and fix? |
Fix issue where stepper callbacks were not being called when using work_q. This was due to the steps being counted down before the work_q was rescheduled.
Added additional unit tests to stepper_api to verify the fix. Needed to increase the tolerance to 30% because of the timing of the work_q.
Fixes: #88821