compubob:

I’ve seem to have run into a wall when trying to get the RPM of a DC Motor to use for PID Control.

I’ve butchered my own code so I’m going to start fresh with examples of just the rpm code so it’s easier to understand.

It would help if you posted all of the relevant code. For example, what is in your ISR? I can assume that it just increments a counter, but I don’t *know.* And I don’t know what type rpmcount is, if it was volatile or static, if you handle roll-overs, or if it gets reset in more than 1 place in the code. It is helpful to post the smallest code set that shows the problem, but post *all* of it.

when using this code to get the rpm, Because of the 1 second delay the PID code is only getting a rpm update every second which is making my motor pulse up and down and it never finds the setpoint, I change this to a very low number and it doesn’t matter, the delay just gets shorter but it doesn’t help. If I remove the delay, then the rpm doesn’t update at all. I assume it should, just with no delay? but I guess not.

```
void loop()
```

{

//Update RPM every second

delay(1000);

//Don’t process interrupts during calculations

detachInterrupt(0);

rpm = 30*1000/(millis() - timeold)*rpmcount;

timeold = millis();

rpmcount = 0;

attachInterrupt(0, rpm_fun, FALLING);

}

I can see why removing the delay is causing you grief. Without it, it is possible that the delta t (millis() - timeold) is 1 or even 0. Also, without the delay, most of your loop is spent with the ISR off, and you miss a bunch of interrupts. You need to design the code so that it doesn’t need the delay.

This code which is basically the same as above but instead of a delay it has a if statement. It updates the RPM every 20 pulses and does not go back to 0 if the rpm input stops, which is a issue. Because if the PID overshoots and the motor turns off, the rpm will never come back down and it will never start again.

```
void loop()
```

{

//Update RPM every 20 counts

if (rpm >= 20) {

//Don’t process interrupts during calculations

detachInterrupt(0);

rpm = 30*1000/(millis() - timeold)*rpmcount;

timeold = millis();

rpmcount = 0;

attachInterrupt(0, rpm_fun, FALLING);

}

Once again, delta t could be 0 or 1, which would make the rpm calculation bad. Also

instead of `if (rpm >= 20) { `

shouldn’t it be `if (rpmcount >= 20) {`

?

If rpm ever goes below 20, it will never change again. That’s not what you wanted, is it?

I can’t figure this out. I basically need the rpm to always be updated with no delay (so the first block of code is out) but the second block of code won’t go back to 0 if the input stops, it just stays at the last rpm it was at.

I might be going at this the wrong way, I don’t know. Someone please put me in the right direction. If it helps I’m using the PID library and the processing front end to monitor what’s going on with this.

Posting the whole actual code would help more.

OK, pointers. First of all, don’t detach and re-attach the ISR. You miss all the pulses while the ISR is detached. If you are careful about how to read the rpmcount, you can just leave the interrupts on all the time. Second, what is the type of rpmcount? It should be `volatile unsigned long`

.

I’m not sure about your calculation of rpm. If delta t is 0 the sketch will crash. If it is 1 then you get back 30,000 * rpmcount. So if you read 20 pulses in about 1 millisecond, your calculated rpm is 600,000. Does that sound right to you? OTOH, if delta t is large, then 30K/delta t may be 1 or 0. This calculation also loses a lot of precision by the order of operations. I’d go back and re-think the rpm calculation. You also don’t mention how fast the motor is or the resolution of your encoder. Too many interrupts coming too fast can overwhelm the arduino, and your rpmcount gets inaccurate.

Back to basics. To calculate the speed, you need to know the number of pulses and the elapsed time, and the resolution of the encoder. P/RT = speed. There are limits though. if T is too small then your calculation is not accurate, or will hit /0 errors. If you don’t have any pulses, then speed will be 0 no matter what T is (if you get 0/0 then the actual speed could be anything). You need to take this into account and watch out for the corner cases. Generally, there are two approaches. 1) measure the amount of time it takes to get a fixed number of pulses, or 2) count the pulses during a fixed interval of time. There’s actually a third approach where you measure both arbitrary time and pulses and divide, but as I mentioned, there are lots of corner cases. You seem to have tried all 3, but not quite correctly ;).

Just off the top of my head (not tested or even compiled), just to give an idea.

```
#define INTERVAL 200 // a fifth of a second. Picked arbitrarily.
#define RESOLUTION 20 // 20 pulses per revolution.
volatile unsigned long rpmcount = 0;
unsigned long oldtime;
void loop()
{
unsigned long deltaT ;
unsigned long pulses;
deltaT = millis() - oldtime;
pulses = rpmcount;
if (deltaT > INTERVAL) {
rpm = (pulses * 60 * 1000) / (RESOLUTION * deltaT); // order of ops is important.
oldtime = millis();
rpmcount = 0;
}
}
```

Some things to think about. Is millisecond resolution fine enough? There are arduino functions for microseconds, too. Is RPM the measurement you want to use? RPS or PPS or even PPmS could be used. If you are still not getting smooth movement from your PID algorithm, you might need to tune it. Tuning PID is tricky.

HTH,