br3ttb / Arduino-PID-AutoTune-Library

420 stars 225 forks source link

New tuning rules, peak detection method, etc. #11

Open t0mpr1c3 opened 11 years ago

t0mpr1c3 commented 11 years ago

Brett, there's a lot here so if you want to pull in just a specific feature let me know and I can create a new fork with just that change that you can pull from. The peak detection I think is an improvement because it uses information from both minima and maxima, instead of maxima only, so it potentially converges half a cycle sooner as well as being more accurate if the minima are variable. Lots of variations on Ziegler-Nichols. Slightly different but still a relay tuning method is AMIGOf, a robust algorithm for tuning PI controllers, although the tuning is slower to complete than other methods. Relay bias is useful sometimes so I have put it as a compile option.

jhagberg commented 10 years ago

Hej nice commit.

I need to change the controller value to this to get it to compile. aTune.SetControlType(PID_ATune::AMIGOF_PI);

Is it just typo?

How long will a run of AMIGOF_PI take?

t0mpr1c3 commented 10 years ago

Thanks, can you do a pull request? I'm not exactly sure what the issue is because the firmware compiled OK for me without this.

AMIGOF takes longer than the default Ziegler Nichols because there is an additional step to estimate K_process which can take a while to converge under some conditions. But in my experience the results are far more robust (i.e. remain close to optimal when the process parameters change slightly).

jhagberg commented 10 years ago

Sure I can! I am testing all of the autotunes This far I can get ZIEGLER_NICHOLS_PI and ZIEGLER_NICHOLS_PID to work Also TYREUS_LUYBEN_PI starts and finds max and min Now running CIANCONE_MARLIN_PID Also seems to work good and find max and mins.

I first tried aTune.SetControlType(PID_ATune::AMIGOF_PI); Had it running all night but nothing really happens. All the other methods Switch the output up and down. But with AMIGOF nothing really happens and output is constant.

Am I doing something wrong or am I missing something? I have cloned your latest git repo!

t0mpr1c3 commented 10 years ago

AMIGOF has an extra stage and under certain conditions it fails to converge. (Usually very sluggish processes, in which case Tyreus Luyben PI is probably a good starting point.) You can experiment with the simulations to see which is likely to work best for your process.

jhagberg commented 10 years ago

Oki but you are supposed to see output changes in the same way as the other? On the serial monitor?

jhagberg commented 10 years ago

Aha so first you want to hold the process steady at baseline then steady after step up. I turned out debug and it made sense!

sylvia43 commented 10 years ago

Can this be merged then?

jhagberg commented 8 years ago

I guess that is up to @br3ttb

rusefi commented 5 years ago

Just wanted to say "Great work @t0mpr1c3 "

edgsc commented 5 years ago

Great work, @t0mpr1c3 !

But I'm having problems to compile it on newer Arduino IDE (1.6.x).

`C:\Users\Acer\Documents\Arduino\libraries\PID_AutoTune_v0\PID_AutoTune_v0.cpp:20:58: error: variable 'tuningRule' must be const in order to be put into read-only section by means of 'attribute((progmem))'

PROGMEM Tuning tuningRule[PID_ATune::NO_OVERSHOOT_PID + 1] = ^`

I tried searching for this error message and I think it's because from IDE 1.0.x to 1.6.x they changed how to use PROGMEM; the code should be adapted (I tried but can't get it to work): https://github.com/arduino/Arduino/wiki/1.6-Frequently-Asked-Questions#errors-related-to-avr-progmem-changes

Errors related to AVR PROGMEM changes Q: I get an error message that says: variable 'message' must be const in order to be put into read-only section by means of 'attribute((progmem))': char message[] PROGMEM = "Hello!";. What can I do? A: The latest avr-gcc compiler (used by Arduino 1.6.x) requires that variables in PROGMEM to be declared as const.

First of all check if an updated version of the libraries used in the sketch has been released. Upgrading libraries is far the best way to solve the problem.

If there are no alternatives you can go to the hard path and change the declaration in your sketch or in your library from

char message[] PROGMEM = "Hello!";

to

const char message[] PROGMEM = "Hello!";

Q: I have a similar error but it seems to be already const! Here's the error: variable 'lotOfMessages' must be const in order to be put into read-only section by means of 'attribute((progmem))': const char* lotOfMessages[] PROGMEM = { A: In this case you have an array of messages (not just one) and you must tell the compiler that each one of them is const. Here how is it done, change:

const char* lotOfMessages[] PROGMEM = {

to

const char * const lotOfMessages[] PROGMEM = {

And, yes, it's really tricky even for experts.

Anyone having the same problem? How to get it to work?

And congratulations for the great work again!

edgsc commented 5 years ago

Great work, @t0mpr1c3 !

But I'm having problems to compile it on newer Arduino IDE (1.6.x).

`C:\Users\Acer\Documents\Arduino\libraries\PID_AutoTune_v0\PID_AutoTune_v0.cpp:20:58: error: variable 'tuningRule' must be const in order to be put into read-only section by means of 'attribute((progmem))'

PROGMEM Tuning tuningRule[PID_ATune::NO_OVERSHOOT_PID + 1] = ^`

I tried searching for this error message and I think it's because from IDE 1.0.x to 1.6.x they changed how to use PROGMEM; the code should be adapted (I tried but can't get it to work): https://github.com/arduino/Arduino/wiki/1.6-Frequently-Asked-Questions#errors-related-to-avr-progmem-changes

Errors related to AVR PROGMEM changes Q: I get an error message that says: variable 'message' must be const in order to be put into read-only section by means of 'attribute((progmem))': char message[] PROGMEM = "Hello!";. What can I do? A: The latest avr-gcc compiler (used by Arduino 1.6.x) requires that variables in PROGMEM to be declared as const.

First of all check if an updated version of the libraries used in the sketch has been released. Upgrading libraries is far the best way to solve the problem.

If there are no alternatives you can go to the hard path and change the declaration in your sketch or in your library from

char message[] PROGMEM = "Hello!";

to

const char message[] PROGMEM = "Hello!";

Q: I have a similar error but it seems to be already const! Here's the error: variable 'lotOfMessages' must be const in order to be put into read-only section by means of 'attribute((progmem))': const char* lotOfMessages[] PROGMEM = { A: In this case you have an array of messages (not just one) and you must tell the compiler that each one of them is const. Here how is it done, change:

const char* lotOfMessages[] PROGMEM = {

to

const char * const lotOfMessages[] PROGMEM = {

And, yes, it's really tricky even for experts.

Anyone having the same problem? How to get it to work?

And congratulations for the great work again!

Got it to work simply by adding "const" before PROGMEM: PROGMEM Tuning tuningRule[PID_ATune::NO_OVERSHOOT_PID + 1] = changed to

const PROGMEM Tuning tuningRule[PID_ATune::NO_OVERSHOOT_PID + 1] =

skaterdude commented 4 years ago

Hi, just tried to compile this in the latest Arduino IDE and got errors resulting from: static const double CONST_PI = 3.14159265358979323846; static const double CONST_SQRT2_DIV_2 = 0.70710678118654752440; in the .h file. From what I read, it appears that the compiler changed and now flags anything that is not an integer when you declare it this way. Supposedly this is because it is non-standard to the compiler spec. All you have to do is change const to constexpr like so: static constexpr double CONST_PI = 3.14159265358979323846; static constexpr double CONST_SQRT2_DIV_2 = 0.70710678118654752440; and I get a clean compile. I'm using Arduino 1.8.12