firmata / ConfigurableFirmata

A plugin-based version of Firmata
GNU Lesser General Public License v2.1
153 stars 72 forks source link

Stop is not correctly implemented? #86

Open nova76 opened 5 years ago

nova76 commented 5 years ago

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 commented 5 years ago

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

nova76 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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