Timestamp_prev data type clarifications

Hi guys,
For several days I had been experiencing an annoying problem: after about 20 minutes of “movements” of the motor, suddenly the shaft began to vibrate uncontrollably. I was forced to restart the ESP32. After several tests, I thought it was related to some overflow, like micros(). I’m not much of an expert, but I noticed that in PID.h, timestamp_prev is a float maybe because it is intended in seconds and not in microseconds. But there is a first problem:
In PIDController::PIDController (in PID.cpp), timestamp_prev = _micros()*1e-6 so in seconds.
In PIDController::operator, Ts = (timestamp_now - timestamp_prev) * 1e-6 where timestamp_now is an unsigned long expressed in microseconds, while timestamp_prev is a floating point expressed in seconds.
I think that this is not a great problem because this problem occurs only on the first iteration when PIDController is called.
About the sudden vibration of the motor, can you confirm that this strange behavior is due to the fact that timestamp_now is an unsigned long and timestamp_prev is a float?



P.S.: I noticed that in the library there is a quick fix for the micros overflow (if(Ts <= 0 || Ts > 0.5) Ts = 1e-3 ) and so, for debug, I added a monitor print to better understand what was happening: the result is that when the motor starts vibrating, I am constantly staying within that if loop.

Interesting, I’ll look into it.
But this is definitely an error in the code.

You can definitely rewrite the (line 13 in pid.c):

timestamp_prev = _micros();

and define timestamp_prev(line 36 in pid.h) as

float timestamp_prev; //!< Last execution timestamp

Would you care to do a quick github issue concerning this so I don’t forget :smiley:

Or even better if this chnage works for you please make a pull request and we will directly integrate your code into the library! :upside_down_face: