Wrong sign on the derivative term.

[mike-matera]

Hello! While validating my own PID controller I've been testing against other PIDs available for Arduino. Here's a plot of AutoPID vs. a reference that matches the excellent work done here:

https://github.com/br3ttb/Arduino-PID-Library

The difference is that your D-term has the wrong sign:

https://github.com/r-downing/AutoPID/blob/master/AutoPID.cpp#L62

Compare that to Brett's ArduinoPID:

https://github.com/br3ttb/Arduino-PID-Library/blob/master/PID_v1.cpp#L83

And my FastPID:

https://github.com/mike-matera/FastPID/blob/master/src/FastPID.cpp#L96

[hitech95]

Are you sure about this? I just cheked and the formula looks fine to me but i'm new on PIDs I was just looking for a implementation for a project!

[mike-matera]

Hi. Yes. Take a look Brett's PID in the second link. He's a PID expert.

[hitech95]

Uhm... Still cant figure it out. The formula he used looks similar to the one you found on Wikipedia

[hitech95]

To me still looks fine:
Same implementation as this, or I'm missing something really simple!
https://gist.github.com/bradley219/5373998#file-pid-cpp-L72

May it be a different formula?
The derivate term is:
derivate = (current_error - prev_error) / deta_time

So I don't see where the problem is :(

I only want to learn, i'm new on PIDs!

[hitech95]

Also on your implementation I see the same formula (You only compensate for the delta_set_point:
https://github.com/mike-matera/FastPID/blob/bf3e1f8faf0353490ba017fa26bc77a66534364a/src/FastPID.cpp#L96

[genoptic-nic]

The division by 1000 is wrong. It should multiply by 1000. Should be:
(current_error - prev_error) / delta_time
where:
delta_time = millis()/1000.
the result:
(current_error - prev_error) / millis() *1000

[hitech95]

I'm definitly dumb, still cant see the issue.
delta_time is in seconds millis() return in milli seconds, so it must be converted back.

But @mike-matera was talink about a wrong sign. So no idea.

[genoptic-nic]

Just do the equation yourself with some numbers. Say:

• current error = 2
• previous error = 3
• return value of the millis function = 1000 (1 second).
• The derivative here is obviously -1.

Now look at the code line in question:
The present code:
(current_error - prev_error) / millis() / 1000
This would give an output of -1/1000000. Which would be the incorrect answer.
My proposed change
(current_error - prev_error) / millis() * 1000 = -1
This is the correct answer.

There is nothing wrong with the sign. The division should be a multiplication.

[Ho-Ro]

see also #11

[mike-matera]

It looks like the D term basically doesn't work. Here's a plot of AutoPID with varying Ds. Notice that all of the lines are the same.