PID function about automationshield

[gergelytakacs]

• I don't see any reason why PID needs its own class, do you @Gabor7697 ? Why can't it be AutomationShield.PID() ??? Remember, you are writing something fairly universal not just a will do for now improvised thing.
• I don't really understand the logic behind your new class with its own comp method etc. Very-very confusing, obscure and not sure about your motivation.
• Ts should be considered as a globally accessible constant, defined somewhere outisde of PID. Just initialize it right at the beginning in the header and assume it is there long int Ts. PID should just assume it is available, or if not, default to 1 or throw an error. There is no good reason to have it entered inside the PID method again.
• What is a "reverse acting system"? Don't you just mean the sense/sign of the feedback loop? But that is just saying u=-u...
• If something you want to do does not work because of e.g. scope issues, re-think your approach and study up how it's done.
• Don't really understand at the first look what's different between the two versions, you really need to comment your code.

[Gabor7697]

pid1() is based on @mgulan s difference equation method.

[gergelytakacs]

Thanks for your contribution and greetings from the train...:) Here is my take:

• I think it should-eventually-be both incremental and absolute formulation. For now any one of them is fine, have to start somewhere.
• Anti-wind up should be part of the PID method as it concerns the internal variables of the PID algorithm. However, it is worth cosidering using a stand-alone method for it, which could aid maintainability of the code.
• Do not forget, this is mainly an educational tool, so we don't want to make it too self-closed and possibly complex. So, imho pre-filtering and saturation should be outside to increase didactic value (and improve code maintainability).
• The method should take, in the most basic version, current error (past should be handled internally), tuning parameters and return input.
• For educational value, tuning should be in the original form (prop., Ti, Td) and the re-calculation to q0,q1 etc shall be internally. This, of course, wastes resources but it is better educationally.

[mgulan]

OK, I agree:-)
To correct myself, the form I sent is in fact the absolute one, yet can be easily used as incremental when only the input increment is used.

[gergelytakacs]

Haven't seen the picture on my cell, yes - this is the one. Absolute is a bit more intuitive, incremental could be an alternate option...

[Gabor7697]

d0948aa

[gergelytakacs]

Ok, I see! In that case it needs to be properly tetsed and thought out how it will be included in the library.

[kulho]

I've been trying to commit my code for PID for at least two hours, but I cannot seem to figure out what's wrong. Do I need a permission to do that or I am still doing something wrong?

[gergelytakacs]

@kulho You haven't accepted the invite yet. Sent a mail about it.

[gergelytakacs]

@Gabor7697 deleted your new PID branch, because I merged to master anyways. The "old" PID is there only until tomorrow, until we discuss how to continue. Also I want to review @kulho changes and show how it is not a good idea to to rewrite someone's code;)

[gergelytakacs]

• Unify coding style, variable names etc. between all the PID versions. The PID functions should not look entirely differently and should be readable. Remember, this is a didactic tool.
• @rkoplinger and @Gabor7697 should agree on unifying the two PID versions (the absolute ones).
• Which style of tuning to use and why? You should look up which one is prefferred, eg. in Matlab PID tuning functionality, Simulink blocks etc. and use that. Should it be Kp, Ki, Kd or Kp, Ti, Td? Why one or theo other?
• documentation in the WiKi
• think about improving integral windup

-~~~leave pid alone and/or combine it with @rkoplinger version~~~
-~~~rename pid1 to pidInc (any better ideas?)~~~
-~~~direction is not needed~~~
-~~~integral widndup for the absolute pid~~~

[gergelytakacs]

Just a quick idea: pick a way to input tuning to PID and stick with it. But then, you could do a function that translates between the two...

[gergelytakacs]

There has been lots of improvement from the original PID function and the redesign helped a lot as well, see #46