Can you spot the bug in this series of 'If' statements?

I've attached my section of code. It's small and seems straightforward and simple, yet when I upload and test it, it isn't perfect :confused:
Basically, I want my LED strip lights to stay completely off when the ambient light is fairly bright, and then in between a certain threshold of lighting level I want the LEDs to begin dimming on in correlation to the decreasing ambient light. Once the ambient light gets to a certain level of darkness, I want the LEDs to be on full regardless as well.

My code works to a degree, however the small bug that makes it imperfect is that when I begin dimming the light, the LEDs turn on and begin dimming at the correct moment but in that threshold that they are supposed to dim on (300-700) they increase to full brightness at about halfway through that and then start over at off at about the halfway point and then continue to increase in brightness again until I get to the next 'if' statement (700).
In short, that 300-700 threshold that they are supposed to gradually dim on within is somehow split into 2 where it does the full run and then restarts halfway through at about 500??

Been trying to solve this for a while (originally had an 'If'-'Else If'-'Else' statement, but it did the same thing. Also tried rearranging the order of 'If' statements.)

Any insight would be greatly appreciated!

Please don't post your code in image files. Place it in your post, inside code tags. </> in the 'Post' window.

And post your complete code, not just a couple of lines.

Unless you're on a Due, analogWrite PWM is eight bit.

It's not your "if" s, but your arithmetic that's the problem

Sorry, I didn't realize that was an option. Just copying and pasting text didn't look very organized, and I only posted the portion that my question was about. Here's the full code I have so far for my project, if it helps...

int PowerLED = 6;              // the pin that the power LED is attached to
int StripLED = 9;              // the pin that the strip LED is attached to
int LightSensor = A0;           // the pin that the Light Sensor is attached to
int MotionDirection1 = 10;      // the first pin that the Motion Sensor is attached to
int MotionDirection2 = 11;      // the second pin that the Motion Sensor is attached to
int MotionDirection3 = 12;      // the third pin that the Motion Sensor is attached to 
int MotionDirection4 = 13;      // the fourth pin that the Motion Sensor is attached to
int LowBattery = 4;             // Pin that the low battery signal would be sent to
boolean DeadBatt = true ;
int LightIntensity = analogRead(LightSensor);   //Light sensor reads amount of ambient light

// the setup routine runs once when you press reset:
void setup() {

  pinMode(PowerLED, OUTPUT);        //Declare pin 6 to be an output,
  pinMode(StripLED, OUTPUT);        //Declare pin 9 to be an output,
  pinMode(LightSensor, INPUT);      //Declare pin A0 to be an input,
  pinMode(MotionDirection1, INPUT); //Declare pin 10 to be an input,
  pinMode(MotionDirection2, INPUT); //Declare pin 11 to be an input,
  pinMode(MotionDirection3, INPUT); //Declare pin 12 to be an input,
  pinMode(MotionDirection4, INPUT); //Declare pin 13 to be an input,
  pinMode(LowBattery, INPUT);       //Declare pin 4 to be an input,
  analogWrite(PowerLED, 5);         //Turns the power light on
  Serial.begin(9600);               //Establish communication between the computer and Arduino
  digitalWrite(StripLED,HIGH),  delay(3000);
  digitalWrite(StripLED,LOW);
 

}

// the loop routine runs over and over again forever:
void loop() {
  
Serial.print(LightIntensity);                   //Updates the serial monitor with light readings
delay(1);                                       //How often it takes/updates the readings

if (LightIntensity <= 300) {                            
//If the ambient light is still fairly bright (less than 300), the LEDs won't turn on
       digitalWrite(StripLED, LOW);
} 

if ((LightIntensity > 300) && (LightIntensity < 700)) {
      analogWrite(StripLED, 255*LightIntensity/700);    
//this ratio will begin to dim on the LEDs within the 300-700 threshold of the photoresistor's readings
}

if (LightIntensity >= 700) {                            
//If it is quite dark (more than 700) the LEDs will remain on full
       digitalWrite(StripLED, HIGH);
}

if  (digitalRead(LowBattery) == LOW && DeadBatt == true )  {
      digitalWrite(StripLED, LOW), delay(500);        
//If the battery is low, blink the LED lights on and off for 0.5 seconds 10 times.
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
      digitalWrite(StripLED, LOW), delay(500);
      digitalWrite(StripLED, HIGH), delay(500);
          DeadBatt = false;                          //The dead battery boolean starts as True, but at the end of these if statement executions it turns it off again.
}

}

The arithmetic makes sense doesn't it? It's a basic ratio of x/255 = 300/700, so x= 255*(300/700). x would be the value of output to StripLED out of the full 255 that I could do (full 5V). Please explain.

in integer arithmetic 300/700 is 0. so X * (300/700) is 0, you want (X*300) / 700

Where does LightIntensity get read? I assume that is coming from an analogRead but I dont see one in your loop. The one you have at global scope won't work, and it needs to be done frequently if you want it to be updated at any point.

Either way analogRead returns 10 hits so your math is off. If you want it out of 255 then you have to divide it by 4.

Why not simply print the values out, and see exactly what causes the problem?

Delta_G:
Where does LightIntensity get read? I assume that is coming from an analogRead but I dont see one in your loop. The one you have at global scope won't work, and it needs to be done frequently if you want it to be updated at any point.

Either way analogRead returns 10 hits so your math is off. If you want it out of 255 then you have to divide it by 4.

Ok, that would make sense. My analogRead for Light Intensity is happening globally at the beginning. So do you think it's necessary to throw this same analogRead maybe before each of those If statements just so it's frequently updated? Also, you're saying to divide that middle If statement equation by 4?
I'm only 1 week in to arduino coding, so I'm completely new to it. I've done SO much research this past week, but it still takes a bit to understand :stuck_out_tongue:
Thanks!

My analogRead for Light Intensity is happening globally at the beginning.

Are you expecting it to change when the program is running ? If so, then you are going to be disappointed.

My analogRead for Light Intensity is happening globally at the beginning.

No NO NO :wink:

Put it in loop()

KeithRB:
in integer arithmetic 300/700 is 0. so X * (300/700) is 0, you want (X*300) / 700

This actually makes sense as well. During my research on Arduino coding this week, I read that Arduino only reads integers (whole numbers), so true, 300/700 would be a decimal/percentage and would therefore not work with Arduino. Your equation would correct this and so I'll also give it a try!
Thanks

Since 300/700 is a constant, calculate the value once, as a float, and multiply. Multiplication is faster than division. Even integer division.

Although, the equation isn't actually 300/700, that's just to show the ratio that x/255 is equal to. In the sketch it was 255*LightIntensity/700 and the value for LightIntensity will be changing constantly.

However, thinking about it now, 300/700 = 0.42 but when LightIntensity=300 I don't want it to already be showing at 42% LED brightness (109 out of 255)... I'd almost have to work in just that difference?? so instead, I'd do 1/400!

So when, say, LightIntensity = 350, I would have to do 255*50/400, making it only show 12.5% of 255 (32 out of 255). That makes sense!!! :smiley:

Also, even globally, my value from LightIntensity was constantly updating as I could see from the serial monitor and my LEDs were turning on (gradually) correctly. There was just a break halfway through when dimming the ambient light. But in any case, this math makes more sense now.

You can use map() to map a analog reading from any range (300 to 700, for instance) to any other range (0 to 255, for instance).

PaulS:
You can use map() to map a analog reading from any range (300 to 700, for instance) to any other range (0 to 255, for instance).

Interesting, I haven't heard of the map() function yet. I looked it up and is this how I would include it in the context of my sketch as the execution of that second If-statement? I could create a new variable called 'Dimming' which would be the power to output to my LED pin:

if (LightIntensity > 300) && (LightIntensity < 700) {
int Dimming = map(LightIntensity, 301,699,1,254);
analogWrite(StripLED, Dimming);
}

my value from LightIntensity was constantly updating

I don't see how it could vary in the code that you posted.

Autumnslash:
Interesting, I haven't heard of the map() function yet. I looked it up and is this how I would include it in the context of my sketch as the execution of that second If-statement? I could create a new variable called 'Dimming' which would be the power to output to my LED pin:

if (LightIntensity > 300) && (LightIntensity < 700) {
int Dimming = map(LightIntensity, 301,699,1,254);
analogWrite(StripLED, Dimming);
}

If you want an output from fully off to fully on, you're close:-

if (LightIntensity > 300) && (LightIntensity < 700){
  int Dimming = map(LightIntensity, 301,699,0,255);
  analogWrite(StripLED, Dimming);
}

N.B. You could use a byte variable instead of int for 'Dimming', too, to conserve RAM, since it will only hold a value from 0 to 255.

This stands out to me:

255*LightIntensity/700

C will multiply the light intensity by 255 and then divide by 700. But your light intensity might be as much as 700. This means it will overflow the two-byte int. This kind of overflow will cause the kind of thing you are seeing, values that should be scalling smoothy tipping over an edge.

Simple fix, use long ints for multiplication, then trim:

(int)( 255L*LightIntensity/700L)

is this how I would include it in the context of my sketch

Yes, it is.