Found a bug in the servo library

I found a bug in the Servo library in 1.0 (and it’s probably been there for a while):

In library/Servo/Servo.h:

 90   else {
 91     // finished all channels so wait for the refresh period to expire before starting over 
 92     if( (unsigned)*TCNTn <  (usToTicks(REFRESH_INTERVAL) + 4) )  // allow a few ticks to ensure the next OCR1A not missed
 93       *OCRnA = (unsigned int)usToTicks(REFRESH_INTERVAL);
 94     else
 95       *OCRnA = *TCNTn + 4;  // at least REFRESH_INTERVAL has elapsed
 96     Channel[timer] = -1; // this will get incremented at the end of the refresh period to start again at the first channel
 97   }

The “+4” on line 92 should be on the other side of the inequality, OR it should be a “-4” instead. As it is, there is a 4-8 microsecond interval during which the logic in the library will set OCR1A to a value that it has just missed, and it has to go all the way around to pick it up.
The comment says “allow a few ticks” but it actually dis-allows those ticks.
Think of the logic: If TCNT1 is the refresh interval plus one (clearly, already too late,) then this if statement will still allow the code to set OCR1A to the refresh interval.

So, if your servos sometimes twitch, or sometimes take longer to respond than they should (but only when you use a few, with some particular set of control values,) then this would be the reason.

As I do not know the details of the servo lib I cannot confirm this bug but your text seems to make sense.

I can confirm this code exists in 0.22 in Servo.cpp (not in .h) so it is indeed there for some time.

You can report this bug @ - http://code.google.com/p/arduino/issues/list -

Do you have a fix for the code please post it here too.

The "+4" on line 92 should be on the other side of the inequality, OR it should be a "-4" instead.

Should the +4 in line 95 be changed too, or is this coincidentaly the same magic number?

robtillaart:

The "+4" on line 92 should be on the other side of the inequality, OR it should be a "-4" instead.

Should the +4 in line 95 be changed too, or is this coincidentaly the same magic number?

It is the same magic number, but it should stay the way it is. The if() statement is supposed to compensate for this magic number, but "compensates" the wrong way.

Also, thanks for the bug reporter link; I filed the issue there.

did you report the bug allready? (and the fix)