search

waspinator / AccelStepper

Fork of AccelStepper
http://www.airspayce.com/mikem/arduino/AccelStepper/
Other
146 stars 86 forks source link

Speed is wrong - there is error accumulation in every runSpeed call #12

Open piotr07da opened 2 years ago

piotr07da commented 2 years ago

Lets assume I am calling runSpeed in my loop which runs once every 1000 microsecond. Lets assume I have set speed to 183.75 which means _stepInterval equals to 5442 microseconds so I should have step in loops number: 6 - because 5442us 1 = 5442us, 11 - because 5442us 2 = 10.884, 17 - because 5442us 3 = 16.326, 22 - because 5442us 4 = 21.768, 28 - because 5442us 5 = 27.210, 33 - because 5442us 6 = 32.652, ...

But what happens is this: After 1st loop (at 1000us), 2nd loop (at 2000us), ... 5th loop (at 5000us) nothing happens. But at the 6th loop (at 6000us) the condition (from runSpeed function) is true: if (time - _lastStepTime >= _stepInterval) which is correct.

The problem is that inside the if body there is a line: _lastStepTime = time; // Caution: does not account for costs in step()

which is wrong because steps will occure in loops number: 6 - because 6000us - _lastStepTime >= 5442us 12 - because 12000us - 6000us >= 5442us 18 - because 18000us - 12000us >= 5442us 22 - because 22000us - 18000us >= 5442us 28 - because 28000us - 22000us >= 5442us 34 - because 34000us - 28000us >= 5442us ...

and the error accumulates over time. So in any case when _stepInterval is not the same as interval between calling runStep then there is the error and difference in speed is quite dramatic.

At speed of 183.75 steps per second and going from position = 0 to position = 147 I expect it to happen in 800ms but it happens in 877ms which is huge error.

What I did to fix it locally was changing the line: _lastStepTime = time; // Caution: does not account for costs in step() to following line _lastStepTime += _stepInterval; // Caution: does not account for costs in step() which I believe is correct but becuase _lastStepTime is 0 at the beginning I had to make ugly trick and I locally initialized it inside setSpeed just because I am starting to loop just after calling setSpeed so in my case it worked.

After those fixes the result is as expected - all takes expected 800ms.

dbartelmus commented 2 years ago

Hello @piotr07da, is this still relevant bug? Should I consider implementing your fix into this library on my local instance?

piotr07da commented 2 years ago

Hi, @dbartelmus From what I see the bug still exists. However to fix it properly my ugly trick with _lastStepTime should be implemented in some nicer way. However changing this line: _lastStepTime = time; to this line _lastStepTime += _stepInterval; looks like correct way to calculate _lastStepTime.

Unfortunately I don't have that code anymore so I cannot check the details. I eventually implemented my own simple code because I didn't need acceleration, just plain linear rotation.