Newbie in need of some basic help

Hello, I am new to the forum and this is my first post, hopefully I have put it in the correct section.

I am also new to Arduino, although I used to be very good at Visual Basic coding (about 10

years ago!).

My first project is causing me some problems, so after two weeks of trying I think that it

is time to ask for some help as I seem to be missing something.

My project is as follows - it is the relatively common fuel flow measurement problem where

I have two flow sensors, one in the fuel line to the engine and one in the return line to

the tank. Each sensor is connected using a transistor to the Arduino Uno digital pins.

I need to subtract the return pulse from the flow pulse and send a pulse from the Arduino

to show the amount of fuel used by the engine. The output pulses will then be sent to a

MPGuino where I can use it as the injector input.

I have sent up a test rig with two square wave signal generators feeding into digital pins

2 and 3 and the output is from Pin 4. I have frequency counters connected to all of the

three pins.

The code ( version number 30 or so !)

volatile int Count = 0; // holds the number of pulses to send to MPGuino
volatile int flowCount = 0; // holds pulses to the engine
volatile int returnCount = 0; // holds the pulses from the engine.

void setup()  
{
  pinMode(2, INPUT);  //Pin 2 to input pulses from "FLOW" sensor
  pinMode(3, INPUT);  // Pin 3 for "RETURN" pulses
  pinMode(4, OUTPUT); // Output pulses (Flow minus Return)
  pinMode(13, OUTPUT); 
  digitalWrite(13, HIGH); // LED off
  attachInterrupt(digitalPinToInterrupt(2), ISR_FLOW, LOW);  
  // interrupt 0 digital pin 2 FLOW
  attachInterrupt(digitalPinToInterrupt(3), ISR_RETURN, LOW); 
  // interrupt 1 digital pin 3 RETURN
}

void loop() 
{
noInterrupts(); // Switch off interuupts while calculating
  Count = flowCount - returnCount;
 
    if (Count >= 1) // This is the only way I can get an output
    {
    Count = 1;
    }

    else
    {
    Count = 0;
    }
 
    if (Count = 1)
     {
    digitalWrite(4, HIGH); // These send a pulse
    digitalWrite(4, LOW);

    }
    
 interrupts(); // Switch interrupts back on
}
 



void ISR_FLOW() 
{

  if (digitalRead(2) == HIGH) // read "flow pin"
  {
    flowCount ++; // increment variable
  }
} 

void ISR_RETURN() 
{ 
  if (digitalRead(3) == HIGH) // read "return pin"
  {
    returnCount ++;   // increment variable
  } 
}

I would be grateful if someone could tell me if this code is too simplistic and if I am

missing something vital.

The output is varying quite a bit and bears no relation to the value that it should be when

using input frequencies of 80Hz for the "Return" and 150Hz for the "Flow".

Thank you for any help on this.

Pete

Just for starters,

    if (Count = 1)

should be

    if (Count == 1)

Get rid of the digital reads in the ISR. The ISR is triggered by the inputs. The fact that it is running means the pin is already active. You need to make your ISR variables type volatile and only disable interrupts when you read the values in the main program.

If the output frequency is supposed to vary in some kind of relationship with the input frequencies, your approach is completely misguided. You need to examine the problem of generating the output frequency which will not be in phase with either input. Your code doesn't produce anything like that. So yes, it is way too simplistic in that regard.

I have two suggestions. First, have a look at some existing ISR code to see how it works, and consult the documentation. Second, develop a high level design for the frequency measuring and conversion. Then code.

void ISR_FLOW()
{

  if (digitalRead(2) == HIGH) // read "flow pin"

Will pin 2 ever be HIGH at this point considering that the ISR is triggered by pin 2 being LOW. In any case, do you really want to trigger on LOW ? That could happen many, many times before the pin goes HIGH again. It would seem to be better to trigger on CHANGE so that it triggers only once with each change.

Many thanks for your reply.

I will change that and see what happens.

Hopefully it is as simple as that - it's quite a steep learning curve, especially when I have to look up the syntax of every command.

Does the basic structure of my sketch look OK? I don't mind spending time debugging it as it is a good way to learn - as long as the logic I am using is sound.

Thanks again.

I'm not sure what you want as an output? I think you want some sort of frequency? So if the flow to the motor is 150Hz and the flow back is 80Hz you want a signal of 70Hz? That's in no way what this code does.

But lets look at the code:

To start with, you say the sensors are connected to the Arduino with a transistor. Do you have a schematic? And do you use a pull up or pull down resistor? Because, depending on the orientation, a transistor can only pull up of pull down a line when switched on. When switched off the line is floating (and not simply the opposite of when on). You need a resistor for that.

And you set up the interrupts to trigger when LOW. But the first thing you in the ISR is to check for high... I think you want to set the RISING. That way the IRS is only called at the start of a pulse.

And what you do in the loop is a bit strange as well... No matter the value of count, as long as it's bigger then 1 you just force it one.... huh....

Also, keep in mind, loop will run very fast. Way faster then the 150Hz. So, when the IRS part was correct, the change of those being increased in the mean time is very small. Also, you never reset the values....

And as the variables go, make it easy for yourself and follow a convention. A normal var starts with a lower case (so count, not Count). flowCount and returnCount are fine. A lot of people (including me) reserve a veriable starting with a upper case for constants to remind myself it's a const and I cannot change it on the run.

Also make it yourself easy and give a name to a pin. Like

const byte FlowPin = 2;

Then when you write

attachInterrupt(digitalPinToInterrupt(FlowPin), ISR_FLOW, LOW);

it's clear what you mean and you don't have to look up which pin it's connected to.

I'm still thinking how I would do this. I think you need to get the time between the pulses and use it as the difference as the interval for the output pulse.

By the way, why does your post have all those random blank lines???

septillion:
I'm not sure what you want as an output? I think you want some sort of frequency? So if the flow to the motor is 150Hz and the flow back is 80Hz you want a signal of 70Hz? That's in no way what this code does.

That's what I am trying to do - I want to subtract the lower frequency from the higher one and output the difference.

septillion:
To start with, you say the sensors are connected to the Arduino with a transistor. Do you have a schematic? And do you use a pull up or pull down resistor? Because, depending on the orientation, a transistor can only pull up of pull down a line when switched on. When switched off the line is floating (and not simply the opposite of when on). You need a resistor for that.

I am using an NPN transistor with a 10K resistor between the +5V and the collector - the base is connected to the output from the square wave generator and the input pin on the Arduino is connected to the collector (sorry about no diagram)

septillion:
And what you do in the loop is a bit strange as well... No matter the value of count, as long as it's bigger then 1 you just force it one.... huh....

That was just to try and get some sort of sensible output as I wondered how fast the Arduino could read the pin changes.

The blank lines are maybe a leftover from coding in VB - a bad habit but it is just to make the structure easier for me to see and I would remove them once it all works.

aarg:
So yes, it is way too simplistic in that regard.

I have two suggestions. First, have a look at some existing ISR code to see how it works, and consult the documentation. Second, develop a high level design for the frequency measuring and conversion. Then code.

Thanks for your reply - I had a feeling that I was looking at this problem too simplistically.

I will spend time looking at ISR code examples.

Thanks.

Okay, the resistor between +5 and the input/collector is the pull up. But I hope you also have a resistor connected between the output of whatever is driving the transistor and the base. A BJT transistor is current driven so just applying a voltage is a bad idea...

And with the blank lines I didn't meant the blank lines in the code, those are very fine. But I meant the many blank lines in the starting post:

peteinchad:
Hello, I am new to the forum and this is my first post, hopefully I have put it in the correct section.

I am also new to Arduino, although I used to be very good at Visual Basic coding (about 10

years ago!).

My first project is causing me some problems, so after two weeks of trying I think that it
[etc]

I've only just noticed the blank lines - sorry!

I think they may be because I drafted it out in Notepad.

Hello again.

Apologies for taking so long to update this thread - I have had a total rethink and would be grateful for a bit more help.

I have got the code partly working - inputs and outputs now work but the maths seems to have a problem.

I produced two pin-change interrupts to count the pulses from the ‘flow’ sensor and the ‘return’ sensor:

const byte COUNT = 13;    // Defines output pin
const byte FLOWPIN = 2;   // Defines input pin from FLOW sensor
const byte RETURNPIN = 3; // Defines input pin from FLOW sensor
volatile byte fuel = 0;   // Variable to hold fuel count



void setup ()
{
  
  pinMode (COUNT, OUTPUT); // OUT to MPGuino
  digitalWrite (FLOWPIN, HIGH);  // internal pull-up resistor
  digitalWrite (RETURNPIN, HIGH);  // internal pull-up resistor
  
}  // end of setup

void loop ()
{
/* Attach interrupt to read FLOW sensor.
 *  isr_Flow reads Digital Pin 2 and for 5 seconds then
 *  it is detached.
 */

  attachInterrupt (digitalPinToInterrupt(FLOWPIN),isr_Flow , CHANGE);    

  delay(5000);  // to slow down code to see results

  detachInterrupt (digitalPinToInterrupt(FLOWPIN)); 


/* Attach interrupt to read RETURN sensor.
 *  Return reads Digital Pin 2 and for 5 seconds then
 *  it is detached.
 */

    attachInterrupt (digitalPinToInterrupt(RETURNPIN),isr_Return , CHANGE);  // attach interrupt handler

    delay(5000);    // to slow down code to see results

    detachInterrupt (digitalPinToInterrupt(RETURNPIN));

} 


//  (ISR) for FLOWPIN
void isr_Flow ()

/* Pin Change interrupt
 * 
 * Outputs one pulse for every FLOW pulse detected. 
 */
{
  if (digitalRead (FLOWPIN) == HIGH)
   { 
      digitalWrite (COUNT, HIGH);
      
      
   }
      else
      { 
      digitalWrite (COUNT, LOW);
      
      }
}  // end of isr_Flow




//  (ISR) for RETURNPIN

void isr_Return ()

/* Pin Change interrupt
 * 
 * Outputs one pulse for every RETURN pulse detected. 
 */
 
{
  if (digitalRead (RETURNPIN) == HIGH)
    { 
      digitalWrite (COUNT, HIGH);
            
      }
      else
      { 
      digitalWrite (COUNT, LOW);
      
    }
}  // end of isr_Return

The code above works fine - it outputs the same frequency as the input frequency from the flow counter for 5 seconds then outputs the same frequency as the return counter for 5 seconds. It then repeats this forever.

I have now altered the code so that, instead of outputting pulses from inside the interrupts, it increments a variable and then uses the variable to output a pulse stream, with the number of pulses that is equal to the value of the variable.

const byte COUNT = 13;      // Defines output pin
const byte FLOWPIN = 2;     // Defines input pin from FLOW sensor
const byte RETURNPIN = 3;   // Defines input pin from FLOW sensor
volatile int fuel = 0;      // Variable to hold fuel count
volatile int x = 0;         // Variable for use in loop



void setup ()
{
  
  pinMode (COUNT, OUTPUT); // OUT to MPGuino
  digitalWrite (FLOWPIN, HIGH);  // internal pull-up resistor
  digitalWrite (RETURNPIN, HIGH);  // internal pull-up resistor
  //attachInterrupt (digitalPinToInterrupt(FLOWPIN),isr_Flow , CHANGE);  // attach interrupt handler
  //attachInterrupt (digitalPinToInterrupt(RETURNPIN),isr_Return , CHANGE);  // attach interrupt handler
  
}  // end of setup

void loop ()
{
/* Attach interrupt to read FLOW sensor.
 *  isr_Flow reads Digital Pin 2 and for 5 seconds then
 *  it is detached.
 */

  attachInterrupt (digitalPinToInterrupt(FLOWPIN),isr_Flow , CHANGE);    

  //delay(5000);  // delay disabled

  detachInterrupt (digitalPinToInterrupt(FLOWPIN)); 


/* Attach interrupt to read RETURN sensor.
 *  Return reads Digital Pin 2 and for 5 seconds then
 *  it is detached.
 */

  attachInterrupt (digitalPinToInterrupt(RETURNPIN),isr_Return , CHANGE);  // attach interrupt handler

  //delay(5000);    // delay disabled

  detachInterrupt (digitalPinToInterrupt(RETURNPIN));

} 

/*
 * do_while loop to output a pulses to COUNT pin
 * while variable 'x' is < or = to variable 'fuel'
 * 'x' and 'fuel' are reset to 0 after loop
 */
    

  do 
  {
  digitalWrite (COUNT, HIGH);
  digitalWrite (COUNT, LOW);

  }

  while (x <= fuel);
     
    x = 0;
   
    fuel = 0;
      
}


 


//  (ISR) for FLOWPIN
void isr_Flow ()

/* Pin Change interrupt
 * 
 * increments 'fuel' for every FLOW pulse detected. 
 */
{
  if (digitalRead (FLOWPIN) == HIGH)
   { 
     fuel ++;   // increment
   }
      else
      { 
      fuel = fuel;  // do not increment
      }
}  // end of isr_Flow



//  (ISR) for RETURNPIN
void isr_Return ()

/* Pin Change interrupt
 * 
 * decrements 'fuel' for every RETURN pulse detected. 
 */
 
{
  if (digitalRead (RETURNPIN) == HIGH)
    { 
      fuel --;      //decrements
      
      }
      else
      { 
      fuel = fuel;  // do not decrement
      }
}  // end of isr_Return

This is where I have hit a problem - I have used a ‘do_while’ loop, a ‘for’ loop and an ‘if’ loop but all of them give the same result.

The results are as follows.

Instead of alternating between the flow and return values, the output just shows 480 for a minute or so and then shows zero. The zero I assume is the frequency counter that is attached to the output pin overflowing.

The frequencies I am inputting are both less than 200Hz - with the ‘flow’ frequency always being larger than the ‘return’ frequency.

I feel that I am quite close to getting this to work but would appreciate any help on how to get the logic / maths of the variables to work.

Thanks

Pete.

Why are you attaching/detaching the ISRs in loop() instead of just attaching them in setup() ?

  attachInterrupt (digitalPinToInterrupt(FLOWPIN), isr_Flow , CHANGE);

  //delay(5000);  // delay disabled

  detachInterrupt (digitalPinToInterrupt(FLOWPIN));

How long do you think that they will stay attached ?

Hi, thanks for your reply.

I tried them in setup but couldn't get any sensible ouputs - the outputs were jumping up to 5000 and down to 24 with values inbetween.

I put them in loop just to see what happened - I am new to Arduino so have to try some trial and error.

I don't know how long they will stay attached but I have managed to get alternating 'flow' and 'return' input values on the output - but maybe that is because of the delay between the attaching and detaching of the interrupts?

I'll move them back to setup and try it.

Thanks for your help.

Hi again,

I’ve put them back into setup()

And I’ve replaced the ‘do_while’ loop with a ‘for’ loop

With both types of loop I get an output of 480Hz with flow input of 110Hz and return input of 70Hz.

I have tried varying the inputs but the output doesn’t change but still goes to zero after approximately a minute (must be an overflow on the frequency meter I assume)

const byte COUNT = 13;      // Defines output pin
const byte FLOWPIN = 2;     // Defines input pin from FLOW sensor
const byte RETURNPIN = 3;   // Defines input pin from FLOW sensor
volatile int fuel = 0;      // Variable to hold fuel count
volatile int x = 0;         // Variable for use in loop



void setup ()
{
  
  pinMode (COUNT, OUTPUT); // OUT to MPGuino
  digitalWrite (FLOWPIN, HIGH);  // internal pull-up resistor
  digitalWrite (RETURNPIN, HIGH);  // internal pull-up resistor
  attachInterrupt (digitalPinToInterrupt(FLOWPIN),isr_Flow , CHANGE);  // attach interrupt handler
  attachInterrupt (digitalPinToInterrupt(RETURNPIN),isr_Return , CHANGE);  // attach interrupt handler
  
}  // end of setup

void loop ()
{


  for (x; x <= fuel; x++) 
  {
  digitalWrite (COUNT, HIGH);
  digitalWrite (COUNT, LOW);
  }
     
    x = 0;
   
    fuel = 0;
      
}


 
//  (ISR) for FLOWPIN
void isr_Flow ()

/* Pin Change interrupt
 * 
 * increments 'fuel' for every FLOW pulse detected. 
 */
{
  if (digitalRead (FLOWPIN) == HIGH)
   { 
     fuel ++;   // increment
   }
      else
      { 
      fuel = fuel;  // do not increment
      }
}  // end of isr_Flow



//  (ISR) for RETURNPIN
void isr_Return ()

/* Pin Change interrupt
 * 
 * decrements 'fuel' for every RETURN pulse detected. 
 */
 
{
  if (digitalRead (RETURNPIN) == HIGH)
    { 
      fuel --;      //decrements
      
      }
      else
      { 
      fuel = fuel;  // do not decrement
      }
}  // end of isr_Return

Thanks

      fuel = fuel;  // do not incrementWhy ?

Just to make sure that if the pin doesn't change state then the fuel variable isn't changed - I do not need it I assume?

I think that I have found the problem. I have just received a better frequency meter in the post and the output frequency is over 80kHz when both inputs are less than 200Hz - that is why the original meter was overflowing (it is a cheap panel meter off eBay with a limit of 10kHz).

I have looked at my code in the loop() section - it is collecting input pulses and doing the maths (I think!) and it is probably storing the correct number in the 'fuel' variable.

The problem is that the way I am outputting the pulses with the 'for' loop means it will output them at the maximum speed that the Arduino can run at.

I think that I need to time how long it took to collect the pulses and then somehow output them over the same period of time - more reading on timing I think!

Thanks for your help.

I'm about to go write a new program that just outputs random lines of code for people who would rather just put random things in random places than to try to learn what those lines of code do and try to make sense of it. Why write out your logic and try to write code to match when you can just randomly shuffle your code and add random lines and hit upload and see what happens. Kinda like a thousand monkeys with a thousand typewriters, eventually they'll write shakespear and probably the code for half of these posts. Just keep hitting the randomize button and eventually after a few thousand years it will do what you want.

Although I think it would really be easier to write it out in English first and then find lines of code to match each step. But that might require actually reading a reference. Who wants to do that when they can just iterate through a for loop, an if statement, a while loop and a do while loop.

I got my Arduino less than a month ago and have had to rely on looking for examples of code that might do the job I am attempting and then change them to try to do what I want. By changing them I have to try to understand them which helps me learn.

I have written out the logic of what I am trying to achieve several times, but I am also having to rely on trial and error with changes I make because I am not familiar with coding on Arduino - I am still at the stage where I have to look up the syntax before I type most things.

I am sorry if my code seems random - it probably is to someone with years more experience than I have. but to me (with only a month's experience) it is the best I have managed to do so far.

Hopefully, in time, I will get better.

I would use an interrupt for the return sensor and have that increment a variable. Then the main loop sits and waits for a pulse from the flow sensor. If the return is zero then that pulse can be sent to the output. Delay() will be ok to use for the pulse width or you use the width of the incoming pulse as your timing. If the return is non zero, then decrement it and don't output a pulse.

The output won't be a uniform frequency but the downstream device is only counting pulses, not trying to sync to this as a clock.

MorganS:
I would use an interrupt for the return sensor and have that increment a variable. Then the main loop sits and waits for a pulse from the flow sensor.

Thank you MorganS - that is the sort of guidance I need.

I'll sit and do that method - it will be a while before I post the results because I am still having to look up the syntax for just about every line I type!

Once again - thanks.

peteinchad:
I think that I need to time how long it took to collect the pulses and then somehow output them over the same period of time - more reading on timing I think!

Indeed, now you do output the difference in pulses but in very fast bursts.

  digitalWrite (COUNT, HIGH);
  digitalWrite (COUNT, LOW);

is just very fast. And I don't think the meter cares about the numer of pulses but the speed of the pulse WHEN it gets pulses... So just counting the pulses and outputting the difference as fast as possible isn't going to work...

So

  1. Set up the interrups. (And you still did not tell anything about the hardware so I assume it works but I have no idea...) Instead of checking for high in the interrupt just set the interrupt to trigger on rising edges, way easier
attachInterrupt (digitalPinToInterrupt(FLOWPIN),isr_Flow , RISING);  // attach interrupt handler

//  (ISR) for FLOWPIN
void isr_Flow ()

/* Pin Change interrupt
 *
 * increments 'fuel' for every FLOW pulse detected.
 */
{
  fuel++; //all you need to do now!
}  // end of isr_Flow
  1. Just let it count for a set time. I tink like 20Hz aka 50ms is okay. Yeay, you have a delay of 50ms but does that really matter?

You can do this just with millis and the Blink without delay methode

  1. Once that triggers check the value of fuel. Save it somewhere else and reset it

  2. and for the rest of the next 50ms you have time to put out that saved amount of pulses. All you have to do is spread them out evenly over the 50ms :slight_smile:

  3. Done!

Thank you for that advice.

The hardware works OK now that I have an accurate frequency meter.

I'm glad I am on the right track with measuring the time.

It doesn't need to be super accurate as each pulse represents only 0.5ml of fuel.

Going to read up on millisec() now!!

Thanks for your help - it is much appreciated,