gin66 / FastAccelStepper

A high speed stepper library for Atmega 168/328p (nano), Atmega32u4, Atmega 2560, ESP32, ESP32S2, ESP32S3, ESP32C3 and Atmel SAM Due
MIT License
282 stars 67 forks source link

Odd behaviour when stopping motors using moveByAcceleration (Perhaps) #262

Closed sharpie7 closed 2 weeks ago

sharpie7 commented 3 weeks ago

I am using the fastAccelStepper V0.30.11 library on an Arduino Nano (classic - ATMEGA 328) and it's been very useful. Many thanks for the hard work.

I have a problem in the project which may be linked to FastAccelStepper. I am using the following code to stop motors:

  s->moveByAcceleration(-quickAcc, false);
  s->applySpeedAcceleration();

and then testing if they are stopped using (stepper->getCurrentSpeedInMilliHz()!=0).

I have very occasional strange behaviours that could be explained if, rarely, the library didn't actually reach a point where getCurrentSpeedInMilliHz()==0 following that stop code.

Some questions:

  1. Is it possible that stopping a motor using the code above doesn't quite reach zero on getCurrentSpeedInMilliHz() (e.g. due to a rounding error)?
  2. Would isRunning() be a better test to determine if the motor has stopped?
  3. Would it be worth re-testing on a later version of the library - e.g. might the fix of #250 have changed this?

Many thanks.

gin66 commented 3 weeks ago
  1. After checking the code of getCurrentSpeedInXXX(), there is apparently one code path, where the result may be garbage. More or less, when the motor is stopped. Reason is, that in getCurrentSpeedInTicks() one code path lets the struct actual_ticks_s speed uninitialized. Seems to be a bug, which I need to investigate.
  2. isRunning() is better
  3. Yes, but it may not fix your problem.

If you have a stripped down examplle, which yields this error, then I can add it to my test suite.

sharpie7 commented 3 weeks ago

Thanks so much. That's very helpful. The possible bug that you describe does sound like it could be the cause of the behaviour I am seeing.

In the short term I will change to isRunning() to test if the motor has stopped.

Unfortunately my use-case is complicated so it's not useful for testing as-is. I will do some experiments to see if I can reproduce in a simple configuration.

sharpie7 commented 3 weeks ago

I have a test program that produces an error that corresponds to a behaviour that I observed in the poject. The test failure suggests that the problem is not just a wrong value returned by getCurrentSpeedInMilliHz() but actually a broader failure to terminate the deceleration normally.

The test accelerates up to a random value and then decelerates again while looking for various anomolies in getCurrentSpeedInMilliHz() and isRunning().

The code is quite long, so I'll put it at the end of this comment. I ran it on an Arduino Duemilanove (ATMEGA 168) using library version 0.30.12.

After several hours the output suddenly produced this output:

No stop iter 21.
No stop iter 22.
No stop iter 23.
No stop iter 24.
No stop iter 25.
No stop iter 26.
No stop iter 27.
...
No stop iter 16547.
... etc...

This means it is stuck in the while loop at line 87. Note that line 93 means that this specific output indicates that during this while loop that isRunning() is also stuck on true.

In a different configuraiton I also saw another error corresponding to the test on line 110 failing. I will see if changing the random seed can produce that again with this code.

Here is the code (sorry for the quality - it was a quick hack). memoryMix() attempts to randomize free memory to try and catch any unallocated memory usage. The meat of the test for the library is in loop()

#include <FastAccelStepper.h>

#define stepPin 9
#define dirPin 10
#define USB_BAUD 250000

FastAccelStepperEngine engine = FastAccelStepperEngine();
FastAccelStepper *stepper = NULL;

const int32_t quickAcc = 100000 * 4;
const int32_t acc = 2000;
const int32_t minSpeed = 1;
const int32_t maxSpeed = 2000;

void memoryMix() {
  int *array;
  size_t size = 1;
  size_t max_size = 0;

  while (1) {
    array = (int *)malloc(size * sizeof(int));
    if (array == NULL) {
        break;  // Stop when allocation fails
    } else {
        free(array);  // Free the successfully allocated block
        max_size = size;  // Update max_size to the last successful allocation size
        size *= 2;  // Double the size for the next allocation attempt
    }
  }

  // Perform a binary search to find the maximum allocatable size
  size_t low = max_size / 2;
  size_t high = max_size;
  while (low <= high) {
      size_t mid = (low + high) / 2;
      array = (int *)malloc(mid * sizeof(int));
      if (array == NULL) {
          high = mid - 1;
      } else {
          free(array);
          low = mid + 1;
          max_size = mid;
      }
  }

  // Allocate memory
  array = (int *)malloc((max_size -random( max_size -1))  * sizeof(int));
  if (array == NULL) {
      Serial.println("Memory error");
      return;
  }

    // Fill the array with random values
    for (int i = 0; i < size; i++) {
        array[i] = random(32767 );
    }

    free(array);
}

void setup() {
  engine.init();
  stepper = engine.stepperConnectToPin(stepPin);
  stepper->setDirectionPin(dirPin);
  Serial.begin(USB_BAUD);
  randomSeed(42);

}

void loop() {
  uint32_t sp, oldSp;
  int i, count;
  long target;
 // Serial.println("Starting");
  target = random(minSpeed, maxSpeed);
  stepper->setAcceleration(acc);
  stepper->setSpeedInHz(target);
  stepper->runForward();
  while (stepper-> getCurrentSpeedInMilliHz()/1000 < target - 100)
    {};
 // Serial.println("Stopping");
  stepper ->moveByAcceleration(-quickAcc, false);
  stepper ->applySpeedAcceleration();
  oldSp = stepper -> getCurrentSpeedInMilliHz();
  count = 0;
  while (sp = stepper -> getCurrentSpeedInMilliHz() != 0) {
    count ++;
    if (count > 20) {
      Serial.print("No stop iter ");
      Serial.print(count);
      Serial.print(".");
      if (!stepper -> isRunning())
        Serial.print("isRunning() reports stopped");
      Serial.println();
  }
    if (sp > oldSp) {
      Serial.print("Speed moves wrong way ");
      Serial.print(sp);
      Serial.print(">");
      Serial.print(oldSp);
      Serial.println();
    }
    oldSp = sp;
    memoryMix();
  }
  for (i =0; i< 100; i++) {
    memoryMix();
    sp = stepper-> getCurrentSpeedInMilliHz();
    if (sp !=0) {
      Serial.print("Speed error. Received value: ");
      Serial.print(sp);
      Serial.print(". Target speed was ");
      Serial.print(target);
      Serial.println();
    }
  }
}
gin66 commented 2 weeks ago

I have created a first test case on a branch. With the current implementation of mixer() the simulator drops for whatever reason into gdb mode. After replacing it with a delay() it is ok. With a too small delay, the "Speed moves wrong way" is triggered. I assume, this is because in the queue are still commands to increase the speed. So this should be fine.

What is surprising, that in the error case isRunning() keeps true. So there must be another issue. Perhaps you can add a print for the current speed and position in case of an error.

The issue with the "several hours" is, that I cannot add this to the test suite. So the issue has to occur much faster.

sharpie7 commented 2 weeks ago

The issue with the "several hours" is, that I cannot add this to the test suite. So the issue has to occur much faster.

I understand. At the moment I can't get the bug(s) to happen on demand. If I leave a program running for long enough eventually they happen.

I've seen enough data now to be pretty sure that there is some real problem, but I think it must be caused by an unusual combination of circumstances that doesn't happen often. Perhaps some odd timing issue.

I do have a new program with new output that provides more information. This output is from a run of about 18 hours on an Arduino Duemilanove (ATMEGA 168). Hopefully this is enough to give you a clue to the cause. It looks like the ramp gets stuck on "1".

Output:

No stop iter# 21. Value: 1 Target: 1978.isRunning() reports true.
No stop iter# 22. Value: 1 Target: 1978.isRunning() reports true.
No stop iter# 23. Value: 1 Target: 1978.isRunning() reports true.
No stop iter# 24. Value: 1 Target: 1978.isRunning() reports true.
...
No stop iter# 201. Value: 1 Target: 1978.isRunning() reports true.
Speed error. Received value: 1977992. Target speed was 1978
Speed error. Received value: 1977992. Target speed was 1978
Speed error. Received value: 1977992. Target speed was 1978
...
Speed error. Received value: 1977992. Target speed was 1978
- Long Delay
No stop iter# 21. Value: 1 Target: 1978.isRunning() reports true.
No stop iter# 22. Value: 1 Target: 1978.isRunning() reports true.
No stop iter# 23. Value: 1 Target: 1978.isRunning() reports true.
...
No stop iter# 201. Value: 1 Target: 1978.isRunning() reports true.
Speed error. Received value: 1977992. Target speed was 1978
Speed error. Received value: 1977992. Target speed was 1978
...
Speed error. Received value: 1977992. Target speed was 1978
- Long Delay
No stop iter# 21. Value: 1 Target: 7.isRunning() reports true.
No stop iter# 22. Value: 1 Target: 7.isRunning() reports true.

And the program

#include <FastAccelStepper.h>

#define stepPin 9
#define dirPin 10
#define USB_BAUD 250000

FastAccelStepperEngine engine = FastAccelStepperEngine();
FastAccelStepper *stepper = NULL;

const int32_t quickAcc = 100000 * 4;
const int32_t acc = 2000;
const int32_t minSpeed = 1;
const int32_t maxSpeed = 2000;

void memoryMix() {
  int *array;
  size_t size = 1;
  size_t max_size = 0;

  while (1) {
    array = (int *)malloc(size * sizeof(int));
    if (array == NULL) {
        break;  // Stop when allocation fails
    } else {
        free(array);  // Free the successfully allocated block
        max_size = size;  // Update max_size to the last successful allocation size
        size *= 2;  // Double the size for the next allocation attempt
    }
  }

  // Perform a binary search to find the maximum allocatable size
  size_t low = max_size / 2;
  size_t high = max_size;
  while (low <= high) {
      size_t mid = (low + high) / 2;
      array = (int *)malloc(mid * sizeof(int));
      if (array == NULL) {
          high = mid - 1;
      } else {
          free(array);
          low = mid + 1;
          max_size = mid;
      }
  }

  // Allocate memory
  array = (int *)malloc((max_size -random( max_size -1))  * sizeof(int));
  if (array == NULL) {
      Serial.println("Memory error");
      return;
  }

    // Fill the array with random values
    for (int i = 0; i < size; i++) {
        array[i] = random(32767 );
    }

    free(array);
}

void setup() {
  engine.init();
  stepper = engine.stepperConnectToPin(stepPin);
  stepper->setDirectionPin(dirPin);
  Serial.begin(USB_BAUD);
  randomSeed(42);

}

void loop() {
  uint32_t sp, oldSp;
  int i, count;
  long target;
 // Serial.println("Starting");
  target = random(minSpeed, maxSpeed);
  stepper->setAcceleration(acc);
  stepper->setSpeedInHz(target);
  stepper->runForward();
  while (stepper-> getCurrentSpeedInMilliHz()/1000 < target - 100)
    {};
 // Serial.println("Stopping");
  stepper ->moveByAcceleration(-quickAcc, false);
  stepper ->applySpeedAcceleration();
  oldSp = stepper -> getCurrentSpeedInMilliHz();
  count = 0;
  while (sp = stepper -> getCurrentSpeedInMilliHz() != 0) {
    count ++;
    if (count > 20) {
      Serial.print("No stop iter# ");
      Serial.print(count);
      Serial.print(". Value: ");
      Serial.print(sp);
      Serial.print(" Target: ");
      Serial.print(target);
      Serial.print(".");
      if (stepper -> isRunning())
        Serial.print("isRunning() reports true.");
      else
        Serial.print("isRunning() reports false.");
      Serial.println();
  }
    if (sp > oldSp) {
      Serial.print("Speed moves wrong way ");
      Serial.print(sp);
      Serial.print(">");
      Serial.print(oldSp);
      Serial.println();
    }
    oldSp = sp;
    memoryMix();
    if (count > 200)
      break;
  }
  for (i =0; i< 100; i++) {
    memoryMix();
    sp = stepper-> getCurrentSpeedInMilliHz();
    if (sp !=0) {
      Serial.print("Speed error. Received value: ");
      Serial.print(sp);
      Serial.print(". Target speed was ");
      Serial.print(target);
      Serial.println();
    }
  }
}
gin66 commented 2 weeks ago

Thanks for the feedback.

Could you please check, if you replace this line:

  array = (int *)malloc((max_size -random( max_size -1))  * sizeof(int));

by that line:

  array = (int *)malloc((max_size -random( max_size -1));
sharpie7 commented 2 weeks ago

I will try with that change, though I think the original does say what was intended.

Though I do see a big bug in the same routine later. I will do some more trials and let you know.

gin66 commented 2 weeks ago

You are right, I have confused the ()-nesting. Your mentioned bug is the size vs max_size in the for-loop ?

sharpie7 commented 2 weeks ago

I fixed the size Vs max_size bug, but ended up commenting out the calls to memoryMix() just to see if we can make a simpler test.

I tried the version below and after about 3 hours got this output. Though it's not completely stuck, it did spend about 220ms with the speed on 1.

Output:

helloNo stop iter# 21. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 22. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 23. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 24. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 25. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 26. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 27. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 28. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 29. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 30. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 31. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 32. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 33. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 34. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 35. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 36. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 37. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 38. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 39. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 40. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 41. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 42. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 43. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 44. Value: 1 Target: 4.isRunning() reports true.
No stop iter# 45. Value: 1 Target: 4.isRunning() reports true.

Code:

#include <FastAccelStepper.h>

#define stepPin 9
#define dirPin 10
#define USB_BAUD 250000

FastAccelStepperEngine engine = FastAccelStepperEngine();
FastAccelStepper *stepper = NULL;

const int32_t quickAcc = 100000 * 4;
const int32_t acc = 2000;
const int32_t minSpeed = 1;
const int32_t maxSpeed = 2000;

void memoryMix() {
  int *array;
  size_t size = 1;
  size_t max_size = 0;

  while (1) {
    array = (int *)malloc(size * sizeof(int));
    if (array == NULL) {
        break;  // Stop when allocation fails
    } else {
        free(array);  // Free the successfully allocated block
        max_size = size;  // Update max_size to the last successful allocation size
        size *= 2;  // Double the size for the next allocation attempt
    }
  }

  // Perform a binary search to find the maximum allocatable size
  size_t low = max_size / 2;
  size_t high = max_size;
  while (low <= high) {
      size_t mid = (low + high) / 2;
      array = (int *)malloc(mid * sizeof(int));
      if (array == NULL) {
          high = mid - 1;
      } else {
          free(array);
          low = mid + 1;
          max_size = mid;
      }
  }

  max_size = max_size -random( max_size -1);
  // Allocate memory
  array = (int *)malloc(max_size  * sizeof(int));
  if (array == NULL) {
      Serial.println("Memory error");
      return;
  }

    // Fill the array with random values
    for (int i = 0; i < max_size; i++) {
        array[i] = random(32767 );
    }

    free(array);
}

void setup() {
  engine.init();
  stepper = engine.stepperConnectToPin(stepPin);
  stepper->setDirectionPin(dirPin);
  Serial.begin(USB_BAUD);
  randomSeed(42);
  delay(100);
  Serial.print("hello");

}

void loop() {
  uint32_t sp, oldSp;
  int i, count;
  long target;
 // Serial.println("Starting");
  target = random(minSpeed, maxSpeed);
  stepper->setAcceleration(acc);
  stepper->setSpeedInHz(target);
  stepper->runForward();
  while (stepper-> getCurrentSpeedInMilliHz()/1000 < target - 100)
    {};
 // Serial.println("Stopping");
  stepper ->moveByAcceleration(-quickAcc, false);
  stepper ->applySpeedAcceleration();
  oldSp = stepper -> getCurrentSpeedInMilliHz();
  count = 0;
  while (sp = stepper -> getCurrentSpeedInMilliHz() != 0) {
    count ++;
    if (count > 20) {
      Serial.print("No stop iter# ");
      Serial.print(count);
      Serial.print(". Value: ");
      Serial.print(sp);
      Serial.print(" Target: ");
      Serial.print(target);
      Serial.print(".");
      if (stepper -> isRunning())
        Serial.print("isRunning() reports true.");
      else
        Serial.print("isRunning() reports false.");
      Serial.println();
  }
    if (sp > oldSp) {
      Serial.print("Speed moves wrong way ");
      Serial.print(sp);
      Serial.print(">");
      Serial.print(oldSp);
      Serial.println();
    }
    oldSp = sp;
    // memoryMix();
    delay(5);
    if (count > 200)
      break;
  }
  for (i =0; i< 100; i++) {
   // memoryMix();
   delay(5);
    sp = stepper-> getCurrentSpeedInMilliHz();
    if (sp !=0) {
      Serial.print("Speed error. Received value: ");
      Serial.print(sp);
      Serial.print(". Target speed was ");
      Serial.print(target);
      Serial.println();
    }
  }
}
gin66 commented 2 weeks ago

Thanks for the update. Removing memoryMix() out of the equation helps a lot. I have updated the example/Issue262/Issue262.ino with your latest code and did some cleanup. I keep this now running for a while. Will see, what will show up.

BTW: while (sp = stepper -> getCurrentSpeedInMilliHz() != 0) { apparently does not do, what is expected. Using platformio, this code gives a warning about missing ()

gin66 commented 2 weeks ago

Failed :-)


helloNo stop iter# 21. Target: 2.isRunning() reports true...
No stop iter# 22. Target: 2.isRunning() reports true...
No stop iter# 23. Target: 2.isRunning() reports true...
No stop iter# 24. Target: 2.isRunning() reports true...
No stop iter# 25. Target: 2.isRunning() reports true...
No stop iter# 26. Target: 2.isRunning() reports true...
No stop iter# 27. Target: 2.isRunning() reports true...
No stop iter# 28. Target: 2.isRunning() reports true...
No stop iter# 29. Target: 2.isRunning() reports true...
No stop iter# 30. Target: 2.isRunning() reports true...
No stop iter# 31. Target: 2.isRunning() reports true...
No stop iter# 32. Target: 2.isRunning() reports true...
No stop iter# 33. Target: 2.isRunning() reports true...
No stop iter# 34. Target: 2.isRunning() reports true...
No stop iter# 35. Target: 2.isRunning() reports true...
No stop iter# 36. Target: 2.isRunning() reports true...
No stop iter# 37. Target: 2.isRunning() reports true...
No stop iter# 38. Target: 2.isRunning() reports true...
No stop iter# 39. Target: 2.isRunning() reports true...
No stop iter# 40. Target: 2.isRunning() reports true...
No stop iter# 41. Target: 2.isRunning() reports true...
No stop iter# 42. Target: 2.isRunning() reports true...
No stop iter# 43. Target: 2.isRunning() reports true...
No stop iter# 44. Target: 2.isRunning() reports true...
No stop iter# 45. Target: 2.isRunning() reports true...
No stop iter# 46. Target: 2.isRunning() reports true...
No stop iter# 47. Target: 2.isRunning() reports true...
No stop iter# 48. Target: 2.isRunning() reports true...
No stop iter# 49. Target: 2.isRunning() reports true...
No stop iter# 50. Target: 2.isRunning() reports true...
No stop iter# 51. Target: 2.isRunning() reports true...
No stop iter# 52. Target: 2.isRunning() reports true...
No stop iter# 53. Target: 2.isRunning() reports true...
No stop iter# 54. Target: 2.isRunning() reports true...
No stop iter# 55. Target: 2.isRunning() reports true...
No stop iter# 56. Target: 2.isRunning() reports true...
No stop iter# 57. Target: 2.isRunning() reports true...
No stop iter# 58. Target: 2.isRunning() reports true...
No stop iter# 59. Target: 2.isRunning() reports true...
No stop iter# 60. Target: 2.isRunning() reports true...
No stop iter# 61. Target: 2.isRunning() reports true...
No stop iter# 62. Target: 2.isRunning() reports true...
No stop iter# 63. Target: 2.isRunning() reports true...
No stop iter# 64. Target: 2.isRunning() reports true...
No stop iter# 65. Target: 2.isRunning() reports true...
No stop iter# 66. Target: 2.isRunning() reports true...
No stop iter# 67. Target: 2.isRunning() reports true...
No stop iter# 68. Target: 2.isRunning() reports true...
No stop iter# 69. Target: 2.isRunning() reports true...
No stop iter# 70. Target: 2.isRunning() reports true...
No stop iter# 71. Target: 2.isRunning() reports true...
No stop iter# 72. Target: 2.isRunning() reports true...
No stop iter# 73. Target: 2.isRunning() reports true...
No stop iter# 74. Target: 2.isRunning() reports true...
No stop iter# 75. Target: 2.isRunning() reports true...
No stop iter# 76. Target: 2.isRunning() reports true...
No stop iter# 77. Target: 2.isRunning() reports true...
No stop iter# 78. Target: 2.isRunning() reports true...
No stop iter# 79. Target: 2.isRunning() reports true...
No stop iter# 80. Target: 2.isRunning() reports true...
No stop iter# 81. Target: 2.isRunning() reports true...
No stop iter# 82. Target: 2.isRunning() reports true...
No stop iter# 83. Target: 2.isRunning() reports true...
No stop iter# 84. Target: 2.isRunning() reports true...
No stop iter# 85. Target: 2.isRunning() reports true...
No stop iter# 86. Target: 2.isRunning() reports true...
No stop iter# 87. Target: 2.isRunning() reports true...
No stop iter# 88. Target: 2.isRunning() reports true...
No stop iter# 89. Target: 2.isRunning() reports true...
No stop iter# 90. Target: 2.isRunning() reports true...
No stop iter# 21. Target: 6.isRunning() reports true...

Curious, what the reason is.

gin66 commented 2 weeks ago

Changing the speed to maxSpeed = 10 makes the issue appear immediately.

The issue is the following: The code expects, that the motor stops within 100ms. But with e.g. target=2, it actually takes 450ms. Now target 2 means: 2 steps/s, this means the time between steps is 500ms. Consequently, in this case the application should not expect for the motor to actually "stop" within 100ms, but 500ms is reasonable.

Background for this implementation:

gin66 commented 2 weeks ago

forgot, there is still the issue of missing initialization of the struct….

sharpie7 commented 2 weeks ago

Well done!

I am glad it wasn't just something wrong in the way that I was using the library that was causing the problem.

gin66 commented 2 weeks ago

Hope the lib will now work as per your expectation.

sharpie7 commented 2 weeks ago

Thanks for detailed investigation and diagnosis. The explanation is helpful.

I think there is still an issue at higher speeds, probably with a different cause. I will open a new issue for that as this one has run its course.