Interrupts and Delays

I'm new to Arduino and I am currently experimenting with fuel injecting a single-cylinder small engine using an Uno. It's pretty simplistic at the moment. The pulsewidth of the injection event is fixed at the moment. To detect engine rotation, I just have a simple magnet and hall-effect sensor. The injection event itself is pretty simple; the arduino just needs to put a pin high for the desired duration, then pull it low again.

I have my interrupt setup as such:

attachInterrupt(0, inject, FALLING);

and that works fine. In my inject function, I had it setup as:

digitalWrite(11, HIGH);
delay(PW / 10);
digitalWrite(11, LOW);

I quickly found out that didn't work correctly at all, as it turns out delays do not function properly within interrupts. I was advised to use delayMicroseconds(), as that should work within an interrupt loop. However, it was also behaving erratically, and the actual pulsewidth was not matching up with the commanded pulsewidth at all. I did some searching, and found that some people were also having trouble getting delayMicroseconds() to behave properly within an interrupt.

A friend eventually suggested a workaround, where instead of having the injection code in the interrupt function, I just have it set a flag = 1, and then put the injection code into the main void loop() as such:

if (flag == 1) {
    flag = 0;
    digitalWrite(11, HIGH);
    delay(PW / 10);
    digitalWrite(11, LOW);

This seems to work well enough, but I feel like it is a kind of dodgy work around. Is there any way to get this working within the ISR? Or perhaps do what I'm trying to do without using delays at all?

What's the engine rotation speed? What sort of timing precision do you need? What's the pulse width you're looking for?

You might not even need an interrupt to do this.

Can you post all your code so we can see if you have other errors.
This thread is the same topic, well eventually we find this out.
http://forum.arduino.cc/index.php?topic=223798.0

Jiggy-Ninja:
What's the engine rotation speed? What sort of timing precision do you need? What's the pulse width you're looking for?

You might not even need an interrupt to do this.

~4500 RPM or so maximum, but I eventually plan to move it to a twin cylinder engine that goes to about 11,000 RPM. The twin will need to utilize two sensors, one for each cylinder. The single cylinder engine triggers off the crank, so around a maximum of 75Hz or so. The twin will trigger off the camshaft, which rotates at half engine speed, so call it about 90-100Hz.

Since I'm not using the arduino to control the spark, not too much timing accuracy is needed (many OEM EFI systems inject against closed valves)

Pulsewidth will range probably between 1-3 ms at idle, to around 15-25ms at the max.

Here's the code I'm currently using, it also implements an RPM display on my VFD.

#include <GU7000_Interface.h>
#include <GU7000_Serial_Async.h>
#include <Noritake_VFD_GU7000.h>
GU7000_Serial_Async interface(38400, 4, 5, 6);
Noritake_VFD_GU7000 vfd;
volatile long lastPulseTime = 0;
volatile long rpm = 0;
int PW = 25;
volatile int flag = 0;


void setup() {
  pinMode(2, INPUT);
  digitalWrite(2, HIGH);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);
  digitalWrite(10, LOW);
  attachInterrupt(0, inject, FALLING);
  _delay_ms(500);
  vfd.begin(140, 32);
  vfd.isModelClass(7000);
  vfd.interface(interface);
  vfd.GU7000_reset();
  vfd.GU7000_init();
  vfd.print("Pulse Width: 2.5ms");
}
void loop() {
  if (rpm < 4500){
  if (flag == 1) {
    flag = 0;
    digitalWrite(11, HIGH);
    delay(PW / 10);
    digitalWrite(11, LOW);
  }
  }
}
void inject()
{ unsigned long now = micros();
  unsigned long interval = now - lastPulseTime;
  if (interval > 2000)
  {
    rpm = 60000000UL / (interval * 1);
    lastPulseTime = now;
  }
  vfd.GU7000_setCursor(0, 30);
  vfd.print("RPM:");
  if (rpm < 1000) {
    vfd.print(" ");
  }
  if (rpm < 100) {
    vfd.print(" ");
  }
  vfd.print(rpm, 10);
  flag = 1;
}

This is another program that implements a rotary encoder so i can manually adjust the pulsewidth:

#include <GU7000_Interface.h>
#include <GU7000_Serial_Async.h>
#include <Noritake_VFD_GU7000.h>
GU7000_Serial_Async interface(38400, 4, 5, 6);
Noritake_VFD_GU7000 vfd;
const int switchPin = 13;
const int switch2Pin = 12;
int switchState = 0;
int switch2State = 0;
int prevSwitchState = 0;
int prevSwitch2State = 0;
volatile long lastPulseTime = 0;
volatile long rpm = 0;
int PW = 0;
volatile int flag = 0;


void setup() {
  pinMode(switchPin, INPUT);
  pinMode(switch2Pin, INPUT);
  pinMode(2, INPUT);
  digitalWrite(2, HIGH);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);
  digitalWrite(10, LOW);
  attachInterrupt(0, inject, FALLING);
  _delay_ms(500);
  vfd.begin(140, 32);
  vfd.isModelClass(7000);
  vfd.interface(interface);
  vfd.GU7000_reset();
  vfd.GU7000_init();
  vfd.print("Pulse Width: 0.0ms");
}
void loop() {
  if (rpm < 4500){
  if (flag == 1) {
    flag = 0;
    digitalWrite(11, HIGH);
    delay(PW / 10);
    digitalWrite(11, LOW);
  }
  }
  switchState = digitalRead(switchPin);
  switch2State = digitalRead(switch2Pin);
  if (switch2State != prevSwitch2State) {
    if (switch2State == LOW) {
      PW = PW + 2;
      vfd.GU7000_setCursor(112, 0);
      vfd.GU7000_setCursor(0, 0);
      vfd.print("Pulse Width:");
      vfd.GU7000_setCursor(84, 0);
      if (PW / 10 < 10) {
        vfd.print(" ");
      }
      vfd.print(PW / 10, 10);
      vfd.print(".");
      vfd.print(PW % 10, 10);
      vfd.print("ms");
    }
    prevSwitch2State = switch2State;
  }
  if (switchState != prevSwitchState) {
    if (switchState == LOW) {
      PW = PW - 2;
      if (PW < 0) {
        PW = 0;
      }
      vfd.GU7000_setCursor(84, 0);
      vfd.GU7000_setCursor(0, 0);
      vfd.print("Pulse Width:");
      vfd.GU7000_setCursor(84, 0);
      if (PW / 10 < 10) {
        vfd.print(" ");
      }
      vfd.print(PW / 10, 10);
      vfd.print(".");
      vfd.print(PW % 10, 10);
      vfd.print("ms");
    }
    prevSwitchState = switchState;
  }
}
void inject()
{ unsigned long now = micros();
  unsigned long interval = now - lastPulseTime;
  if (interval > 2000)
  {
    rpm = 60000000UL / (interval * 1);
    lastPulseTime = now;
  }
  vfd.GU7000_setCursor(0, 30);
  vfd.print("RPM:");
  if (rpm < 1000) {
    vfd.print(" ");
  }
  if (rpm < 100) {
    vfd.print(" ");
  }
  vfd.print(rpm, 10);
  flag = 1;
}

It seems a bit pointless doing that division calculation over and over again.

The argument of the delay() function is an integral type ( actually it is a long ), so when you divide PW by 10, then if PW is 25, the answer you get is 2, not 2.5 . Not only does that division waste time, but it also gives you the wrong answer. And if you increase PW in small steps to say, 31, the actual delay will suddenly jump by 50% from 2 to 3. This may make you control seem rather jerky.

Which calculation are you talking about?

Anyways, so should I switch to delayMicroseconds then?

Which calculation are you talking about?

delay(PW / 10);

Pre-calculate the value, don't do it every time. But there are other problems as noted.

int x = PW / 10;
...
delay(x);


Rob

Using an interrupt to set a flag is normal. It's not a kludge.

You would only need the injection code within the interrupt routine if you need very accurate timing - within a few microseconds. And if you need that digitalWrite(), delay() and delayMicroseconds() would probably be too coarse anyway. You would probably write a short loop of assembler code whose time you would measure with an oscilloscope. None of this seems necessary for your application.

...R

I've simplified the code and re-implemented the digital encoder control of the pulsewidth. It's now setup to inject every other cycle (will need to see how this works on the engine itself) and update the RPM every 10 pulses. I'm hoping this will cut down on time wasted writing to the display. The library provided by the VFD manufacturer seems to be rather slow at writing to the display.

#include <GU7000_Interface.h>
#include <GU7000_Serial_Async.h>
#include <Noritake_VFD_GU7000.h>
GU7000_Serial_Async interface(115200, 4, 5, 6);
Noritake_VFD_GU7000 vfd;
const int sPin = 13;
const int s2Pin = 12;
volatile long lastPulseTime = 0;
volatile long rpm = 0;
long PW = 0;
volatile int inj = 0;
volatile int rpto = 0;


void setup() {
  pinMode(sPin, INPUT);
  pinMode(s2Pin, INPUT);
  pinMode(2, INPUT);
  digitalWrite(2, HIGH);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);
  digitalWrite(10, LOW);
  attachInterrupt(0, inject, FALLING);
  _delay_ms(500);
  vfd.begin(140, 32);
  vfd.isModelClass(7000);
  vfd.interface(interface);
  vfd.GU7000_reset();
  vfd.GU7000_init();
  vfd.print("Pulse Width: 0.0ms");
}
void loop() {
  if (rpm < 4500)
  {
    if (inj == 2) 
    {
      inj = 0;
      digitalWrite(11, HIGH);
      delayMicroseconds(PW);
      digitalWrite(11, LOW);
    } 
  }
  if (digitalRead(s2Pin) == HIGH) 
  {
    PW = PW + 500;
    pwud();
  }
  if (digitalRead(sPin) == HIGH && PW > 0)
  {
    PW = PW - 500;
    pwud();  
  }
}
void inject(){
  unsigned long now = micros();
  unsigned long interval = now - lastPulseTime;
  if (interval > 2000)
  {
    rpm = 60000000UL / (interval * 1);
    lastPulseTime = now;
  }
  inj = inj + 1;
  rpto = rpto + 1;
  rpmud();
  }
void pwud(){
  vfd.GU7000_setCursor(112, 0);
  vfd.GU7000_setCursor(0, 0);
  vfd.print("Pulse Width:");
  vfd.GU7000_setCursor(84, 0);
  if (PW / 1000 < 10) 
  {
    vfd.print(" ");
  }
  vfd.print(PW / 1000, 10);
  vfd.print(".");
  vfd.print(PW % 1000, 10);
  if (PW % 1000 < 10)
  {
    vfd.print("00");
  }
  vfd.print("ms");
}
void rpmud(){
  if (rpto == 10){
    rpto = 0;
    vfd.GU7000_setCursor(0, 30);
    vfd.print("RPM:");
    if (rpm < 1000) 
    {
      vfd.print(" ");
    }
    if (rpm < 100) 
    {
      vfd.print(" ");
    }
    vfd.print(rpm, 10); 
}
}

Why not separate the updating of the display from the injection pulses and RPM calcs so that they happen when they are needed without compromise and the display is updated only as often as necessary to make it useful. I should have though that updating the display more than once per second would be irrelevant, and every 5 seconds may be sufficient.

...R

In this case, I need a relatively "live" display of the RPM. The manual pulsewidth control will eventually be used to tune the engine across a variety of operating conditions. There will be a matrix of throttle position vs. rpm with a potentiometer later installed.

How would I go about making a periodically updated display? Just put the update function the main loop with an if statement and some sort of timer implemented?