External Interrupt PID Speed Control with Pulse as Setpoint

Hello There!

I am new to Interrupt stuff, and was wondering if anyone could look on my code, before i add LCD and Stuff.

It is for a Eddy Current Hub Dyno for Cars.
I alreaddy have the ControlSystem for one retarder, so i need one more to operate as a Slave.
with PID Setpoint from the other one.

/*Slave PID with Setpoint from Master.
 Eddy Current Brake Retarders connected to Driveshaft on Car
 Driverside Retarder gets Speedsettings from other Controller.
 Slave is only following at equal Speed.
 */

#include <PID_v1.h>

const int pulserev = 60;            // the pickup's number of impulses per revolution
const int maxrpm = 4000;          // Max.RPM of the spindle
const int stablerpm = 50;         // Min. RPM for your setup to run stable
int rpmS;                           // measured RPM on Slave
int rpmM;                            // measured RPM on Master
long time = 0;                     // time between two interrupts
long timeOld = 0;                  // absolute time of last interrupt
float frequency;                   // frequency of the interrupts in Hz

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

double Kp = 0.15, Ki = 0.1, Kd = 0.005;

//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint, Kp, Ki, Kd, REVERSE);

void setup()
{
  // Sample Time of 5 ms
  myPID.SetSampleTime(5);

  // Interrupt on Pin 3
  attachInterrupt(1, measurerpmS, RISING);

  // Interupt on Pin 2
  attachInterrupt(0, measurerpmM, RISING);

  // turn the PID on
  myPID.SetMode(AUTOMATIC);
}

void loop()
{
  Setpoint = rpmM;

  rpmM = 60 * (frequency / pulserev);                     //Calculating the RPM of Master

  rpmS = 60 * (frequency / pulserev);                    // Calculating the RPM of SLAVE
  
  Input = rpmM;                                          // Input variable for PID control

  myPID.SetTunings(Kp, Ki, Kd);

  myPID.Compute();

  if(Setpoint <= stablerpm){                            // No PWM output on Pin 9 if the desired RPM isn't running stable
    analogWrite(9, 0);
  }
  
  else{
  analogWrite(9, Output);                               // PWM Output to motorcontroller on Pin 9
  } 
}
void measurerpmS()                                       // Interrupt for measuring the frequency on SLAVE
{
  time = micros() - timeOld;
  timeOld = micros();
  frequency = time;
  frequency = 1000000 / frequency;
}
void measurerpmM()                                       // Interrupt for measuring the frequency on Master
{
  time = micros() - timeOld;
  timeOld = micros();
  frequency = time;
  frequency = 1000000 / frequency;
}
  rpmM = 60 * (frequency / pulserev);                     //Calculating the RPM of Master

  rpmS = 60 * (frequency / pulserev);                    // Calculating the RPM of SLAVE

How are the variables frequency and pulserev and the constant 60 related to the master or the slave? It looks to me like rpmM and rpmS will always be exactly the same value.

  // Interrupt on Pin 3
  attachInterrupt(1, measurerpmS, RISING);

  // Interupt on Pin 2
  attachInterrupt(0, measurerpmM, RISING);

Maybe this is the wrong way to do it?

Maybe this is the wrong way to do it?

Nothing wrong with that snippet. What happens IN the ISRs is wrong, though. You use frequency and pulsrev in both routines, assuming that somehow, magically, the Arduino will have separate memory locations for the master data and the slave data. That is not a realistic assumption.

I thought there was something wrong, but could not put my finger on it!

Do you by any chance have i nice trick to fix it?

Is it that simple, that i have to make:

float frequencyM
float frequencyS

Pulserev is the same on both

Regards

Larsperformance:
Is it that simple, that i have to make:

float frequencyM
float frequencyS

Pulserev is the same on both

Regards

Yes, you need two different variables to hold the two different frequencies. You also need two different variables to hold the two different times that the interrupt handlers get triggered.

By the way, variables, like frequency, that are used in the ISRs and in other functions, like loop(), need to be declared volatile.

Variables that are used in only one function, like timeOld, should NOT be global. They should be local to the function that they are used in. If the reason form making them global is to make them retain values between function calls, making them global was the wrong approach. They should be local AND static, so that they retain there values between calls to the function.

Hmm!

Probably have to read up some more!

That did not make much sense to me!

Regards