My code isn't doing what I think it should.

Hello,

I was having an issue on a larger project, and created the following to try to narrow down the cause with minimal extra complication. What I'd expect to happen is that the light turns on when the test button is pressed (test pin goes low) and then turns off when the reset button is pressed (reset button pin goes low). Note that this is not the reset button on the arduino, but would be seen by the user as "reset the test". The light should not come on again until the test pin goes low again.

What's actually happening is that the light is only off when the test reset pin is being held low, and immediately comes back on again when it is released.

"Light, HIGH" only appears once, and it's inside the if condition of (TestPassed == true). TestPassed is set to false in the ISR, and only set to be true when the test pin is read to be low, and yet the light is turning back on even when the test pin is connected to the 5V rail with a jumper wire.

bool LightClearRequested = false;
bool TestPassed = false;

int ResetButton = 3;
int Light = 11;

int TestPin = 8;



void Reset() {

  LightClearRequested = true;
  TestPassed = false;

}

void setup() {

pinMode(TestPin, INPUT_PULLUP);
pinMode(ResetButton, INPUT_PULLUP);
pinMode(Light, OUTPUT);
  
attachInterrupt(digitalPinToInterrupt(ResetButton), Reset, LOW); //Sets up interrupt to call Reset ISR when ResetButton is low.
}

void loop() {

if (LightClearRequested == true)
{
  digitalWrite(Light, LOW);
  LightClearRequested = false;
}

if (TestPassed == true)
  {
    digitalWrite(Light, HIGH);
  }
  
 


if (digitalRead(TestPin == LOW))

    {
      TestPassed = true;
  }
  else
  {
    TestPassed = false;
  }
  

}

I haven't done any coding in quite a while, I'd appreciate if someone could point to what I'm doing wrong here, or any flaws in my reasoning.

Thanks,

Chris

volatile bool LightClearRequested;
volatile bool TestPassed;

(deleted)

Hi there,

If the button you are talking about is an external mechanical button then it is probable that cases of bouncing maybe occurring. If this the case then it can be solved by both ways hardware and software. There is much content on internet about debouncing. But I am not totally sure that it is main problem or not.

Raj

spycatcher2k:
Oops, try

if (digitalRead(TestPin) == LOW)

It works! Thank you. I need to read up more on exactly how parentheses work. I must admit there have been times where I've added them around stuff until the compiler stopped shouting at me, but I've got away with it thus far.

If I understand correctly the if condition for the test pin was always being satisfied, and thus TestPassed was always being reset to true, regardless of the actual state of the pin. I'd inadvertently instructed it to skip the actual test, or at least disregard the result of it.

I tried to figure out exactly what was going on by having it print serial data at each significant point, and it does seem that it was evaluating the digitalRead TestPin as true every time.

TheMemberFormerlyKnownAsAWOL:

volatile bool LightClearRequested;

volatile bool TestPassed;

I tried this with and without volatile bool and didn't see any change, but thanks for pointing it out. I'm not entirely clear on the situations when it would be necessary, but it looks useful.

Chris

“ I need to read up more on exactly how parentheses work. ”

You should spend some time in the reference section.

https://www.arduino.cc/reference/en/

Because the expression in an 'if' statement is a boolean expression, you can simplify your sketch a bit:

void loop()
{
  if (LightClearRequested)  // No need for "== true" when testing a boolean expression
  {
    digitalWrite(Light, LOW);
    LightClearRequested = false;
  }


  if (TestPassed)  // No need for "== true" when testing a boolean expression
  {
    digitalWrite(Light, HIGH);
  }
  // No need use an 'if' statement to store a boolean expression in a boolean variable.
  TestPassed = digitalRead(TestPin) == LOW;
}

Chris935:
I need to read up more on exactly how parentheses work.

function()
function(argument)
function(argument, argument2)   - function calls
(expression)                            - just parenthesized expression
(int) foo                               - cast to int.

And there's the special syntax for for-loops:

for (declarations ; condition ; stepping_code)