What am I doing wrong in this "if" statement?

Hi,
I'm wanting to count revolutions of a small flywheel using a hall effect sensor and have it control a transistor after a certain number of revolutions. Right now, I have it counting down from 5 to 0 and then resetting to 5 again but I need to be able to reset the count (labeled "hallRevCounter") manually when pin A0 goes low. I'm using a Pro Trinket 5v 16k board but I loaded my sketch on an Uno also so I could use the serial monitor to help with debugging. If I comment out the last two lines of this code, then the sketch works as expected but if I add those two lines back in my count stays at 10 and never counts down. I tapped the voltage at pin A0 and it shows 4.96 volts when high and -.02 when low on my voltmeter so that appears to be ok. I'm assuming that last "if" statement is my problem but why isn't it ignoring that statement when A0 is high? Or do I not have pin A0 set up correctly?
Thanks for taking the time to read this!
Stephen

Here's the code I'm using:

const int tripwire = 9;
const int hallSignal = 3;
const int countReset = A0;

int hallRevCounter = 5;
int revState = 0;
int lastRevState = 0;



void setup() {
  pinMode(tripwire, OUTPUT);
  pinMode(hallSignal, INPUT);
  pinMode(countReset, INPUT);
  
  digitalWrite(tripwire, HIGH);
  //Serial.begin(9600);

}

void loop() {
  revState = digitalRead(hallSignal);
  if (revState != lastRevState) 
    if (revState == HIGH) {
      hallRevCounter--;
      //Serial.println(hallRevCounter);
    }
    

    lastRevState = revState;

    if (hallRevCounter == 0) {
      digitalWrite(tripwire, LOW);
      delay(50);
      hallRevCounter = 5;
    } else {
      digitalWrite(tripwire, HIGH);
  }
   if (countReset == LOW); {
      hallRevCounter = 10;
   }
    
}

if (countReset == LOW);Why does this "if" look different to any other you've ever seen?

Why does this "if" look different to any other you've ever seen?

Look like a lot of other if statements posted on this forum, to me. Usually in posts that ask "Why doesn't my if statement work?".

Hi,

const int countReset = A0;

 if (countReset == LOW);

Hmm.. yes....
Tom.... :slight_smile:

TomGeorge:
Hi,

const int countReset = A0;

if (countReset == LOW);



Hmm.. yes....
Tom.... :)

I'm thinking this is a clue. "Low" is not an "int"? lol. I'll give it a try when I get home. I believe that's what I get for staying up too late playing. If that's my problem, thanks for the pointer! If not, I'll be back! lol
Stephen

"Low" is not an "int"?

Yes, it is. But, comparing a pin number to a state does not make sense. The pin NUMBER is not LOW. You appear to want to read the state of the pin whose number is in countReset, and compare the state to LOW.

if (digitalRead(countreset) == LOW) {

That is how I would try that line, without the semicolon you currently have. I'm far from fluent in C++ but I have been getting a lot of help and learning from people here. I know sometimes you just can't see the problem and I always appreciate it when someone lends a hand without sounding like I'm an idiot for not knowing something. Anyway, I hope this helps, lemmee know.

PaulS:
Yes, it is. But, comparing a pin number to a state does not make sense. The pin NUMBER is not LOW. You appear to want to read the state of the pin whose number is in countReset, and compare the state to LOW.

I think I get what you're saying now! I need to add another variable that I can THEN read at pin A0?

Something like:

const int tripwire = 9;
const int hallSignal = 3;
const int countReset = A0;

int hallRevCounter = 5;
int revState = 0;
int lastRevState = 0;
int reset_now = 1;
int last_reset = 1;

void setup() {
  pinMode(tripwire, OUTPUT);
  pinMode(hallSignal, INPUT);
  pinMode(countReset, INPUT);
  
  digitalWrite(tripwire, HIGH);
  Serial.begin(9600);

}

void loop() {
  revState = digitalRead(hallSignal);
  if (revState != lastRevState) 
    if (revState == HIGH) {
      hallRevCounter--;
      Serial.println(hallRevCounter);
    }
    

    lastRevState = revState;

    if (hallRevCounter == 0) {
      digitalWrite(tripwire, LOW);
      delay(50);
      hallRevCounter = 5;
    } else {
      digitalWrite(tripwire, HIGH);
  }
  reset_now = digitalRead(countReset);
  if (reset_now != last_reset)
   {if (reset_now == 0);
   {
    hallRevCounter = 10;
    //Serial.println(reset_now);
   }
   }
}

if (reset_now == 0);Read carefully

Brewskio:
if (digitalRead(countreset) == LOW) {

That is how I would try that line, without the semicolon you currently have. I'm far from fluent in C++ but I have been getting a lot of help and learning from people here. I know sometimes you just can't see the problem and I always appreciate it when someone lends a hand without sounding like I'm an idiot for not knowing something. Anyway, I hope this helps, lemmee know.

I DEFINITELY suffer from HIPS. Hidden In Plain Sight! lol

I feel fortunate that there's a community like this to help newbies like me get their projects going.

Brewskio, I tried changing just that one line like you suggested but I think PaulS has the answer. I can't compare a pin number to a state. I need a variable to read at that pin number before it's going to work.

but I think PaulS has the answer. I can't compare a pin number to a state. I need a variable to read at that pin number before it's going to work.

No. You can test the result of digitalRead() without storing it in a variable, as Brewskio showed.

Its the ; that you ; keep putting ; on the ; end of ; your if statements ; that is the ; problem.

Semicolons are not holy water. Use them where they are needed. Do not sprinkle them every where in the hopes that they will trick the compiler into doing what you want instead of what you say.

bracketracer:
I DEFINITELY suffer from HIPS. Hidden In Plain Sight! lol

I feel fortunate that there's a community like this to help newbies like me get their projects going.

Brewskio, I tried changing just that one line like you suggested but I think PaulS has the answer. I can't compare a pin number to a state. I need a variable to read at that pin number before it's going to work.

I need to update this post. What Brewskio said appears to work fine. I had a parenthesis in the wrong spot when I first tried it in addition to the extra semicolon and didn't catch my error until just now.

Now I have:

const int tripwire = 9;
const int hallSignal = 3;
const int countReset = A0;

int hallRevCounter = 5;
int revState = 0;
int lastRevState = 0;

void setup() {
  pinMode(tripwire, OUTPUT);
  pinMode(hallSignal, INPUT);
  pinMode(countReset, INPUT);

  digitalWrite(tripwire, HIGH);
  Serial.begin(9600);
}
void loop() {
  revState = digitalRead(hallSignal);
  if (revState != lastRevState)
  if (revState == HIGH) {
    hallRevCounter--;
    Serial.println(hallRevCounter);
    }
    lastRevState = revState;

if (hallRevCounter == 0) {
    digitalWrite(tripwire, LOW);
    delay(50);
    hallRevCounter = 5;
} else {
  digitalWrite(tripwire, HIGH);
}
if (digitalRead(countReset) == LOW) {
  hallRevCounter = 10;
}
}

And it resets only when I ask it to!

Thanks guys!

Thanks guys!

Now if you could just work on your curly brace positioning (all by themselves on a line of their own) and indenting (Tools + Auto Format to the rescue), your code could look good as well as being functional.

I'll give it a try! Thanks for the tip!

bracketracer:
I'll give it a try! Thanks for the tip!

And adding some comments in there wouldn't hurt. So you know what the code is doing when you look at it again a year from now. (Something on an trying to get myself to do.)

Hi,
Its only a personal preference, but I like to put any { and } associated with decision statements on their own line.
The Auto Format lines the pairs up underneath each other.

const int tripwire = 9;
const int hallSignal = 3;
const int countReset = A0;

int hallRevCounter = 5;
int revState = 0;
int lastRevState = 0;

void setup() {
  pinMode(tripwire, OUTPUT);
  pinMode(hallSignal, INPUT);
  pinMode(countReset, INPUT);

  digitalWrite(tripwire, HIGH);
  Serial.begin(9600);
}
void loop() {
  revState = digitalRead(hallSignal);
  if (revState != lastRevState)
  {
    if (revState == HIGH)
    {
      hallRevCounter--;
      Serial.println(hallRevCounter);
    }
  }
  lastRevState = revState;

  if (hallRevCounter == 0)
  {
    digitalWrite(tripwire, LOW);
    delay(50);
    hallRevCounter = 5;
  }
  else
  {
    digitalWrite(tripwire, HIGH);
  }
  if (digitalRead(countReset) == LOW)
  {
    hallRevCounter = 10;
  }
}

Tom... :slight_smile: