Make step pulse timing per-driver instance members; add setMinStepPulse (fixes #136)#140
Open
laurb9 wants to merge 1 commit into
Open
Make step pulse timing per-driver instance members; add setMinStepPulse (fixes #136)#140laurb9 wants to merge 1 commit into
laurb9 wants to merge 1 commit into
Conversation
…se (fixes #136) The per-driver step_high_min/step_low_min/wakeup_time were declared as `static const int` in each derived driver, shadowing the base class members. Because BasicStepperDriver::nextAction() and enable() reference these with static binding to the base class, every driver actually used the base values (step_high_min=1, wakeup_time hardcoded 2us). The per-driver datasheet values were dead code, and step_low_min was never enforced (floor hardcoded to 1us). Changes: - BasicStepperDriver: step_high_min/step_low_min/wakeup_time are now protected `short` instance members (defaults 1/1/0). Derived drivers assign their datasheet values in every constructor (A4988 1/1/1000, DRV8825 2/2/1700, DRV8834 2/2/1000, DRV8880 1/1/1500, TMC2100 1/1/1000). DRV8880 uses 1 not 0 so a zero delay can't produce sub-datasheet pulses on fast ARM boards. - New public API: setMinStepPulse(high_us, low_us) plus getMinStepPulseHigh() and getMinStepPulseLow() inline getters. This is what fast MCUs like the Arduino Opta need (setMinStepPulse(8, 8)) since their GPIO outpaces the 1us default; fixes #136. - nextAction() now floors the STEP LOW interval at step_low_min instead of 1us, computed in unsigned form to avoid adding branches to the hot path. - enable() now waits wakeup_time (datasheet tWAKE), floored to 2us for the base default of 0. DRV8825 now correctly waits 1.7ms after wake before stepping. RAM impact: +6 bytes per driver instance (three shorts), acceptable. Verified: builds clean for arduino:avr:uno, sim-test verdicts match baseline. Agent: claude-opus-4-8 (Claude Code 2.1.153), amended and verified by claude-fable-5; max context and thinking level not exposed to the agent Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The per-driver
step_high_min/step_low_min/wakeup_timewere declaredstatic const intin each derived driver, shadowing the base class members. SinceBasicStepperDriver::nextAction()andenable()bind statically to the base class, every driver actually ran with a 1µs STEP pulse and the per-driver datasheet values were dead code;step_low_minwas never enforced andwakeup_timewas never used at all.Fixes #136 (Arduino Opta GPIO outpaces the driver input: users can now call
setMinStepPulse(8, 8)).Changes
step_high_min/step_low_min/wakeup_timeare now protectedshortinstance members; each driver assigns its datasheet values in all constructors (A4988 1/1/1000, DRV8825 2/2/1700, DRV8834 2/2/1000, DRV8880 1/1/1500, TMC2100 1/1/1000). DRV8880 uses 1µs, not the datasheet 0.47µs rounded down to 0, so fast ARM boards can't emit sub-datasheet pulses.setMinStepPulse(high_us, low_us),getMinStepPulseHigh(),getMinStepPulseLow(). The low value is floored at 1 becausenextAction()returning 0 signals end-of-move.nextAction()floors the STEP LOW interval atstep_low_min(computed in unsigned form, no extra branch on the hot path).enable()waits the driver'swakeup_time(tWAKE) instead of a hardcoded 2µs; DRV8825 now correctly waits 1.7ms after wake.Behavior / performance impact
enable()gains the datasheet wake delay (up to 1.7ms for DRV8825) — one-time cost per enable, prevents lost steps right after wake.shorts). Hot path instruction count unchanged (verified builds with--warnings all).Testing
make sim-test(simavr, ATmega328P): 14/14 verdict lines match the golden baseline.make BasicStepperDriver.hex / NonBlocking.hex / MultiAxis.hex TARGET=arduino:avr:uno: clean, no warnings in library code.pio ci --lib src --board esp32dev examples/BasicStepperDriver: SUCCESS (32-bit target).Agent: claude-opus-4-8 (Claude Code 2.1.153), reviewed and verified by claude-fable-5; max context and thinking level not exposed to the agent
🤖 Generated with Claude Code