Turning on pin if something is active for x amount of time not working

Hello, I have some code here that i have tried re-writing multiple times, and it still doesn't seem to be doing what I'd like. The point is to activate pin 3 after light hits a sensor for x seconds (5 in this case). I have a feeling the order of my if statements might be the culprit, but any input would be much appreciated.

I know that the wiring is okay because the part of the program that is controlled by the button works just fine.

// Label pins
int lightSensorPin = A0;
int buttonPin = 2;
int mosfetPin = 3;
int ledPin = 13;

// Initialize main variables
int light = 80;
int dark = 1;
unsigned long startMillis;
unsigned long currentMillis;
unsigned long otherMillis;
int power = 255;
int onTime = 2500;

void setup(){
  Serial.begin(115200);
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(mosfetPin, OUTPUT);
  pinMode(lightSensorPin, INPUT);
  pinMode(LED_BUILTIN, OUTPUT);
}

// Loop breaks that I'm not yet sure about
int breaker = 0; //#1
int powerBreak = 0; //#2

void loop(){
  // Start millis in loop
  startMillis = millis();

  // Convert button from positive pullup
  int button = digitalRead(buttonPin);
  button = !button;

  // Read light value from light pin
  int lightValue = analogRead(lightSensorPin);

  // 5 Second light protection

  //#1
  if (breaker == 0 && lightValue > light){
    otherMillis = millis();
    breaker = 1;
  }
  if (lightValue < light){
    breaker = 0;
  }
  //#2
  if (powerBreak = 0 && startMillis - otherMillis > 5000){
    analogWrite(mosfetPin, power);
    powerBreak = 1;
  }
  if (lightValue < dark){
    powerBreak = 0;
  }

  
  // Button activates mosfet
  if (button){
    analogWrite(mosfetPin, power);
    currentMillis = millis();
  }

  // Turn off motor after onTime
  if (startMillis - currentMillis > onTime){
    digitalWrite(mosfetPin, 0);
  }

  // LED if mosfet is getting power
  if (digitalRead(mosfetPin) == HIGH){
    digitalWrite(LED_BUILTIN, HIGH);
  }
  if (digitalRead(mosfetPin) == LOW){
    digitalWrite(LED_BUILTIN, LOW);
  }
  
  // Print things to test
  Serial.println(lightValue);
}

Any help/ input would be appreciated.

  if (powerBreak = 0 && startMillis - otherMillis > 5000){

Did you mean '==' instead of '=' ?

I don't know if @Whandall's suggestion will fix the problem but your code seems to me to be over complicated.

I would do it like this pseudo code

if (sensor Is Dark) {
   latestDarkTime = millis(); // reset the timer
}

if (millis() - latestDarkTime >= 5000) {
   // sensor has been bright for 5000 msecs
   // do stuff
}

...R

  // Start millis in loop
  startMillis = millis();

The timer that millis() reads is already running, so that is NOT what the code is doing.

The name startMillis is useless, because it does not define what is starting.

  // Convert button from positive pullup
  int button = digitalRead(buttonPin);
  button = !button;

Or just understand that LOW means pressed.

  if (breaker == 0 && lightValue > light){
    otherMillis = millis();

The name of the variable should reflect the event for which the time is to be recorded. So, this is an "other" event. What the heck is an other event?

I'm picking on your names because meaningful names would not be confusing you.

You are not going about the coding correctly, anyway.

What you need to be concerned about is when the light level BECOMES above (or below) the threshold, not when the light level IS above (or below) the threshold.

Whandall:

  if (powerBreak = 0 && startMillis - otherMillis > 5000){

Did you mean '==' instead of '=' ?

I did mean "==" instead of "=", thanks!

Robin2:
I don't know if @Whandall's suggestion will fix the problem but your code seems to me to be over complicated.

Unfortunately, it did not fix the problem at hand. I'll try out your way; thanks for the suggestion.

However, doing as you suggest, wouldn't that just keep updating the time while the if condition is met, thus requiring a loop breaker?

Robin2:

if (sensor Is Dark) {

latestDarkTime = millis(); // reset the timer
}
}

PaulS:

  // Start millis in loop

startMillis = millis();



The timer that millis() reads is already running, so that is NOT what the code is doing.

The name startMillis is useless, because it does not define what is starting.

But don't I still have to put it in a variable, so I can use that variable to subtract later?

PaulS:

  // Convert button from positive pullup

int button = digitalRead(buttonPin);
  button = !button;



Or just understand that LOW means pressed.

Yea, but I like having 0 being LOW and 1 being HIGH; just seems logical. I wish Arduino had some PULLDOWN function.

PaulS:
What you need to be concerned about is when the light level BECOMES above (or below) the threshold, not when the light level IS above (or below) the threshold.

Can you give me an example of what something like that would look like?

I take it "<" ">" are "IS" operators... What are the "BECOMES" operators?

Yes that just keeps resetting updating the start time while the sensor is dark. When the sensor gets light then it stops updating constantly and after 5 seconds pass the millis if is satisfied and does whatever thing you want done if the light has been on for 5 seconds. Isn't that what you wanted? Something to happen if the sensor got light for 5 seconds?

toxicxarrow:
... What are the "BECOMES" operators?

Arudino IDE: File --> Examples --> 02.Digital --> StateChangeDetection

I take it "<" ">" are "IS" operators...

No, those are less than and greater than. There's not an operator for becomes. Becomes is when something is now but wasn't last time. That just involves keeping track of what it was last time.

Yea, but I like having 0 being LOW and 1 being HIGH; just seems logical. I wish Arduino had some PULLDOWN function.

Yeah but there's a reason why it's normally done the other way round. Ground wires shorting to ground aren't quite as big a problem as 5V wires are. Needing LOW to be the same as OFF is something you should disabuse yourself of now. It's more often than not the opposite.

toxicxarrow:
.However, doing as you suggest, wouldn't that just keep updating the time while the if condition is met, thus requiring a loop breaker?

It only updates the time if the unwanted condition is met. Have you tried it?

...R

Delta_G:
Yes that just keeps resetting updating the start time while the sensor is dark. When the sensor gets light then it stops updating constantly and after 5 seconds pass the millis if is satisfied and does whatever thing you want done if the light has been on for 5 seconds. Isn't that what you wanted? Something to happen if the sensor got light for 5 seconds?

I guess it is, yea. I just didn't get it at the time; using a complete opposite route to get there! Thanks!

gfvalvo:
Arudino IDE: File --> Examples --> 02.Digital --> StateChangeDetection

I'll give it a look, thanks.

Delta_G:
Yeah but there's a reason why it's normally done the other way round. Ground wires shorting to ground aren't quite as big a problem as 5V wires are. Needing LOW to be the same as OFF is something you should disabuse yourself of now. It's more often than not the opposite.

I guess that makes sense, from an electronic standpoint about the shorting wires and such. Thanks! It just seems to go against everything else, binary: 0 = off/ LOW, 1 = on/ HIGH; even in the Arduino IDE when you initialize, it defaults to 0.

Robin2:
It only updates the time if the unwanted condition is met. Have you tried it?

...R

It didn't really click at the time, but I think I get it now; different, simpler way to get my desired result. I'll try it as soon as I get the chance. Thank you.

You guys seem to have everything in hand, but I'd just like to make one comment: stop using the words ON and OFF. As you've just demonstrated, they cause confusion. The accepted terminology is ACTIVE and IDLE, and as DG pointed out, most commonly signals are active LOW. The reason for this is that any dummy can pull a signal to ground and they're all the same. As the signal pulls away from ground it becomes idle and this usually occurs somewhere around a bit less that 1V. At the system level, nobody really tests for active HIGH as there are often issues such as de-bounce that can cause the HIGH value to waver above and below the threshold. A grounded signal however is a grounded signal, so the tests in reality are: grounded?ACTIVE:IDLE;

I guess it is, yea. I just didn't get it at the time; using a complete opposite route to get there! Thanks!

Maybe a different variable name would make it clearer...

if (sensor is dark) {
    lastDarkTime = millis();
}

howLongSinceItWasDark = millis() - lastDarkTime;  // this keeps getting 0 when it's still dark

if(howLongSinceItWasDark >= 5000){
   //do your thing here
}