arduino-libraries / Stepper

Stepper Library for Arduino
http://arduino.cc/
107 stars 114 forks source link

Stepper class does not initialize step_delay which can lead to lock-ups #18

Open per1234 opened 3 years ago

per1234 commented 3 years ago

Moved from https://github.com/arduino/Arduino/issues/3300 by @cxrodgers

I was having an intermittent issue where my arduino would occasionally lock up while stepping a stepper. It turned out that it was because I had neglected to call setSpeed explicitly. I had thought that it defaulted to some reasonable value, but upon examining the code, I see that it does not, meaning that step_delay will be whatever random value resides in that memory location. https://github.com/arduino/Arduino/blob/master/libraries/Stepper/src/Stepper.cpp

Here was my forum posting on this topic. http://forum.arduino.cc/index.php?topic=327132.0

I believe that the memory is typically set to zero, but when using dynamic memory allocation (for instance: stepper = new Stepper(...), there is no guarantee that this memory location will start at zero. Regardless, it seems dangerous to rely on the memory containing any particular value without being initialized.

I think step_delay should be initialized to zero in the constructor. Alternatively, I think it should be documented that a call to setSpeed is required before calling step.

per1234 commented 3 years ago

From katarinayee on 2016-11-23:

Code examples should be sure to setSpeed explicitly, e.g., the "One Step At A Time" example code will produce motor malfunctions if the code in the loop is changed to go more than one step at a time.

per1234 commented 3 years ago

From amayii0 on 2017-08-09:

Found this report as I had the exact same observation. Others are explictly set to 0, this one should be too. https://github.com/arduino/Arduino/blob/master/libraries/Stepper/src/Stepper.cpp#L87

this->step_number = 0;    // which step the motor is on
this->direction = 0;      // motor direction
this->last_step_time = 0; // time stamp in us of the last step taken

BTW I find tricky to have 3 very close constructors because of wires count. Could be refactored at the same time (no time for a PR right now sorry).