Skip to content
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

Stop is not correctly implemented? #86

Open
nova76 opened this issue Jan 23, 2019 · 13 comments
Open

Stop is not correctly implemented? #86

nova76 opened this issue Jan 23, 2019 · 13 comments

Comments

@nova76
Copy link

nova76 commented Jan 23, 2019

here's the code

if (stepCommand == ACCELSTEPPER_STOP) {
if (stepper[deviceNum]) {
stepper[deviceNum]->stop();
isRunning[deviceNum] = false;
reportPosition(deviceNum, true);
}
}

But the accelstepper library cannot handle the quick stop. Therefore, the deceleration must be waited before the stepper motor stops. Is the report too early?

@soundanalogous
Copy link
Member

Have you tested this? If so which Firmata client library did you use?
@dtex FYI

@nova76
Copy link
Author

nova76 commented Jan 24, 2019

ConfigurableFirmata 2.10.1
AccelStepper 1.58.0

It is not certain that this is a bug, maybe the author knowingly did this. It was a little confused for me,

When we issue the stop command to the motor, it doesn't stop yet. See stop command in accelstepper library (https://www.airspayce.com/mikem/arduino/AccelStepper/classAccelStepper.html#a638817b85aed9d5cd15c76a76c00aced). It only starts the deceleration So it doesn't complete, only stop/complete with just a few cycles later, if you set a deceleration parameter. The question is, can an earlier function be evaluated as correct? Is running the motor? Yes, it's running (it's rotating), but the stop process has started.

So, I think this would be correct (if there is a deceleration):
// isRunning[deviceNum] = true;
reportPosition(deviceNum, false);

@dtex
Copy link
Contributor

dtex commented Jan 24, 2019

I wouldn't expect it to report the position prior to the stop completing, but it sounds like that's what you are describing. I'll take a look after work tonight.

@dtex
Copy link
Contributor

dtex commented Jan 24, 2019

I'm not with hardware yet, but it looks like it will report the position twice. Once when stop is fired and once when the deceleration has stopped.

@dtex
Copy link
Contributor

dtex commented Jan 24, 2019

I'm running AccelStepper 1.23.0 and it stops immediately which is by design. I made a conscious decision to stop immediately even when acceleration is used. Here was my reasoning.

Is it decelerating when stopping for you? Perhaps something changed in accelStepper?

@jcvillegasfernandez
Copy link

jcvillegasfernandez commented Mar 6, 2019

I am trying to make a Lazarus client (pascal) and in my tests found Stop function does not work properly, it stops suddenly not smoothly.

Looking in the source code I think I found the problem, it works for me.

In AccelStepperFirmata.cpp file I removed or commented two lines

else if (stepCommand == ACCELSTEPPER_STOP) {
if (stepper[deviceNum]) {
stepper[deviceNum]->stop();
// isRunning[deviceNum] = false; // Line to comment
// reportPosition(deviceNum, true); // line to comment
}

@dtex
Copy link
Contributor

dtex commented Mar 6, 2019

It was a conscious decision to have stop not decelerate based on the use cases I imagined (limit switches and panic buttons). Tell me about your use case.

@jcvillegasfernandez
Copy link

I personally prefer a smooth stop but I agree with you in some cases it is better to stop suddenly (if you have limits like a garage door hitting your car), but that can cause problems too, imagine a motor with heavy load, that means a lot of inertia can cause problems.
The other problem is inconsistency because after a stop if you run motor again it starts at the speed it stopped (usually full speed).
Perhaps the solution will be two different stop functions or an enable disable sudden/smooth stop.

@dtex
Copy link
Contributor

dtex commented Mar 6, 2019

Ah, so we should definitely update the current speed. Let me look into how accelStepper would handle an immediate stop when a stepper has deceleration.

@soundanalogous
Copy link
Member

The AccelStepperFirmata documentation for the Stepper stop command does state "If an acceleration is set, stop will not be immediate". I propose we add a second command, one stopping immediately, the other decelerating until stop.

@jcvillegasfernandez
Copy link

Or maybe a little modified stop command like this
0 START_SYSEX (0xF0)
1 ACCELSTEPPER_DATA (0x62)
2 ACCELSTEPPER_STOP (0x05)
3 device number (0-9)
4 Stop_smoothly (0-1) // optional to send, that makes compatibilty
4 or 5 END_SYSEX (0xF7)

@jcvillegasfernandez
Copy link

Because I need to continue developing my Lazarus client I definitely changed the source code of AccelStepperFirmata, commenting the two lines, so I have a smooth stop now, when I need a sudden stop I have implemented a faststop function, setting Acceleration to max_acceleration, then calling stop (in this case with max deceleration) and then set normal acceleration again.

@LinuksGuru
Copy link

Is abrupt stop problem can be only solved as jcvillegasfernandez suggests or there update with fix on the way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants