moveForward() broken in library on GitHub

In this thread…

[url]Can't handle negative turns?]

there was a discussion about allowing negative values as arguments to functions like moveForward(). I think this feature makes a lot of sense, and I see that a change has been made in the Sparki repository on GitHub. However, the change is broken.

Here, for example…

void SparkiClass::moveForward(float cm) { float run = 222.222222*cm; if(cm == 0){ moveBackward(); } else{ if(cm < 0){ moveForward(cm); } else{ moveBackward(); delay(long(run)); moveStop(); } } }

…a negative argument will cause an infinite loop. The “if(cm<0)” calls moveForward() again with the same cm value, which will result in infinite recursion (until the function call stack overflows), and Sparki will hang. And there are other bugs in this function; moveForward and moveBackward seem to be somewhat mixed up.

I think it was supposed to be this:

void SparkiClass::moveBackward(float cm) { float run = 222.222222*cm; if(cm == 0){ moveBackward(); } else{ if(cm < 0){ moveForward(-cm); } else{ moveBackward(); delay(long(run)); moveStop(); } } }

Note that the problem is not unique to moveForward(); this one was just my example.

Please don’t release the library like this. It already has plenty of other bugs. :frowning:

I actually forked the Sparki repository on GitHub and started rewriting the motor control code. I shouldn’t have to do this, but my confidence in the library is pretty low at this point. Can somebody make me feel better?

Github code isn’t release code. We don’t necessarily test it before putting it on Github.

I understand that the latest on GitHub isn’t necessarily release material. That’s why I wrote “Please don’t release the library like this”. Still I would expect at least a basic level of screening that would prevent bugs like this from sneaking into a commit.

In any case, my intent was to flag the error so that it would not leak through to a release.