Pointer Help in PID Library

Hi all!
I have been trying to build my own PID library to control a RC airplane. After troubleshooting for a little bit I simplified my code so I could magnify my issue.
Essentially, I want to set up pointers in the class constructor so I do not have to constantly feed new values to different functions within the class.
My .h looks like this:

#ifndef PID_h
#define PID_h

class PID {
 public:
        PID(double);
        void calculate();
        double var;
 private
        double *pointer;
};

#endif

//My .cpp looks like this:

#include "Arduino.h"
#include "PID.h"

PID::PID(double input){
pointer = &input;
}

void PID::calculate(){
var = *pointer;
}

//And finally my arduino code:

#include <PID.h>
double pitch;
PID pid(pitch);

void loop (){
  accel();
  convert();
  pid.calculate();
  Serial.print(pitch);
  Serial.print("  ");
  Serial.println(pid.var);
}

When the PID instance "pid" is created in the beginning at global scope, it should pass "pitch" to the header file. Within the constructor the private double "*pointer" should store the location of "pitch".

The function accel() grabs accelerometer values over the i2c bus. The convert () function takes the accelerometer value and puts into degress, this is stored the double "pitch." This works just fine.

Once the arduino calls "pid.calculate ()" the public double "var" should be redirected and then store the value "pitch"

The serial bus should then print the value pitch, and the value pid.var. These should be the same.
The problem is: the value pitch is live accelerometer data, but the "pid.var" only ever reads "-0.00" instead of the value pitch.

Any help on this one guys? I have been working on this one for a while but I cannot see what I am missing....

Read this before posting a programming question

Code tags please.

I didn't think one could have a arduino sketch compile without errors when missing a void setup() function?

Lefty

Sorry, I just didn't include it in my code. It just contains a couple i2c addresses

void setup()
{
   Serial.begin(9600);
   Wire.begin();

   WriteByte(accelAddr, REG_BW_RATE_ADDR, 0x08);
   WriteByte(accelAddr, REG_PWR_CTL_ADDR, 0x08);
   WriteByte(accelAddr, REG_INT_ENABLE_ADDR, 0x0);
}

Why pointers exactly? Why not just save the actual value?

Well I want the PID to refresh as quickly as possible.
and it seems preferable to configure the memory locations once rather than constantly passing values back and forth between the library and the arduino code. I would like to have multiple instances controlling different axes on the airplane (pitch and roll)

I can get something like this working (still very lean, but just for example) where we pass all values through the pid.calculate() function and then return the servo output. This seems to work OK... but I don't understand why my pointer didn't work

Header File:

#ifndef PID_h
#define PID_h

class PID {
 public:
        PID();
        double calculate(double,double,double,double,double);
        double p,i,d;
        double error, lastError, now, last, dt;
};

#endif

C ++:

#include "Arduino.h"
#include "PID.h"

PID::PID(){}

double PID::calculate(double input,double setpoint, double kp,double ki, double kd)
{
 now = millis();
 error= input-setpoint;
 dt = now - last;
        p = error*kp;
        i = (error*ki*dt)+i;
        d = (error - lastError)*kd/dt;

        double output = p+i+d;

 lastError = error;
 last = now;

return output;

}

Arduino

double p,r, pitch;
double setpoint = 0, kp =1, ki = 1, kd= 1;
  PID pid;

void setup()
{
   Serial.begin(9600);
   Wire.begin();

   WriteByte(accelAddr, REG_BW_RATE_ADDR, 0x08);
   WriteByte(accelAddr, REG_PWR_CTL_ADDR, 0x08);
   WriteByte(accelAddr, REG_INT_ENABLE_ADDR, 0x0);
}

void loop (){
  accel();
  gyro();
  convert();
  double a = pid.calculate(pitch,setpoint,kp,ki,kd);
  Serial.println(a);
}

It seems tidier to pass memory locations on the creation of the instance so that setting can be changed on the fly
(ie setpoints, pid constants etc)

Not sure I agree with that. Passing doubles around wouldn't take much more effort than passing pointers around. Anyway, I see one issue:

PID::PID(double input){
pointer = &input;
}

You are passing a pointer to the argument, which is on the stack and then ceases to exist (goes out of scope). If you must pass pointers you need to pass a pointer to some static variable, eg.

PID::PID(double * input){
pointer = input;
}

I would prefer to keep the data as a double in the class and have some sort of "update" function if you need to change it.

Bear in mind BTW that the Arduino does not really support doubles. They are internally the same as float.

Ok, so I think I want to set it up so that the only pointer used is to update the output variable for that specific PID instance. Now my header file looks like this:

#ifndef PID_h
#define PID_h
class PID {
 public:
        PID(double);
        double now;
        bool calculate();
        double kp, ki, kd;
        void setKonstants (double, double, double);

 private:
        double *out;

};
#endif

my .cpp

#include "Arduino.h"
#include "PID.h"
PID::PID( double * output)
{
 output = out;
}

bool PID::calculate(){
 now = *out;
}

void PID:: setKonstants(double _kp, double _ki, double _kd){
 kp = _kp;
 ki = _ki;
 kd = _kd;
}

and my arduino reads:

long now, last, dt;
double p,r, pitch;
double setpoint = 0, kp =1, ki = 1, kd= 1;
PID PID(p);
  

void setup()
{
   PID.setKonstants(kp,ki,kd);
   Serial.begin(9600);
   Wire.begin();

   WriteByte(accelAddr, REG_BW_RATE_ADDR, 0x08);
   WriteByte(accelAddr, REG_PWR_CTL_ADDR, 0x08);
   WriteByte(accelAddr, REG_INT_ENABLE_ADDR, 0x0);
}

void loop (){

  last = millis();
  accel();
  gyro();
  convert();
  now = millis();
  dt = now-last;
  Serial.print(PID.now);
  Serial.print("  ");
  Serial.println(dt);
}

I am just timing the main loop to see how long different implementations will take.
However, setting up the pointer like this I get the following error:

/usr/share/arduino/libraries/PID/PID.cpp:3:1: error: prototype for ‘PID::PID(double*)’ does not match any in class ‘PID’
/usr/share/arduino/libraries/PID/PID.h:3:11: error: candidates are: PID::PID(const PID&)
/usr/share/arduino/libraries/PID/PID.h:5:9: error:                 PID::PID(double)

So the way I set it up before: the variable was deleted before it could call the calculate () function, so the pointer did not point to anything.
Now we are trying to set something static? I did a little reading about passing pointers through arguments, and I only found a small article about dynamic memory? I tried writing the header:

class PID {
 public:
        PID(double*);
        double now;
        bool calculate();
        double kp, ki, kd;
        void setKonstants (double, double, double);

 private:
        double *out;

};
#endif

but that didn't work either.
Anyway, I suspect that I am not setting up the header to match with my new .cpp...

You REALLY need to explain how you think updating a variable through a pointer is going to be faster/easier/better than updating it directly.

PID::PID( double * output)
{
 output = out;
}

The out variable doesn't point to anything. Making the pointer used to construct the instance point to crap, too, seems useless.

I don't know if you really need to use pointers for this either, but see if this helps. You have a couple of your assignments backwards. Warning I haven't compiled this...

PID::PID( double * output)
{
 out=output;
 now=1;
}

bool PID::calculate(){
 now *= kp * ki * kd;  // some complicated formula
 *out = now;  // send it to your external variable too
}

void PID:: setKonstants(double _kp, double _ki, double _kd){
 kp = _kp;
 ki = _ki;
 kd = _kd;
}
...

double outvar;
PID pid( &outvar );
pid.setKonstants( 2,3,7);
pid.calculate();
Serial.print(output);   // the answer is: 42

I still think the whole approach is, well, silly. That is because you are breaking the whole encapsulation thing that classes are supposed to give you. Having it rely on some variable which it only has the pointer to is kind-of weird. Plus, the savings, compared to the rather lengthy times needed to do float arithmetic (which is not hardware supported) will be very very marginal.

However ...

double p,r, pitch;
double setpoint = 0, kp =1, ki = 1, kd= 1;
PID PID(p);

You have changed it to expect a pointer, so supply one:

double p,r, pitch;
double setpoint = 0, kp =1, ki = 1, kd= 1;
PID PID(&p);