3

In my recent project, I have RPM Square wave from vehicle. Now, based on that I want square wave based on falling edge but only alternative pulse of RPM square wave.

I think below image clear more properly.

Pulse

Upper yellowish square wave which I need. And below pinkish square wave is RPM square wave.

I made a code for that and it's working fine but when RPM is stable. But when RPM change rapidly then it's not working fine.

const int kPinRPM = 3;        // RPM / Digital Pin 3
const int RPMInterrupt = 1;   // RPM's interrrupt pin
byte engineCylinders = 2;         // Engine cylinders = 4;
byte engineCycles = 4;            // Engine cycle = 2
int refreshInterval = 750;        // Milliseconds between sensor updates
unsigned long previousMillis = 0;
volatile int RPMpulses = 0;
int RPM, Final_RPM;

const int kPinInj = 12;

volatile uint32_t kInjTime = 0;

void setup()
{
  Serial.begin(250000);

  pinMode(kPinRPM, INPUT_PULLUP); // Enable internal pullup
  attachInterrupt(RPMInterrupt, countRPM, FALLING); //Interrupt for RPM

  pinMode(kPinInj, OUTPUT);  
}

void loop()
{
  if(millis() - previousMillis > refreshInterval)
  {
    previousMillis = millis();
    RPM =  getRPM();
  }

  Serial.println(RPM);

  if((kInjTime > 0) && ((millis() - kInjTime) >= 10))
  {
    kInjTime = 0;
    digitalWrite(kPinInj, LOW);
  }
}

void countRPM()
{
  RPMpulses++;

  if(RPMpulses % 2 == 0)           // Skip alternative pulse
  {
    kInjTime = millis();
    digitalWrite(kPinInj, HIGH);
  }
}

int getRPM()
{
  Final_RPM = int(RPMpulses * (60000.0 / float(refreshInterval)) * engineCycles / engineCylinders / 2.0);
  RPMpulses = 0;
  Final_RPM = min(99999, Final_RPM);
}

After SUGGESTION, updating my codes. But it output waveform has some problem. In white circle, you can see that problem.

Waveforms

1

Rather than saying

if(RPMpulses % 2 == 0)           // Skip alternative pulse
  {
    kInjTime = millis();
    digitalWrite(kPinInj, HIGH);
  }

and later setting kPinInj in the main loop, you could act as follows:

digitalWrite(kPinInj, RPMpulses & 1); // Set or clear kPinInj
if(RPMpulses & 1) 
  kInjTime = millis();  // Store alternate times

This would ensure that your trace 1 toggles properly. If you want a shorter up-time on that trace, you could treat CHANGE interrupts rather than FALLING, and maintain RPMpulses as a number twice as big as now, ie use 120000 instead of 60000 in the RPM calculation, and could say:

digitalWrite(kPinInj, (RPMpulses & 3)==1); // Set or clear kPinInj
if((RPMpulses & 3)==1) 
  kInjTime = millis();  // Store alternate fall times

Note, the floating point arithmetic in getRPM() is slow. At a 750 ms refresh interval, that may not matter much. But you probably can get good enough results rather more quickly using just integer arithmetic, if you parenthesize the expression somewhat as follows:

Final_RPM = ((RPMpulses * (60000UL / 2) * (engineCycles/engineCylinders)) / refreshInterval;

That is, do any remainderless divisions early; multiply all the remaining numerator terms together into a large number; then divide by the comparatively small refreshInterval, ending up with numerous significant digits without the use of floating point arithmetic, which takes many milliseconds on typical Arduino systems.

Edit 2: If your modified code still says RPMpulses = 0; within getRPM(), you may get a glitch at the end or beginning of each refresh interval. You may instead need to say RPMpulses = 1;, or RPMpulses &= 1;, or some such. Also, to avoid an extra count when the latter expression is 1 instead of 0, say RPM0 = RPMpulses &= 1; and use (RPMpulses-RPM0) instead of RPMpulses in calculating Final_RPM.

3
  • Thank you. I tried out this code. But I faced some problem in Output waveform. I just posted out photo. Please see it. – Hasan Mar 6 '17 at 6:26
  • @Hasan see edit 2 – James Waldby - jwpat7 Mar 6 '17 at 6:49
  • Ok, thank you. It is working fine now after your second suggestion. Of course, I do. Don't worry. – Hasan Mar 6 '17 at 7:19
1

There are some things you shouldn't be doing within an interrupt service routine (ISR) like countRPM():

  1. Don't call the millis() function. It depends on other interrupts which will be disabled during the execution of your ISR.

  2. Don't call digitalWrite or any other functions

  3. Don't use execution cycles hungry operators like % (modulo). To obtain modulo 2, just use a bit mask for the LSB.

What you should actually be doing within the ISR is just setting/resetting a flag or variable. Then, do all your stuff in the loop() function according the value of the flag or variable.

3
  • 1
    Calling millis() is OK. It won't increment inside the IRS, but it works fine in an use case like this. – Edgar Bonet Mar 2 '17 at 17:05
  • 1. As E.B. notes, calling millis() is ok – it doesn't need interrupts to report a result. 2. Although digitalWrite() is slow – a couple of microseconds – it's ok to call it. It's true that calling non-reentrant functions should be avoided. 3. Typically, compiler implements div or mod by any power of 2 by shifting or via bitwise AND with a mask; ie, while % may in general be "execution cycle hungry", %2 isn't. That said, I'd use a mask as you suggest. – James Waldby - jwpat7 Mar 2 '17 at 17:28
  • 1
    Hey, @jwpat7, I'm also E.B.! :) – Enric Blanco Mar 2 '17 at 17:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.