Control speed of motor and measure speed from a brake disc

Hello,

This is a continuation of a previous project (Measure speed of brake disc with optical sensor) which worked in the end but, I'm now facing another issue and I need your help.

I was able to measure the speed thanks to the help of all you contributors but when adding the code to control the motor I made slight (I believe necessary) adjustments. But when I measure my speed now it doesn't work anymore..

#define SO 2 //Optical sensor
#define motor 4
#define HT 3 // Clock

float vitesse = 0.0 ; //speed in french

volatile unsigned long T1;
volatile unsigned long T2;
unsigned long T2_c;
unsigned long T1_c;
unsigned long dT_op;

volatile int b=0;

volatile float target;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  
  pinMode(HT, INPUT);
  attachInterrupt(1, Clock, RISING);
  pinMode(SO, INPUT);
  attachInterrupt(0, readSO, RISING);
  pinMode(motor,OUTPUT);

  target = 20.00;
}


void  Clock(){
  b = b+1;
}

void readSO(){
  T1=T2;
  T2=micros();

}

void loop() {
  // put your main code here, to run repeatedly:
  noInterrupts();
  T2_c = T2;
  T1_c = T1;
  interrupts();
  dT_op = T2_c-T1_c;
  vitesse = (1e6)/ (16*dT_op);
  
  if (b>2){
    Serial.print("vitesse ");
    Serial.print(vitesse);
    Serial.print("  dT  ");
    Serial.println(dT_op);
    b=0;
  }
  
  //Constants PID
  float kp=10;
  
  //Error
  float e = target-vitesse;
  if (e<0){
    e=0;
  }
  
  //Corrector
  float u = kp*e;

  //Motor power 
  float pwr=fabs(u);
  if( pwr > 255 ){
    pwr = 255;
  }
   
  //signal the motor
  analogWrite(motor,pwr);
}

Yes the target is not hit but that's just due to PID constants (no?)

I believe there's a issue with the measure of dT but I can't figure what's the problem, I'm quite novice..
I would really love any help !

What Arduino are you using, and is pin 4 a pwm pin?

The variable b used in ISR Clock() is at risk. You shouldn't read the 2 byte int b without blocking the interrupt. The same goes for assigning an int incremented in an interrupt.

  if (b > 2) {
    Serial.print("vitesse ");
    Serial.print(vitesse);
    Serial.print("  dT  ");
    Serial.println(dT_op);
    b = 0;

I suggest:

  nointerrupts();
  int TmpB = B;
   interrupts();
  if (TmpB > 2) {
    Serial.print("vitesse ");
    Serial.print(vitesse);
    Serial.print("  dT  ");
    Serial.println(dT_op);
    nointerrupts();
    b = 0;
    interrupts();

I've no idea how much b is incremented.....

Oops.... analogWrite uses ant int, not a float variable..... Change that!

  //Constants PID
  float kp = 10;

  //Error
  float e = target - vitesse;
  if (e < 0) {
    e = 0;
  }

  //Corrector
  float u = kp * e;

  //Motor power
  float pwr = fabs(u);
  if ( pwr > 255 ) {
    pwr = 255;
  }

  //signal the motor
  analogWrite(motor, pwr);

This just can't be right. You use the error to produce an absolute pwm. I would like You to use the previous pwm value and the error value in some way.

The variable b used in ISR Clock() is at risk.

@gauthierhk would be better to use a byte for b and not have to deal with protected access.

From the previous posting, I believe it is counting a 100ms clock tick.

I might be wrong but I remember some knowing helper once told that byte is converted to/ used like, ant int.

You don't practise a real PID. PID adds the feedback to the basic, P factor calculated value. Combining the error signal with kp doesn't make it PID.

I'm using an Arduino Mega so pins 2-13 are pwn pins if I'm correct. The frequency on the 4th is different from others. Do I need to take that into account?

Yep I did it for the dT but not the b, missed it...

Okay will do thanks a lot for your help !!

So I implemented your advices and it seems to work!

#define SO 2 //Optical sensor
#define motor 5
#define HT 3 // Clock

float vitesse = 0.0 ; //speed in french

volatile unsigned long T1;
volatile unsigned long T2;
unsigned long T2_c;
unsigned long T1_c;
unsigned long dT_op;

volatile byte b=0;
volatile int b_c=0;

volatile float target;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  
  pinMode(HT, INPUT);
  attachInterrupt(1, Clock, RISING);
  pinMode(SO, INPUT);
  attachInterrupt(0, readSO, RISING);
  pinMode(motor,OUTPUT);

  target = 50.00;
}


void  Clock(){
  b = b+1;
}

void readSO(){
  T1=T2;
  T2=micros();

}

void loop() {
  // put your main code here, to run repeatedly:
  noInterrupts();
  T2_c = T2;
  T1_c = T1;
  b_c = b;
  interrupts();
  dT_op = T2_c-T1_c;
  vitesse = (1e6)/ (16*dT_op);

  if (b_c>2){
    Serial.print("vitesse ");
    Serial.println(vitesse);
    noInterrupts();
    b=0;
    interrupts();
    b_c=0;
  }
  
  //Constants PID
  float kp=100;
  
  //Error
  float e = target-vitesse;
  
  //Corrector
  float u = kp*e;
  if (u<0){
    u=0;
  }

  //Motor power 
  int pwr= (int) fabs(u);
  if( pwr > 255 ){
    pwr = 255;
  }
   
  //signal the motor
  analogWrite(motor,pwr);
}

I still get some weird values, are there any possible issue with the code still or it's just vibrations that might cause issue?

Thanks again both of you for your help, I just saved days of troubleshooting !