Second If only tested when first If is true

Hey,
I'm trying to get an IR tester to work. Principle is easy, I'll leave it running and if he recieves a specific IR signal, an LED turns on and stays on. When pressing an reset switch, the LED turns of again.

Problem is:
When the tester recieves the signel, the LED turns on.
When I press the switch after the LED turned on, it doesn't turn it off.
Only then pressing the switch, while the signal is recieved, the LED turns off.

It looks like, the second If is only tested, as long the first If is true.

The part in question:

void loop() {
  // Wait for an IR Code
  numberpulses = listenForIR();
  Reset = digitalRead(ResetPin);

  if (IRcompare(numberpulses, RedButton, sizeof(RedButton) / 4)) {
    digitalWrite(SwitchPin, HIGH);
    delay(100);
    
  }

  if ((Reset == HIGH) && (SwitchPin == HIGH)) {
    digitalWrite(SwitchPin, LOW);
    Serial.print("Reset");
    delay(100);
    
  }

}

Whole Code:

// SPDX-FileCopyrightText: 2018 Limor Fried/ladyada for Adafruit Industries
//
// SPDX-License-Identifier: CC-BY-SA-3.0

/* Trinket/Gemma compatible Raw IR decoder sketch
This sketch/program uses an Adafruit Trinket or Gemma
ATTiny85 based mini microcontroller and a PNA4602 to
decode IR received. This can be used to make a IR receiver
(by looking for a particular code) or transmitter 
(by pulsing an IR LED at ~38KHz for the durations pulse_index

Based on Adafruit tutorial http://learn.adafruit.com/ir-sensor/using-an-ir-sensor

and ATTiny program by TinyPCRemote Nathan Chantrell http://nathan.chantrell.net
under Creative Commons Attribution-ShareAlike 3.0 Unported (CC BY-SA 3.0) license

SendSoftwareSerial Lirary modification by Nick Gammon from NewSoftwareSerial code
GNU Lesser General Public License as published by the Free Software Foundation version 2.1 
at http://gammon.com.au/Arduino/SendOnlySoftwareSerial.zip
*/

// We need to use the 'raw' pin reading methods because timing is very important here
// and the digitalRead() procedure is slower!

#define IRpin_PIN PIND  // ATTiny85 had Port B pins
#define IRpin 2     //IR Reciever Pin
#define SwitchPin 3   //Pin switched by Signal
#define ResetPin 4    //Pin to Reset SwitchPin

#define MAXPULSE 65000  // the maximum pulse we'll listen for - 5 milliseconds
#define NUMPULSES 50    // max IR pulse pairs to sample
#define RESOLUTION 20   // time between IR measurements
#define FUZZINESS 25    // What percent we will allow in variation to match the same code
#define HIGH 0x1
#define LOW 0x0

// we will store up to 100 pulse pairs (this is -a lot-)
uint16_t pulses[NUMPULSES][2];  // high and low pulses
uint16_t pulse_index = 0;       // index for pulses we're storing
uint16_t numberpulses = 0;

int Reset = 0;

#include "signalsample.h"

void setup() {
  pinMode(IRpin, INPUT);
  pinMode(SwitchPin, OUTPUT);
  pinMode(ResetPin, INPUT);
  Serial.begin(9600);
}

void loop() {
  // Wait for an IR Code
  numberpulses = listenForIR();
  Reset = digitalRead(ResetPin);

  if (IRcompare(numberpulses, RedButton, sizeof(RedButton) / 4)) {
    digitalWrite(SwitchPin, HIGH);
    delay(100);
    
  }

  if ((Reset == HIGH) && (SwitchPin == HIGH)) {
    digitalWrite(SwitchPin, LOW);
    Serial.print("Reset");
    delay(100);
    
  }

}

//KGO: added size of compare sample. Only compare the minimum of the two
boolean IRcompare(int numbpulses, int Signal[], int refsize) {

  for (int i = 0; i < numbpulses - 1; i++) {
    int oncode = pulses[i][1] * RESOLUTION / 10;
    int offcode = pulses[i + 1][0] * RESOLUTION / 10;

    // check to make sure the error is less than FUZZINESS percent
    if (abs(oncode - Signal[i * 2 + 0]) <= (Signal[i * 2 + 0] * FUZZINESS / 100)) {
    } else {
      return false;
    }

    if (abs(offcode - Signal[i * 2 + 1]) <= (Signal[i * 2 + 1] * FUZZINESS / 100)) {
    } else {
      return false;
    }
  }
  return true;
}

uint16_t listenForIR() {  // IR receive code
  pulse_index = 0;
  while (1) {
    unsigned int highpulse, lowpulse;  // temporary storage timing
    highpulse = lowpulse = 0;          // start out with no pulse length

    while (IRpin_PIN & _BV(IRpin)) {  // got a high pulse
      highpulse++;
      delayMicroseconds(RESOLUTION);
      if (((highpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][0] = highpulse;

    while (!(IRpin_PIN & _BV(IRpin))) {  // got a low pulse
      lowpulse++;
      delayMicroseconds(RESOLUTION);
      if (((lowpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][1] = lowpulse;
    pulse_index++;
  }
}

The code is an abdomination from multiple examples and my beginner coding and is normally for an Adafruit Trinket but I have a ported Version for my Uno clone for debugging.
The signalsample.h is what the name says, the sample of the signal which triggers the LED.

Welcome to the forum

Please post the full sketch

  if ((Reset == HIGH) && (SwitchPin == HIGH))

Where and how is SwitchPin declared ?
Will it ever be anything but HIGH ?
Should there be a digitalRead() in there somewhere ?

Please post the full sketch

The lower code is the full sketch

Where and how is SwitchPin declared ?
Will it ever be anything but HIGH ?
Should there be a digitalRead() in there somewhere ?

My bad, that was a test code.
forget the "&& (SwitchPin == HIGH))" part, thats not part of the "real" code.

You can't use pin 0 on Uno, it's reserved for use uploading your code and using Serial monitor.

Please post the real code

Ok, never post anything other than the real code. That is wasting the forum's time.

Post the real code.

Doesn't "int reset = 0" define a variable "reset" with the value "0"?

As far as i've learnd, you define a Pin with "#define ResetPin 0", or am I wrong?

The real code is as the first posted code but without the && (SwitchPin == HIGH)), I just didn't notice I haven't deleted that yet. My Bad

The Code as is:

// SPDX-FileCopyrightText: 2018 Limor Fried/ladyada for Adafruit Industries
//
// SPDX-License-Identifier: CC-BY-SA-3.0

/* Trinket/Gemma compatible Raw IR decoder sketch
This sketch/program uses an Adafruit Trinket or Gemma
ATTiny85 based mini microcontroller and a PNA4602 to
decode IR received. This can be used to make a IR receiver
(by looking for a particular code) or transmitter 
(by pulsing an IR LED at ~38KHz for the durations pulse_index

Based on Adafruit tutorial http://learn.adafruit.com/ir-sensor/using-an-ir-sensor

and ATTiny program by TinyPCRemote Nathan Chantrell http://nathan.chantrell.net
under Creative Commons Attribution-ShareAlike 3.0 Unported (CC BY-SA 3.0) license

SendSoftwareSerial Lirary modification by Nick Gammon from NewSoftwareSerial code
GNU Lesser General Public License as published by the Free Software Foundation version 2.1 
at http://gammon.com.au/Arduino/SendOnlySoftwareSerial.zip
*/

// We need to use the 'raw' pin reading methods because timing is very important here
// and the digitalRead() procedure is slower!

#define IRpin_PIN PIND  // ATTiny85 had Port B pins
#define IRpin 2     //IR Reciever Pin
#define SwitchPin 3   //Pin switched by Signal
#define ResetPin 4    //Pin to Reset SwitchPin

#define MAXPULSE 65000  // the maximum pulse we'll listen for - 5 milliseconds
#define NUMPULSES 50    // max IR pulse pairs to sample
#define RESOLUTION 20   // time between IR measurements
#define FUZZINESS 25    // What percent we will allow in variation to match the same code
#define HIGH 0x1
#define LOW 0x0

// we will store up to 100 pulse pairs (this is -a lot-)
uint16_t pulses[NUMPULSES][2];  // high and low pulses
uint16_t pulse_index = 0;       // index for pulses we're storing
uint16_t numberpulses = 0;

int Reset = 0;

#include "signalsample.h"

void setup() {
  pinMode(IRpin, INPUT);
  pinMode(SwitchPin, OUTPUT);
  pinMode(ResetPin, INPUT);
  Serial.begin(9600);
}

void loop() {
  // Wait for an IR Code
  numberpulses = listenForIR();
  Reset = digitalRead(ResetPin);

  if (IRcompare(numberpulses, RedButton, sizeof(RedButton) / 4)) {
    digitalWrite(SwitchPin, HIGH);
    delay(100);
    
  }

  if (Reset == HIGH) {
    digitalWrite(SwitchPin, LOW);
    Serial.print("Reset");
    delay(100);
    
  }

}

//KGO: added size of compare sample. Only compare the minimum of the two
boolean IRcompare(int numbpulses, int Signal[], int refsize) {

  for (int i = 0; i < numbpulses - 1; i++) {
    int oncode = pulses[i][1] * RESOLUTION / 10;
    int offcode = pulses[i + 1][0] * RESOLUTION / 10;

    // check to make sure the error is less than FUZZINESS percent
    if (abs(oncode - Signal[i * 2 + 0]) <= (Signal[i * 2 + 0] * FUZZINESS / 100)) {
    } else {
      return false;
    }

    if (abs(offcode - Signal[i * 2 + 1]) <= (Signal[i * 2 + 1] * FUZZINESS / 100)) {
    } else {
      return false;
    }
  }
  return true;
}

uint16_t listenForIR() {  // IR receive code
  pulse_index = 0;
  while (1) {
    unsigned int highpulse, lowpulse;  // temporary storage timing
    highpulse = lowpulse = 0;          // start out with no pulse length

    while (IRpin_PIN & _BV(IRpin)) {  // got a high pulse
      highpulse++;
      delayMicroseconds(RESOLUTION);
      if (((highpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][0] = highpulse;

    while (!(IRpin_PIN & _BV(IRpin))) {  // got a low pulse
      lowpulse++;
      delayMicroseconds(RESOLUTION);
      if (((lowpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][1] = lowpulse;
    pulse_index++;
  }
}

By bad, you are correct.

It only becomes a pin definition if used to do something to a pin, otherwise its just a constant used in the sketch for whatever.

How to you have the reset switch wired?

The Switch has a 10k Ohm pull-down resistor.
The Led has a 220 Ohm resistor

That code still has this line in it

  if ((Reset == HIGH) && (SwitchPin == HIGH))

and

#define SwitchPin 3     //Pin switched by Signal

So, I repeat

OK, I usually ignore comments, but if this is literally true

  // Wait for an IR Code
  numberpulses = listenForIR();

your sketch is blocking until it receives.

Since loop(), um, loops, you can't just move the reset logic - blocking for reception means nothing else gets done, no button is read much less acted upon.

So out on a limb assuming that is true, you'll need to find a non-blocking receiving function or give up on having a reset button the way you do now.

It may be that this is a valid case for using an interrupt, which would in its interrupt service routine merely turn the LED off.

Any blocking will affect your ability to grow this sketch and have it perform all its functions smoothly. My preference in this case would be to find a way around the blocking.

A quick look didn't turn up the library for a look-see. Can you post a link to it?

# include "signalsample.h"

a7

Is the whole loop blocked by the "numberpulses = listenForIR();" or just the part after that?

The signalsample.h is a selfmade library, with the previosly decoded IR signal. Thats why you can't find it online.
Here the signalsample.h

int RedButton[] = {
	414, 394,
	60, 194,
	50, 202,
	58, 192,
	60, 194,
	56, 94,
	58, 94,
	58, 94,
	56, 96,
	56, 94,
	58, 94,
	56, 94,
	58, 94,
	56, 96,
	54, 96,
	58, 94,
	56, 96,
	54, 196,
	56, 196,
	54, 198,
	56, 196,
	50, 204,
	54, 196,
	56, 196,
	54, 200,
	54, 1010,
	410, 398,
	56, 196,
	48, 204,
	56, 196,
	56, 196,
	58, 94,
	56, 96,
	56, 94,
	58, 94,
	56, 98,
	54, 98,
	52, 98,
	54, 98,
	54, 96,
	54, 98,
	54, 98,
	54, 94,
	56, 196,
	54, 200,
	54, 196,
	56, 198,
	48, 204,
	54, 196,
	56, 0};

That code still has this line in it

I've double checked the code and can't finde the " if ((Reset == HIGH) && (SwitchPin == HIGH))" in the code in Post#8 anymore.

Thats a copy of the code from Post#8:

void loop() {
  // Wait for an IR Code
  numberpulses = listenForIR();
  Reset = digitalRead(ResetPin);

  if (IRcompare(numberpulses, RedButton, sizeof(RedButton) / 4)) {
    digitalWrite(SwitchPin, HIGH);
    delay(100);
    
  }

  if (Reset == HIGH) {
    digitalWrite(SwitchPin, LOW);
    Serial.print("Reset");
    delay(100);
    
  }

}

Yeth. That's why I said loop() loops! I actually thought briefly that moving the button test above the blockage would work. Briefly.

If loop does A, B, C, A, B, C, A, B, C, A, B, C, A, B, C, you can see it doesn't matter except for the very first loop what order those are in. If B blocks, it blocks. After any startup (which of A, B or C is first, very first to execute,) the loop ensures that we get A, B, C indefinitely.

To see if that function does indeed block, it will be helpful to see signalsample.cpp or wherever the function code is.

Or you could put a print statement before and after, viz:

  Serial.println("waiting for am IR code...");
  numberpulses = listenForIR();
  Serial.println("        and we have one!");

If that function doesn't block, you'll get a crap-ton of output, and we look elsewhere for the issue. If instead you only get one line

waiting for am IR code...

and everything seems to stop, it will be because... from the level you are working, everything has. Stopped. listenForIR() is hogging all the processor time.

a7

You are correct. My apologies to you

It may be that this is a valid case for using an interrupt, which would in its interrupt service routine merely turn the LED off.

Thank you! The interrupt is the answer to my question. I've implemented an interrupt, that sets the LED Pin to LOW.

// SPDX-FileCopyrightText: 2018 Limor Fried/ladyada for Adafruit Industries
//
// SPDX-License-Identifier: CC-BY-SA-3.0

/* Trinket/Gemma compatible Raw IR decoder sketch
This sketch/program uses an Adafruit Trinket or Gemma
ATTiny85 based mini microcontroller and a PNA4602 to
decode IR received. This can be used to make a IR receiver
(by looking for a particular code) or transmitter 
(by pulsing an IR LED at ~38KHz for the durations pulse_index

Based on Adafruit tutorial http://learn.adafruit.com/ir-sensor/using-an-ir-sensor

and ATTiny program by TinyPCRemote Nathan Chantrell http://nathan.chantrell.net
under Creative Commons Attribution-ShareAlike 3.0 Unported (CC BY-SA 3.0) license

SendSoftwareSerial Lirary modification by Nick Gammon from NewSoftwareSerial code
GNU Lesser General Public License as published by the Free Software Foundation version 2.1 
at http://gammon.com.au/Arduino/SendOnlySoftwareSerial.zip
*/

// We need to use the 'raw' pin reading methods because timing is very important here
// and the digitalRead() procedure is slower!

#define IRpin_PIN PIND  // ATTiny85 had Port B pins
#define IRpin 2     //IR Reciever Pin
#define SwitchPin 4   //Pin switched by Signal
#define ResetPin 3    //Pin to Reset SwitchPin

#define MAXPULSE 65000  // the maximum pulse we'll listen for - 5 milliseconds
#define NUMPULSES 50    // max IR pulse pairs to sample
#define RESOLUTION 20   // time between IR measurements
#define FUZZINESS 25    // What percent we will allow in variation to match the same code
#define HIGH 0x1
#define LOW 0x0

// we will store up to 100 pulse pairs (this is -a lot-)
uint16_t pulses[NUMPULSES][2];  // high and low pulses
uint16_t pulse_index = 0;       // index for pulses we're storing
uint16_t numberpulses = 0;

#include "signalsample.h"

void setup() {
  pinMode(IRpin, INPUT);
  pinMode(SwitchPin, OUTPUT);
  pinMode(ResetPin, INPUT);
  attachInterrupt(1, resetfunction, CHANGE);
  Serial.begin(9600);
}

void loop() {
  // Wait for an IR Code
  Serial.println("waiting for am IR code...");
  numberpulses = listenForIR();
  Serial.println("        and we have one!");

  if (IRcompare(numberpulses, RedButton, sizeof(RedButton) / 4)) {
    digitalWrite(SwitchPin, HIGH);
    delay(100);
    
  }

}

//KGO: added size of compare sample. Only compare the minimum of the two
boolean IRcompare(int numbpulses, int Signal[], int refsize) {

  for (int i = 0; i < numbpulses - 1; i++) {
    int oncode = pulses[i][1] * RESOLUTION / 10;
    int offcode = pulses[i + 1][0] * RESOLUTION / 10;

    // check to make sure the error is less than FUZZINESS percent
    if (abs(oncode - Signal[i * 2 + 0]) <= (Signal[i * 2 + 0] * FUZZINESS / 100)) {
    } else {
      return false;
    }

    if (abs(offcode - Signal[i * 2 + 1]) <= (Signal[i * 2 + 1] * FUZZINESS / 100)) {
    } else {
      return false;
    }
  }
  return true;
}

uint16_t listenForIR() {  // IR receive code
  pulse_index = 0;
  while (1) {
    unsigned int highpulse, lowpulse;  // temporary storage timing
    highpulse = lowpulse = 0;          // start out with no pulse length

    while (IRpin_PIN & _BV(IRpin)) {  // got a high pulse
      highpulse++;
      delayMicroseconds(RESOLUTION);
      if (((highpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][0] = highpulse;

    while (!(IRpin_PIN & _BV(IRpin))) {  // got a low pulse
      lowpulse++;
      delayMicroseconds(RESOLUTION);
      if (((lowpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][1] = lowpulse;
    pulse_index++;
  }
}

void resetfunction() {
  digitalWrite(SwitchPin, LOW);
}

To see if that function does indeed block, it will be helpful to see signalsample .cpp or wherever the function code is.

As far as I know, the signalsample.h is all that there is. I've got the values in the sample.h with an other script, that just analyses the recieved IR signal.

The function "listenForIR" is defined lower in the whole code:

uint16_t listenForIR() {  // IR receive code
  pulse_index = 0;
  while (1) {
    unsigned int highpulse, lowpulse;  // temporary storage timing
    highpulse = lowpulse = 0;          // start out with no pulse length

    while (IRpin_PIN & _BV(IRpin)) {  // got a high pulse
      highpulse++;
      delayMicroseconds(RESOLUTION);
      if (((highpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][0] = highpulse;

    while (!(IRpin_PIN & _BV(IRpin))) {  // got a low pulse
      lowpulse++;
      delayMicroseconds(RESOLUTION);
      if (((lowpulse >= MAXPULSE) && (pulse_index != 0)) || pulse_index == NUMPULSES) {
        return pulse_index;
      }
    }
    pulses[pulse_index][1] = lowpulse;
    pulse_index++;
  }
}

Indeed it is... Sry sry sry. I will slap myself with a damp trout as required by forum rules. :expressionless:

In Arduino sketches, including a file is usually a sign of using code that gets dragged in from a library, so I defend the indefensible - I just didn't think to look for it in "your" code.

For all the screaming we do about posting all the code...

My beach buddy is rolling towards me now. Cold (-ish), sunny and almost no wind and the crew will be there. In the time I gave it just now, that function does indeed look like it blocks.

a7