Only two PID loop seems to work out of 4

Hi!

I have 4 PID instance in the code, two used to control RPM and two used to control the YAW positions of the propellers.

I have noticed that THe RPM PIDs always function but the YAW PIDs only work sometimes, otherwise it just outputs the Maximum and Minumum values. could you please help determine why this is hapenning.

I have attached the code since it is too large to include here. Also included a snap of the output window.

Combined.ino (12.2 KB)

You need to post your code rather than attach it. Not too many people will download the file.

Having said that, wkat board are you using? From your prodigious use of pis it would appear to be a mega 2560. If this is the case, you need to revisit the board definition. While the mega does have 6 external interrupts, they are attached to pins 2, 3, 18, 19, 20 and 21. If I'm reading your code correctly, you're trying to attach to pins 31, 33, 35 41, 43 and 45 none of which have either an external or pin change interrupt attached to them.

Also included a snap of the output window.

You took a picture of text, and wasted a boatload of bandwidth posting it. Pathetic.

Do ALL those variables in your code NEED to be global?

  while(SerialUSB.available()){
    String incoming = SerialUSB.readString();
    inputSpeed = incoming.toInt();
  }

You got some objection to parseInt() which doesn't waste resources on String objects?

  noInterrupts();
  botMotorSpeed = SpeedInRPMBot*1000;
  topMotorSpeed = SpeedInRPMTop*1000;
  botMotorCount = encoderCountBot;
  topMotorCount = encoderCountTop;    
  interrupts();

How much of that really needs interrupts disabled?

Why are interrupts not disabled when you reset the encounter counts before this?

DKWatson:
You need to post your code rather than attach it.

It’s too big - as identified in the Original Post.

…R

This is a poor way to get serial input in a program that needs to be responsive because it blocks the Arduino from doing other things.

 while(SerialUSB.available()){
    String incoming = SerialUSB.readString();
    inputSpeed = incoming.toInt();
  }

Have a look at the examples in Serial Input Basics - simple reliable non-blocking ways to receive data. There is also a parse example to illustrate how to extract numbers from the received text.

What is the purpose of all the ISRs? I can't figure what they do.

If you get into the habit of putting spaces between "words" in your code it will be a lot more readable. Instead of this

PID botMotorPID(&botMotorPIDInput,&botMotorPIDOutput,&botMotorPIDSetpoint,Kp1,Ki1,Kd1,DIRECT);

try this

PID botMotorPID(&botMotorPIDInput, &botMotorPIDOutput, &botMotorPIDSetpoint, Kp1, Ki1, Kd1, DIRECT);

...R

DKWatson: You need to post your code rather than attach it. Not too many people will download the file.

Having said that, wkat board are you using? From your prodigious use of pis it would appear to be a mega 2560. If this is the case, you need to revisit the board definition. While the mega does have 6 external interrupts, they are attached to pins 2, 3, 18, 19, 20 and 21. If I'm reading your code correctly, you're trying to attach to pins 31, 33, 35 41, 43 and 45 none of which have either an external or pin change interrupt attached to them.

Thank you for your inputs. To answer your questions, I am using an Arduino Due, since any of it's pins can be used for interrupts.

Robin2:
This is a poor way to get serial input in a program that needs to be responsive because it blocks the Arduino from doing other things.

 while(SerialUSB.available()){

String incoming = SerialUSB.readString();
    inputSpeed = incoming.toInt();
  }



Have a look at the examples in [Serial Input Basics](http://forum.arduino.cc/index.php?topic=396450.0) - simple reliable non-blocking ways to receive data. There is also a parse example to illustrate how to extract numbers from the received text.

What is the purpose of all the ISRs? I can't figure what they do.

If you get into the habit of putting spaces between "words" in your code it will be a lot more readable. Instead of this


PID botMotorPID(&botMotorPIDInput,&botMotorPIDOutput,&botMotorPIDSetpoint,Kp1,Ki1,Kd1,DIRECT);



try this


PID botMotorPID(&botMotorPIDInput, &botMotorPIDOutput, &botMotorPIDSetpoint, Kp1, Ki1, Kd1, DIRECT);





...R

Hi Robin, The ISRs are for reading pulses from a quadrature incremetal encoder that gives me a 4096 pulse count per revolution. I am trying to synchronize two motors/propellers based on their respective encoder counts. I add or subtract small increments to the speed of the motors so that their count values are the same any time they are read, leading then to be in sync with each other. Please let me know if i should explain more.

shankarkumarj: Please let me know if i should explain more.

OK, now I understand your objective. But the code in your ISRs is rather cryptic. Can you write a few paragraphs to explain how it works?

...R

Robin2: OK, now I understand your objective. But the code in your ISRs is rather cryptic. Can you write a few paragraphs to explain how it works?

...R

The rotary encoder gives out A,B,I Incremental Interface signals. The I signal occurs once every revolution. So In the Interrupt for the 'I' signal I take the time difference between to successive 'I' Signal occurrences and divide by a constant to give me the RPM of the motor.

A rotary encoder usually has a 'PPR' description which states how many pulses it can generate in one rotation. The one I have states 4096 PPR. The background behind this is that the rotary encoder is nothing but a 14 bit counter, so it can have 0-16393 values. These A and B pulses are generated upon each step of rotation.

The A signal comes first, followed by the B signal and this repeats.

In the ISR what i do is read if the particular pin is high or not. If i were to use only the A signal, I can measure every low to high transition or high to low transition and increment a counter on the same. so i'd have 2048 counts in a revolution using just the 'A' signal.

When I also include the 'B' signal, I can check if the 'A' signal came before 'B' or did the 'B' signal come before 'A', thereby also giving me the direction of rotation. So in all the ISRs are checking if the A pin went high or not before/after the 'B' pin went high to increment a common counter variable. This combined method gives me 4096 steps per revolution which is the maximum resolution of the encoder.

I was really hoping that you give a few lines of explanation about each of your ISR functions - something like this

DoEncoderABot() is triggered by xxx and it does yyy

DoEncoderBBot() is triggered by kkk and it does ppp

to help me understand the program code

...R

Robin2:
I was really hoping that you give a few lines of explanation about each of your ISR functions - something like this

DoEncoderABot() is triggered by xxx and it does yyy

DoEncoderBBot() is triggered by kkk and it does ppp

to help me understand the program code

...R

Sorry about that,

DoEncoderABot() is triggered by the 'A' signal from the encoder, checks if DoEncoderBBot() (i.e by the B signal) was triggered before it. If yes, which meant we are moving in the forward direction, increment the counter. Else decrement the counter.

DoEncoderBBot() is triggered by the 'B' signal from the encoder, checks if DoEncoderABot() (i.e by the A signal) was triggered before it. If yes, which meant we are moving in the forward direction, increment the counter. Else decrement the counter.

I think the below waveform will better explain it

ABI image

shankarkumarj: Sorry about that,

This is like trying to get blood from a stone.

You have six ISRs, yet you only mention two of them.

Help us to help you.

...R

Okay. I appreciate your time in reading what I post and helping me out. I have already mentioned that there are two motors. So two encoders. So 2 pairs of ABI signals. So whatever ISRs I have explained just duplicated for the second motor.

To sum it up

For the Bottom Motor:

DoEncoderABot() is triggered by the 'A' signal from the encoder, checks if DoEncoderBBot() (i.e by the B signal) was triggered before it. If yes, which meant we are moving in the forward direction, increment the counter. Else decrement the counter.

DoEncoderBBot() is triggered by the 'B' signal from the encoder, checks if DoEncoderABot() (i.e by the A signal) was triggered before it. If yes, which meant we are moving in the forward direction, increment the counter. Else decrement the counter.

DoEncoderIBot() is triggered by the 'I' signal from the encoder. It measures the timestamp it was triggered, and the difference between two timestamps helps me get the RPM. This ISR outputs the time difference.

All these ISRs are duplicated for the Top motor.

so we have

DoEncoderATop()

DoEncoderBTop()

DoEncoderTTop()

with the same functionality as mentioned above.

On a side note,why is everyone so sarcastic over here? I am no expert, not even close, I'm here to learn and sought out help because I cannot figure out what's wrong and I might've overlooked things in my code. Hell I might've even written crappy code that might have people laugh, but constructive criticism is always welcome :)

I am near the end of my patience.

You have 6 ISRs.

  • DoEncoderABot()
  • DoEncoderBBot()
  • DoEncoderIBot()
  • DoEncoderATop()
  • DoEncoderBTop()
  • DoEncoderITop()

Some of them seem to have something to do with time - you have not mentioned them at all.

Over to you …

I don’t think you need two ISRs to detect the direction of the encoder. And if you are controlling a motor don’t you already know the direction?

…R