Problems with an "if" Statement

Hi All,

Please can i have some pointers on what i’m doing wrong with this line of code. I wish for a photon to read a resistance and check it is in range, then check to see if two other inputs are high or low/True or Fails. It compiles fine but wont work.

if (R2 > 8000 && R2 < 8500 && D5 == LOW && D6 == LOW)

I have now corrected my code to this:

if (R2 > 8000 && R2 < 8500 && digitalRead(5) == LOW && digitalRead(6) == LOW)

It seems to work now, thanks.

if (R2 > 8000 && R2 < 8500 && digitalRead(5) == LOW && digitalRead(6) == LOW)

You still want to use the proper pin names in your read functions, D5, D6.

its is still no quite right, if i change D6 to HIGH the photon goes mad.

If I set D5 & D6 to low the switches operated and the led on pin D2 operates correctly. if i set D6 to HIGH and then set the switches correctly the photon goes mad and pulses the D2 pin.
Please see Full code not sure if it is a hardware of software fault now :scream:

int   Vin       = 3.3;     //voltage at 3.3V pin of Photon

float Vout      = 0;       //voltage at A0 pin of Photon
float R1        = 8200.0;  //value of known resistance
float R2        = 0;       //value of unknown resistance
int   Edge_data = 0;    
float buffer    = 0;
float var       = R2;

int   GreenLED  = D2;
int   Relay     = D4;
int   NOTE_LED  = D7;

void setup() 
    pinMode(GreenLED, OUTPUT);
    pinMode(Relay, OUTPUT);
    pinMode(NOTE_LED, OUTPUT);
    pinMode(A1, INPUT);
    pinMode(A2, INPUT);

void loop()
    Edge_data = analogRead(A0);
        buffer = Edge_data * Vin;
        Vout   = buffer / 4095.0;
        buffer = Vout / (Vin - Vout); 
        R2     = R1 * buffer;
        // Insert your check here
        if (R2 > 8000 && R2 < 8500 && digitalRead(D5) == LOW && digitalRead(D6) == HIGH)
        //digitalRead(2) == HIGH && digitalRead(3) == HIGH)
            // In Range
            digitalWrite(GreenLED, HIGH); 
            digitalWrite(Relay, LOW);
            digitalWrite(NOTE_LED, LOW);
            digitalWrite(GreenLED, LOW);
            digitalWrite(Relay, HIGH);
            digitalWrite(NOTE_LED, HIGH);

I have cleand up your code indentation and also added some spaces to improve readiability of your code. It’s just so much nicer and easier to follow someone else’s code when it’s properly formatted.

Can you calrify what this means?

What are A1 & A2 used for? Also what is the state of D5 at that time?

Your symptom description …

… suggests that your condition goes alternates between true and false.

Another style suggestion:
You are setting some pin “constants” (D2, D4, D7) but then use other pins without such constants (A0, A1, A2).
It’s good practice to use (proper) constants (e.g. const int Relay = D4) but then do it consistently and with a consistent naming scheme too :wink:

I mean switches in the real world that correspond to the input pins D5 and D6.

I want to be able at the end to implement another “if” statement with a different resistance range and combination of D5 and D6 allowing for multiple resistors to be read based on what range is selected with the combinations of the D5 and D6 inputs.

Attached is a picture of my project so far.

If you have switches (like your dip switches) you need to prevent the respective detecting pins from floating via pull-resistors (external or via a fitting pin mode INPUT_PULLUP or INPUT_PULLDOWN).

I don’t see you set the pinMode() for D5 & D6 at all, so that will at least contribute to the issue.

It’s definetly not best style to rely on the default pin mode.

For that purpose I’d suggest some other means than a bunch of if conditions.
I’d use the respective pin states as bits in a single numeric value and use that as index for a resistor range array.

Thanks for the advise, on the pin modes, I will try that and let you know. As far as the last section on not using “if” statement instead use pin states as bits in a numeric value and then index, I would be lying if I told you I understood what you mean :worried: but am going to go away and research. Thanks :+1:

1 Like

With two switches you can select 4 different ranges, with one more 8 and so on, so this would be what I had in mind (with more than two pins, I’d opt for an array of pins tho’)

const int pinS0       = D5;          // pin for switch 0  
const int pinS1       = D6;          // pin for switch 1
const int pinR        = A0;          // pin to "read" the resistance
const int pinLED      = D7;
float     factor      = 123.456;     // for illustrational purposes only 
int       sw          = 0;           // aggregated switch setting
float     range[4][2] = 
{ {  8000,  8500 }                   //  0 switch 0b0000
, { 16000, 17000 }                   //  1        0b0001
, { 32000, 34000 }                   //  2        0b0010
, { 64000, 68000 }                   //  3        0b0011

void setup() {
  pinMode(pinLED, OUTPUT);
  pinMode(pinS0, INPUT_PULLUP);      // closing to LOW means active LOW 
  pinMode(pinS1, INPUT_PULLUP);      // hence reading needs to be inverted 
  // pinMode(pinR, INPUT);           // DON'T set pin mode for analogRead pins, 
                                     //   it's done internally

void loop() {
  float val;
  sw  = 0;
  sw |= (!pinMode(pinS0) & 1) << 0;  // inverted pin state as bit 0 
  sw |= (!pinMode(pinS1) & 1) << 1;  // inverted pin state as bit 1
  val = analogRead(pinR) * factor;
  if (range[sw][0] < val && val < range[sw][1]) {
    digitalWrite(pinLED, HIGH);
    Serial.printlnf("value %.1f lies between %.1f and %.1f", val, range[sw][0], range[sw][1]);
    digitalWrite(pinLED, LOW);


BTW, I think you won’t need all these bridges on your bread board rails. The only bridges needed are near row 33 (where the blue/red rail lines are broken) :wink:


WOW :exploding_head: thank you so much for your help!. I will have ago playing and learning your code tonight when back from work. I really appreciate your help thank you. :+1:

1 Like