Tacho- I need an output signal a specified rpm

[Cyber]

Hi All

Can I please ask for help with the following.

I have a Tacho sketch working but I want to add an output for an led to be ON between 0 and 200 rpm, and to go off at a above 200 rpm.
I have tried the following but it only works once then I have to reset power.

if(RPM>0 && RPM<200)
digitalWrite(LED, HIGH);

 

[JonSea]

You need another IF statement to turn the LED off if the rpm is greater than 200.

 

[Cyber]

You need another IF statement to turn the LED off if the rpm is greater than 200.

Hi
thanks for your quick reply. would it bee something like this:-
I am not able to
if(RPM>0 && RPM<200)
digitalWrite(LED, HIGH)

if(RPM>200 && RPM<2000)
digitalWrite(LED, LOW)

or can I just do :-

if(RPM>0)
digitalWrite(LED, HIGH)

if(RPM>200 )
digitalWrite(LED, LOW)

 

[dknguyen]

Use more brackets in your conditional expressions to prevent any ambiguity and ensure the compiler does what you want it to do:

i.e. if( (RPM > 0) && (RPM < 200) )

As for your question, doing it the second way means that the LED will always turn on for a bit before it turns off. That's a problem. Not really one in this case, but bad practice since if you were driving something other than LEDs it could cause issues. The worst thing that could happen with LEDs is the LED might be dimmer than it could be or you might perceive flashing.

How come you aren't using "if{} else if{}else{}"?

 

[Cyber]

The backets worked

I now have:-

if((RPM>0) && (RPM<200))
digitalWrite(LED, HIGH);
else
if((RPM>201) && (RPM<1000))
digitalWrite(LED, LOW);

Is this the best way to do it?

 

[Jeff Wahaus]

The backets worked

I now have:-

if((RPM>0) && (RPM<200))
digitalWrite(LED, HIGH);
else
if((RPM>201) && (RPM<1000))
digitalWrite(LED, LOW);

Is this the best way to do it?

Well, at 200 and 201 RPM neither statement would execute; try:

if((RPM>0) && (RPM<200))
digitalWrite(LED, HIGH);
else
digitalWrite(LED, LOW);

or to simplify

if (RPM < 200)
digitalWrite(LED, HIGH);
else
digitalWrite(LED, LOW);

or to be really brief:

digitalWrite(LED, (RPM < 200) ? HIGH : LOW);

 

[JonSea]

You might also want to test for RPM = 0 to turn the LED off when it's not turning.

 

[Cyber]

Well, at 200 and 201 RPM neither statement would execute; try:

if((RPM>0) && (RPM<200))
digitalWrite(LED, HIGH);
else
digitalWrite(LED, LOW);

or to simplify

if (RPM < 200)
digitalWrite(LED, HIGH);
else
digitalWrite(LED, LOW);

or to be really brief:

digitalWrite(LED, (RPM < 200) ? HIGH : LOW);

Hi Thank you for your advice, All works fine now.

I would like to go one step further and have the LED flashing at about half a second.

I have tried just inserting a delay but the led stays on longer than the set time, about 2 seconds.
adjusting the time delay has no effect.
I think it is something in the rest of the code affecting the timing.
I have also tried millis to flash the led, but the led is on all the time.

I can get the millis code working in a separate sketch ok but will not flash the led when added to my Tacho sketch.

Any Ideas please.

 

[Jeff Wahaus]

can get the millis code working in a separate sketch ok but will not flash the led when added to my Tacho sketch.
Any Ideas please.

You'll have to post the source code segment you're having trouble with.

 

[Cyber]

I have slowed speed down to try and find out the problem. altering the delay time has no effect.
The led is on until the rpm sensor is triggered on he next revolution.

 {
      if((RPM>20) && (RPM<25))
       
      digitalWrite(LED, HIGH); 
     delay(10);
      digitalWrite(LED, LOW); 
   
     
     if((RPM>25) && (RPM<100)) 
     
     digitalWrite(LED, HIGH); 
    delay(10);
     digitalWrite(LED, LOW); 
    }

 

[Jeff Wahaus]

Your eye isn't going to notice a 10ms delay, Try 500 for 1/2 a second (500ms)

 

[Cyber]

I have tried 500ms it make no differance.
The LED stays on too long,

 

[Cyber]

Is there a way to tell the pin to pulse on then off without the delay?

 

[Jeff Wahaus]

I have tried 500ms it make no differance.
The LED stays on too long,

If the LED is staying on too long then it's not a result of the code you've given above. It looks like you're missing the brackets for the scope of the "if" statements. Your code as it is will flash the LED for 1/100 of a second and you're very unlikely to even see it turn on at that speed.

Post your schematic, perhaps this isn't a software issue.

 

[Jeff Wahaus]

Is there a way to tell the pin to pulse on then off without the delay?

Yes, but there is no built-in function to do this. You'll have to design your algorithm to do this. It's generally not a good idea to have long delays in your main loop as it will stop the processing of everything else.

if you use the millis() function you can make thing happen at set intervals without blocking the rest of your program. For example

loop()
{

if (flash_the_led)
{
on_time = millis();
off_time = on_time + 500;
flash_the_led = false
digitalWrite(LED, HIGH);
}

if (millis() >= off_time)
{
flash_the_led = true;
digitalWrite(LED, LOW);
}

... /* rest of your code */

}


The above code will not block the rest of your code from executing. It's a bit more complex than using delay() but well worth the effort.

 

[Cyber]

Hi Jeff
Thanks for your advice, I have tried your code for millis, I am having trouble compiling it.

First error i get is
'flash_the_led' was not declared in this scope

So I included this in the scope
boolean UPDATE=false, DONE=true, flash_the_led=true;

the next error is
redefinition of 'boolean UPDATE'

Any idea what I am doing wrong

 

[Jeff Wahaus]

Hi Jeff
Thanks for your advice, I have tried your code for millis, I am having trouble compiling it.

First error i get is
'flash_the_led' was not declared in this scope

So I included this in the scope
boolean UPDATE=false, DONE=true, flash_the_led=true;

the next error is
redefinition of 'boolean UPDATE'

Any idea what I am doing wrong

The error just means that you're redefining UPDATE which is likely defined in some. .h file thats not really a part of your code. Just pick another name (or use lower case). By convention in the C language, all caps typically refers to a #define, not a variable. Use. Lower case or mixed case for variables.

 

[Jeff Wahaus]

The above code was just meant to illustrate the use of the millis() function. It wasn't meant to be used as is. The following is a function that should properly flash the led and can be called from your main loop and is non-blocking.

void flash_LED(bool flash)
{
static bool change_state = true;
static bool led_state = false;
static unsigned long timer_cmp = 0;

if (flash)
{
if (millis() >= timer_cmp)
change_state = true;

if (change_state)
{
if (led_state) // if LED is on
{
digitalWrite(LED, LOW); // turn it off
led_state = false;
}
else // LED is off
{
digitalWrite(LED, HIGH); // turn it on
led_state = true;
}
timer_cmp = millis() + 500; // calculate next state change
change_state = false; // indicate state changed
}
}
else
{
// No flashing so just turn LED off
digitalWrite(LED, LOW);
change_state = true;
led_state = false;
}
} // end Flash_LED()


loop()
{
flash_LED(true);
}

 

[Cyber]

Hi Jeff
I have tried your latest code, it works but the led stops flashing after about 1 minute.

Is this how it should work?

 

[Jeff Wahaus]

Hi Jeff
I have tried your latest code, it works but the led stops flashing after about 1 minute.

Is this how it should work?

No, as long as you keep calling flash_LED(true) from the main loop, the LED should continue to flash until you call flash_LED(false). The flash rate should be a consistent rate of 1/2 second on and 1/2 second off. The code assumes that it will get called from your main loop "loop()" at a reasonable rate which shouldn't be a problem unless you have something else in the loop that is blocking execution like long delay's somewhere. In a typical main loop it should be getting called 100's to 1000's of times a second but it really only needs to be called 2 times a second at a minimum.

The LED likely stopped flashing after a minute because I used an "unsigned int" rather than an "unsigned long" in the definition of timer_cmp. (now fixed above)