Suggestion for my code

Hello.
Maybe i could get some advices how to improve this code. Working principle should be:
If Value is less than ..., then led should light up and shine for specific time. After that it should turn off.

`
const int ledPin = 4;
int ldr=A0;//Set A0(Analog Input) for LDR.
int value=0;

int ledState = LOW;

unsigned long previousMillis = 0;

const long interval = 2000;
void setup() {

pinMode(ledPin, OUTPUT);
}

void loop() {
value=analogRead(ldr);//Reads the Value of LDR(light).

if(value<300 ){
digitalWrite(ledPin, HIGH);
unsigned long currentMillis = millis();

if(currentMillis - previousMillis >= interval)
{

	digitalWrite(ledPin,LOW);
previousMillis = currentMillis;

}

}
}

`

Please post your code in code tags.

This:

const long interval = 2000;

Should be:

const unsigned long interval = 2000;

If value is < 300 the light will always be on the way you have it coded. Also if the value is < 300 and then the value is >= 300 the light may not turn off.

It is likely that once the light turns on then it will stay on.

Ok.
So how i should correct it ?

It depends. Do you want the LED to always be on if the value is <300? If not, do you want the LED to turn off after the interval and not turn back on until value is >= 300 and then less than 300 again?

If value is less than 300, led should turn on for some time after that time it should turn off or it should turn off if value is >= 300

OK. When it turns off after being on should it ever turn back on again?

Yes. If value becomes more >=300 and after that it becomes again <300

OK. Then you have to remember the previous value of value. The problem always seems simple until it is fully defined.

This works. It could be optimized. I used a threshold to detect a change in the analog value since analogs tend to bounce.

const int ledPin = 4;
const int ldr = A0; //Set A0(Analog Input) for LDR.
const unsigned long interval = 2000;
const int threshold = 10;

int value = 0;
unsigned long previousMillis = 0;

void setup() 
{
  pinMode(ledPin, OUTPUT);
}

void loop() 
{
  unsigned long currentMillis = millis();
  static int prevValue = 2000;
  value = analogRead(ldr); //Reads the Value of LDR(light).
    
  if (abs(value - prevValue) > threshold)
  {
    if (value < 300 && prevValue >= 300) 
    {
      digitalWrite(ledPin, HIGH);
      previousMillis = currentMillis;
    }
    else if (value >= 300)
    {
      digitalWrite(ledPin, LOW);
    }
    prevValue = value;
  }
  if ((digitalRead(ledPin) == HIGH) && (currentMillis - previousMillis >= interval))
  {
    digitalWrite(ledPin, LOW);
  }
}

Thank you very much. I will need some time to study this code, but it works (i checked).

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.