I've written a function to run data through a PID loop to generate an output as below

#include <Arduino.h>
unsigned long start = 0;
unsigned long current = 0;
unsigned long iterationtime = 0;
long output (int setpoint, int Input, int Kp, int Ki, int Kd, int delaytime) {
float integral_prior = 0;
float error_prior = 0;
start = millis();
float error = setpoint - Input;
float integral = (integral_prior + error)* iterationtime;
float derivative = (error - error_prior)/iterationtime;
long output = (Kp * error) + (Ki * integral) + (Kd * derivative);
error_prior = error;
integral_prior = integral;
delay(delaytime);
current = millis();
iterationtime = current - start;
return output;
}

I'm aware the millis() timing is almost useless...but I wrote it so it shall remain...

The next step is converting the Output into useful data, so essentially mapping it to the range with my servo operates (Which is 20 degrees in either direction in my project). The way I plan to do this is as follows

int MaxOutput;
long OutputX = map(output(Setpoint, Xax, Kp, Ki, Kd, 10), 0, MaxOutput, -20, 20);
long OutputXMap = OutputX +20;

The problem is I don't know the max output. I guess I could create another function to calculate the max possible output but the slight problem with that is that it seems like an awful lot of work... and I can't be bothered.....

Is there an easy way to map the values without knowing the current range or an easy way to calculate the current range? Is everything about this horribly wrong and incompetently written?

To do the same thing you could use the constrain() function (instead of map()):

long OutputX = constrain(output(Setpoint, Xax, Kp, Ki, Kd, 10), -20, 20);
Note: the map() function does NOT constrain the input or output values. Passing in a value outside the input range will result in an output value outside the output range.

The problem is I don't know the max output. I guess I could create another function to calculate the max possible output but the slight problem with that is that it seems like an awful lot of work... and I can't be bothered.....

Not as much of a problem, as it is for us, since we know even less than you do about your system. It sounds like you need to determine some loop limits and gain. This is not really a coding problem, it's a design problem so you will have to be bothered.

johnwasser:
Warning: If you don't mark these as 'static' they get set to 0 each time the function runs.

float integral_prior = 0;

float error_prior = 0;

Thanks for the suggestion! I tried changing them to static. For some reason its made it output -20 continuously even when there is no error.... Do you think this could be indicative of something being done wrong? An elaboration on my setup is that I running this in VSCode and have added this as a library then called the function in the void loop(). Not sure why I've added it as a library, just getting to grips with VSCode and the under the bonnet side of things too so thought I'd just try writing a library. It also makes it a bit more organised I guess.

Can the library carry and store its own running values when it's in a loop? If I create the variables globally would they store the previous data each time the function runs again because that variable isn't being initialised to 0 again?

johnwasser:
To do the same thing you could use the constrain() function (instead of map()):

long OutputX = constrain(output(Setpoint, Xax, Kp, Ki, Kd, 10), -20, 20);

Note: the map() function does NOT constrain the input or output values. Passing in a value outside the input range will result in an output value outside the output range.

Thanks again, I've gone back and reverted the library functions and applied this in the main script and it's kept the ouput within +/-90.

aarg:
Not as much of a problem, as it is for us, since we know even less than you do about your system. It sounds like you need to determine some loop limits and gain. This is not really a coding problem, it's a design problem so you will have to be bothered.

Apologies for the vagueness, I thought I wouldn't bore you with the details if they weren't relevant. This is for part of a flight controller for a RC rocket. This control loop will be the way that the vehicle self stabilises. The maximum I can move the motor (which is a ducted fan) within the gimbal is 20 degrees before it hits against the inner fuselage. I think the using the constraint function as John suggested has done the trick on that front.
Just joking about not being bothered, I very much try to work smart instead of hard where possible and I was confident that someone more knowledgable than me would have a better solution than me expending all that brain power on complicated math! And low and behold, a simpler solution has been found.

Managed to come full circle in this project and get stumped on this again. Turns constraining the values doesn't do a great job as the PID have to be smaller than a whole number to return values well within the range. What I notice happening is that it moves in increments much larger than 20 degrees so it tends to just oscillate the servo all the way from 20 to -20 and vice versa. Even removing the 20 degree constraint and while working within the normal parameters of your average hobby servo of 180 degrees (moving from -90 to 90) does not work, again it appears the increments are too big work with within this range.

The solution I'm considering is returning to the idea of mapping which will hopefully return a value appropriately scaled in proportion to the working range which again puts me back at square one of trying to establish the maximum output .... but its dawned on me .... theoretically does a maximum output exist for a PID loop? Surely the loop will keeping increasing the output til the desired output is achieved therefore the output is infinitely large...Or I'm way off and missing something simple.

Anyway, any suggestions on how to remedy the output value into something useful would be greatly appreciated. Do let me know if there is any other details of the project that would be helpful to include.

Oscillation is a sign that your K values are way off, especially Kp.

Start by examining typical error inputs and outputs, and pick a reasonable starting value for Kp, with Kd and Ki = 0. Then tune for optimum performance.

Constraining the output is generally only useful for severe error outliers.

I've just updated QuickPID with a built-in AutoTune function. Included is an example using an RC filter - with this, AutoTune takes about 12-sec to determine and apply Kp, Ki and Kd. Should be available using library manager within a few hours.