RPM Interrupt speed limit

system · November 4, 2012, 7:08pm

I am recording the RPM of a motor as start to a bigger project, the below code seems to have a limit of around 7000 to 8000 RPM. I am trying find out if this is a limitation of my hardware, the design of the hardware, code or Arduino.

The hardware consists of a IR LED and IR photosensor (SFH229 and SFH4350) spaced about 7mm apart at a radius of 23mm, a propeller (blade 12mm wide) pass's the IR's twice every revolution.

Any help or suggestions would be appreciated

Regards Simon

// RPM output to Serial Monitor using Interupt

volatile byte rpmcount;
unsigned int rpm;
unsigned long timeold;

void setup()
{
  Serial.begin(9600);
  //Interrupt 0 is digital pin 2, so that is where the IR detector is connected
  //Triggers on FALLING (change from HIGH to LOW)
  attachInterrupt(0, rpm_fun, FALLING);

  rpmcount = 0;
  rpm = 0;
  timeold = 0;
}

void loop()
{
  //Don't process interrupts during calculations
  detachInterrupt(0);
  //Note that this would be 60*1000/(millis() - timeold)*rpmcount if the interrupt
  //happened once per revolution instead of twice. Other multiples could be used
  //for multi-bladed propellers or fans
  rpm = 30*1000/(millis() - timeold)*rpmcount;
  timeold = millis();
  rpmcount = 0;
  //Print out result to Serial
  Serial.println(rpm);


  //Restart the interrupt processing
  attachInterrupt(0, rpm_fun, FALLING);
}

void rpm_fun()
{
  //Each rotation, this interrupt function is run twice, so take that into consideration for 
  //calculating RPM
  //Update count
  rpmcount++;
}

system · November 4, 2012, 7:19pm

  //Don't process interrupts during calculations
  detachInterrupt(0);

The comment is wrong. Some (maybe even many) interrupts will happen while your interrupt is detached.

  rpm = 30*1000/(millis() - timeold)*rpmcount;
  timeold = millis();
  rpmcount = 0;
  //Print out result to Serial
  Serial.println(rpm);

  //Restart the interrupt processing
  attachInterrupt(0, rpm_fun, FALLING);

Your interrupts are detached for a long time.

Detach the interrupt. Copy the values in rpmcount, now, and then. Reset stuff. Then, reattach the interrupt. Only then, perform the calculations, based on the copies, and print the result.

system · November 4, 2012, 9:34pm

PaulS

The comment is wrong. Some (maybe even many) interrupts will happen while your interrupt is detached.

Does this have any connection with my question or is this just an observation of my code comments?

Your offer a statement that the interrupt is detached for a long time. Is this why a see a limit at around 7000 RPM? Thank you for telling me what's wrong but it doesn't give me an opportunity to learn or move forward. Simon

system · November 4, 2012, 10:36pm

I suspect you may have a problem with the rpmcount overflowing in between consecutive executions of loop().

I suggest you estimate the longest possible interval between executions of loop() - I suspect the Serial output will be the limiting factor on loop frequency - and work out the maximum possible number of interrupts in that period. Choose an unsigned integer type big enough to hold that value without overflow, and use that throughout your calculations.

I’m not sure why you are detaching the interrupt each time you read rpmcount, but if you’re trying to ensure the rpmcount is not updated during the read operation then as a byte value that is not an issue since a byte assignment is inherently atomic; if you increase the size it might become an issue in which case you might need to disable interrupts while you do an atomic copy from the volatile global variable to a local variable and enable them afterwards. Note that disabling interrupts is not the same as detaching the handler and if an interrupt happened to occur during that precise moment when you were doing the assignment then the interrupt would only be delayed, not lost. If you detach the interrupt handler, any interrupts that occur while it’s detached are lost, though. (I suspect this is what PaulS is trying to prompt you to think about.)

system · November 4, 2012, 11:13pm

Does this have any connection with my question or is this just an observation of my code comments?

In any topic that contains the words why, doesn’t, my, code, and work, in that order, with few intervening words, the whole of the code is fair game for analysis.

Often, the comments are an indication of the way the poster thinks the code works. When they are wrong, it is only right to point out the discrepancy between what the comment says and what the code does.

Your offer a statement that the interrupt is detached for a long time.

And, I showed all the code that happens during the do-not-interrupt-me period.

Is this why a see a limit at around 7000 RPM?

I think so. For a large percentage of the time, you are ignoring interrupts.

system · November 5, 2012, 10:16am

Peter, Thank you for the explanation.

Paul S, I am not suggesting your comment about other interrupts still running is untrue, but it is unclear if your are just pointing out my ignorance or it has an impact on my goal, with out an explanation it is difficult to decide if it is a worthwhile avenue to explore. Helping people is not just about saying what is wrong, is it? I don't want you to code it for me, I can buy code, I want to learn, have fun, as you can see by the style of my code and how times I have posted that I am very new to this and at this stage need a bit of "hand holding". Hopefully as time goes on, I can get better and help others, until then I am just another newbie asking questions in blissful ignorance.

Riva · November 5, 2012, 10:41am

As has been previously stated your stopping interrupt code for a VERY long time in the world of a 16Mhz microcontroller. I can see no reason to detach or stop interrupts during calculations of RPM in the main loop as the counter incremented by the ISR (rpmcount) is a byte value so should be written/read in a single instruction (thus no conflict between ISR and main loop using it)

system · November 5, 2012, 12:39pm

but it is unclear if your are just pointing out my ignorance or it has an impact on my goal

I'm pointing out nothing more than that the comment is wrong. It is then up to you to look at what detachInterrupt() does. When you do, you'll see that only the one interrupt is disabled. Not all the interrupts. There is another function for that.

Does that impact your project? No. Should you fix the comment anyway? Yes. Doing so will indicate that you understand what your code is doing.

as you can see by the style of my code and how times I have posted that I am very new to this

Dealing with interrupts is not a beginner project. If you are going to attempt a non-beginner-project, you can't also expect to be treated like a beginner.

system · November 6, 2012, 2:21am

Hi guys,

So I’ve got a program where I communicate to my arduino via serial from a windows form C# app on my pc.
I’m trying to pickup rpm (hall effect) and temperature (voltage) from sensor. Code would look something like this:

int message = 0;   //  This will hold one byte of the serial message
int ledPin = 13;   //  This is the pin that the led is conected to
//int LED = 0;      //  The value or brightness of the LED, can be 0-255
volatile byte rpmcount;
unsigned int rpm;
unsigned long timeold;
int rpmPin = 12;



void setup() 
{  
  pinMode(rpmPin, OUTPUT);
  
  Serial.begin(9600);  //set serial to 9600 baud rate
  pinMode(ledPin, OUTPUT);
  attachInterrupt(0, rpm_fun, RISING);
  rpmcount = 0;
  rpm = 0;
  timeold = 0;
}

void loop()
{
  digitalWrite(rpmPin, HIGH);
  delay(100);
  digitalWrite(rpmPin, LOW);
  delay(100);
  
  if (Serial.available() > 0) 
  { 
    message = Serial.read();   

    if (message == 'A')
   {  
     digitalWrite(ledPin, HIGH);      
     Serial.println("LED on");  
   }
   if (message == 'a')
   {  
     digitalWrite(ledPin, LOW);         
     Serial.println("LED off"); 
   }
   if (message = 's')
   {
     {
       if (rpmcount >= 2) 
       { 
         //Update RPM every 20 counts, increase this for better RPM resolution,
         //decrease for faster update
         rpm = 30*1000/(millis() - timeold)*rpmcount;
         timeold = millis();
         rpmcount = 0;
         Serial.println(rpm,DEC);
       }
     }

  } 
}
}

 void rpm_fun()
 {
   rpmcount++;
   //Each rotation, this interrupt function is run twice
 }

I get stuck in the the rpm function. Is this a limitation of the arduino or should I be able to serial input commands and read values (via serial) from different pins while running an interrupt?

Thanks in advance.

To add more clarity. I would like to send a command to run function to stream rpm (‘s’) then change functions for temp (‘t’)then stream that pin. Is this possible?

Moderator edit:
 [code] 

 [/code] 
tags added.

nickgammon · November 6, 2012, 6:40am

What do you mean "stuck"?

This is wrong:

   if (message = 's')

That should be "==".

How many counts are you expecting?

system · November 6, 2012, 11:43am

   if (message = 's')
   {
     {
       if (rpmcount >= 2) 
       { 
         //Update RPM every 20 counts, increase this for better RPM resolution,
         //decrease for faster update
         rpm = 30*1000/(millis() - timeold)*rpmcount;
         timeold = millis();
         rpmcount = 0;
         Serial.println(rpm,DEC);
       }
     }

  }

Maybe you need some more curly braces here...

system · November 6, 2012, 1:30pm

Cyclones: Is this a limitation of the arduino

No.

MarkT · November 6, 2012, 3:37pm

That's not a reliable way to communicate with an interrupt routine - you read the value of the rpmcount variable, decide to reset it to zero, then set it to zero. At any point between reading it and setting it to zero the interrupt routine might run and increment it - result is you miss this count (or counts).

Fortunately there is a simple way to avoid missing counts (without even having to disable interrupts).

byte lastcount ;
volatile byte rpmcount ;

void loop ()
{
  ...
  byte  newcount = rpmcount ;  // read the volatile counter once only.
  byte change = newcount - lastcount ;  // this assumes change is 127 or less
  lastcount = newcount ;  // ready for next time round loop
  totalcount += change ;
  ....
}

Each time round the loop you read the rpmcount variable and compare to the last time - this gives the number of counts since last time without ever updating the rpmcount variable. Counts cannot be lost.

Note that if the volatile counter was more than one byte (an int, say), then you would need to disable interrupts to read it as the ATmega328 has a byte-wide memory bus and ints take more than one instruction to read or write to memory.

system · November 6, 2012, 4:43pm

[quote author=Nick Gammon link=topic=130923.msg984817#msg984817 date=1352184047] What do you mean "stuck"?

This is wrong:

   if (message = 's')

That should be "==".

How many counts are you expecting? [/quote]

Thanks,

I'm expecting 0 - 4,000rpm

system · November 6, 2012, 4:54pm

Note that if the volatile counter was more than one byte (an int, say), then you would need to disable interrupts to read it as the ATmega328 has a byte-wide memory bus and ints take more than one instruction to read or write to memory.

You could have the ISR use two bytes, and emulate an int. Increment the "low order byte". If it overflows, increment the "high order byte". If the high order byte overflows, you're screwed anyway.

system · November 6, 2012, 6:25pm

PaulS: You could have the ISR use two bytes, and emulate an int. Increment the "low order byte". If it overflows, increment the "high order byte". If the high order byte overflows, you're screwed anyway.

Is there any difference between that, and using an actual int? Either way, it's no longer safe to assume that increments are atomic.

system · November 6, 2012, 6:28pm

Is there any difference between that, and using an actual int? Either way, it's no longer safe to assume that increments are atomic.

In the ISR, they will be. It's the copy of the data that is non-atomic. And, no, now that I think about it, this isn't an improvement.

nickgammon · November 6, 2012, 7:47pm

Please do not cross-post. This wastes time and resources as people attempt to answer your question on multiple threads.

Threads merged.

Both threads had similar code and were about the same concept, speed of interrupt processing. Some people answered one thread, and some another. Please don't do this.

Moderator

gardner · November 7, 2012, 4:21am

Maybe I am missing something, but I am unsure why no one has suggested simply using cli() and sei() to block interrupts whilst reading a totally ordinary int or long value, then unblocking.

#include <avr/interrupt.h> 

volatile long rpmcount;

 void rpm_fun()
 {
   rpmcount++;
 }

void setup() 
{  
  attachInterrupt(2, rpm_fun, RISING);
}

void loop()
{
 long count;

 cli();
 count = rpmcount;
 sei();

 // what do I want to do with this count now?
}

cli and sei are documented here: avr-libc: interrupt.h File Reference

nickgammon · November 7, 2012, 5:02am

In the original post he had it as a byte so that wasn't necessary.

To answer the original question, and skip the intervening discussion a bit, I think your limitation comes from the fact that you are storing into a single byte (rpmcount).

Second, at 8000 RPM you have 133.33 RPM per second, so you have 1.9 seconds before wrap-around.

Meanwhile, to display (say) 8000 to the serial port at 9600 baud plus a carriage-return/linefeed would take 6.25 mS ((1/960) * 6).

At 133 interrupts per second you will get one every 1/133 mS (ie. every 7.5 mS).

I can imagine that at much over 8000 RPM the rate at which you are displaying will take more time than you have in hand. Plus you should probably clear the pending interrupt before re-enabling interrupts.

See: http://www.gammon.com.au/interrupts

Scroll down to "How are interrupts queued?". In particular you could clear any pending interrupt like this:

EIFR |= _BV (INTF0);  // clear flag for interrupt 0

I'm inclined to agree with gardner, except that I would clear the counter when interrupts are off, as well:

 noInterrupts ();  // critical section
 count = rpmcount;  // make copy
 rpmcount = 0;
 interrupts ();  // end critical section

At low speeds, RPM is more accurately measured by getting the period (and inverting it) rather than the frequency. I discuss this here:

http://www.gammon.com.au/forum/?id=11504

At 133 revs/sec you are clearly going to have around 0.75% error rate (ie. the next count would be 132 or 134). But if you measure the period with the hardware timer (or even just measure time between pulses) you would get a more precise measurement.