Skip to main content
+ a few comments.
Source Link
Edgar Bonet
  • 44.3k
  • 4
  • 41
  • 79

Edit: Here are some extra comments about your question and your sketch, not necessarily related to the problem you are seeing.

You wrote:

measuring [loop()] with micros() proved it never took longer than 0.26ms.

It seems you measured incorrectly. Maybe you measured a version of the program that was not printing so much to the serial port. Or maybe you looked only at the first iterations of the loop, where Serial.println() is fast because the output buffer hasn't filled yet.

The following code is fragile:

bool accel(double increment, double newSpeed) {
  if (abs(Speed - newSpeed) < 0.1) {
    return true;
  }
  // ...
}

The value 0.1 is arbitrary, and the “right choice” is dependent on the program timing. I suggest something more robust, like:

bool accel(double increment, double newSpeed) {
  if (increment > 0) {
    Speed = min(Speed + increment, newSpeed);
  } else {
    Speed = max(Speed + increment, newSpeed);
  }
  return Speed == newSpeed;
}

Note that, although it is generally recommended to not test floating point numbers for exact equality, in this case it is safe, because min() and max() return exactly one of their arguments.

The optimization consisting of calling motor::accel() only during the acceleration phase is futile: this methods takes an very tiny fraction of the loop() time. In the same vein, there would be no harm in calling Servo::writeMicroseconds() on every loop iteration, even if some of the calls turn out to be useless.

The code would be more readable if instructions was an array of struct rather than an array of arrays: this way the fields of each instruction could have evocative names rather than numeric indices.

You are using too much floating point here. Most of the data fields, including all the contents of instructions, could be integers of suitable length.


Edit: Here are some extra comments about your question and your sketch, not necessarily related to the problem you are seeing.

You wrote:

measuring [loop()] with micros() proved it never took longer than 0.26ms.

It seems you measured incorrectly. Maybe you measured a version of the program that was not printing so much to the serial port. Or maybe you looked only at the first iterations of the loop, where Serial.println() is fast because the output buffer hasn't filled yet.

The following code is fragile:

bool accel(double increment, double newSpeed) {
  if (abs(Speed - newSpeed) < 0.1) {
    return true;
  }
  // ...
}

The value 0.1 is arbitrary, and the “right choice” is dependent on the program timing. I suggest something more robust, like:

bool accel(double increment, double newSpeed) {
  if (increment > 0) {
    Speed = min(Speed + increment, newSpeed);
  } else {
    Speed = max(Speed + increment, newSpeed);
  }
  return Speed == newSpeed;
}

Note that, although it is generally recommended to not test floating point numbers for exact equality, in this case it is safe, because min() and max() return exactly one of their arguments.

The optimization consisting of calling motor::accel() only during the acceleration phase is futile: this methods takes an very tiny fraction of the loop() time. In the same vein, there would be no harm in calling Servo::writeMicroseconds() on every loop iteration, even if some of the calls turn out to be useless.

The code would be more readable if instructions was an array of struct rather than an array of arrays: this way the fields of each instruction could have evocative names rather than numeric indices.

You are using too much floating point here. Most of the data fields, including all the contents of instructions, could be integers of suitable length.

Source Link
Edgar Bonet
  • 44.3k
  • 4
  • 41
  • 79

There are two things going on here.

You are printing too much to the serial port. After the first second (between 1000 and 9999 ms), each line you print is 20 bytes, including the CRLF terminator. At 9600 b/s, each bytes takes 1.04 ms (1 start bit, 8 data bites and 1 stop bit). This is about 20.8 ms for the whole line, which is slightly longer than interval. You should either print less often (say, every other loop iteration) or faster. Just moving to the next standard speed (19200 b/s) will be enough to ensure you do not miss any interval. You can go faster though.

The second issue is the way you keep track of previousMillis:

if (currentMillis - previousMillis >= interval) {
  previousMillis = currentMillis;
  // ...
}

This is prone to accumulating errors. The test will most likely not happen exactly at previousMillis + interval, mostly because of the serial port. You are a little bit late and, by updating previousMillis this way, these errors are cumulative. You can avoid this by updating previousMillis as:

previousMillis += interval;

You will still have timing errors, but these will manifest themselves in the form of jitter, not as a systematic timing drift.

Interestingly, you are updating lastInstructionMillis the right way!