Sketch crashes radomly after adding Millis

Dear

I was trying to improve a sketch by adding the millis. The main reason is that the LDR light sensor doesn't need to give values all the time...because lights will switch on and of when the analog value "of decision" is reached. therefor I added a gap in the analog value from 50 to 60.

The problem is: when I added the milis function...the sketch freezes randomly. I don't know what I'm doing wrong.

unsigned long interval = 600000;
unsigned long previousMillis = 0;

void loop() {
  unsigned long currentMillis = millis();
  if ((unsigned long)(currentMillis - previousMillis) >= interval) { 
    val = analogRead(5);
    Serial.println(val);
    previousMillis = millis();
  }
 
  if (val < 50) { 

// DO SOME STUFF
  }
  else if (val > 60) {
// DO OTHER STUFF
  }
}

I don't immediately see anything wrong with the code in your Original Post. Have you omitted stuff where your have the comment // DO SOME STUFF? If so, maybe that's where the problem lies.

This line

  if ((unsigned long)(currentMillis - previousMillis) >= interval) {

can be simplified to

  if (currentMillis - previousMillis >= interval) {

The demo Several Things at a Time illustrates the use of millis() to manage timing without blocking. It may help with understanding the technique.

...R

The program will read the value of pin 5 every time the interval is exceeded and update val at that time

interval has a value of 600000 milliseconds. How long is that in seconds ?
Try printing the value of currentMillis - previousMillis so that you can see what is going on

Note that the code as posted does not compile because val is not declared and there is no setup() function in it

Freezes randomly...how randomly? After a few seconds? Hours? Is the last value of val always the same, or similar, or just totally random?

600,000 is ten hours...that’s quite a long time, and millis() does reset or rollover at some point (can’t remember exact value, it’s days, but if you’re checking only roughly twice a day, that could be the problem...which is why we need to know more about the freezing behaviour...)

OK I made a serial monitor output: (i lowered 600000 to 60000=1minute in stead of 10 minutes)

[currentMillis-previousMillis] [LDR analog value]
60051 2
60000 0
60000 1010
60000 1010

(I played with some ducktape over the LDR sensor)

The freezing is randomly. Mostly half an hour or an hour

UKHeliBob:
The program will read the value of pin 5 every time the interval is exceeded and update val at that time

interval has a value of 600000 milliseconds. How long is that in seconds ?
Try printing the value of currentMillis - previousMillis so that you can see what is going on

Note that the code as posted does not compile because val is not declared and there is no setup() function in it

Indeed: val is declared but i did'nt wrote all the code. Everything was working before but the problem started when using milis.
(I can put the whole code..but It is quit long and maybe not that clear what the problem is)
600000 is 10minutes...I lowered to 1 minute.

Offie:
The freezing is randomly. Mostly half an hour or an hour

Exactly what happens when the program freezes?

What happens if you reduce the interval to (say) 6 seconds.

Post the latest version of your complete program.

...R

Ten hours. What was I thinking? Give me a break, it’s Sunday and it’s early...

The purpose of the code is to illuminated the stairs at home. When it is dark enough...the first and last step are on brightness 20. When it is light enough, all lights go out.

When it is dark: the stairs will go on to brightness 100 one by one and off in the same direction. (first and last step dim from 100 to 20 again)

The problem with the "freezing": for exemple: the first and last step are on but nothing else works when passing a pir sensor

here is the complete code: (modification of the code from: https://stairsautomation.wixsite.com/home)

#define SHIFTPWM_NOSPI                           
const int ShiftPWM_dataPin = 6;
const int ShiftPWM_clockPin = 7;
const int ShiftPWM_latchPin = 8;
#include <ShiftPWM.h>
const bool ShiftPWM_invertOutputs = false;
const bool ShiftPWM_balanceLoad = false;
#define sensorLight_PIN A5
#define sensorBottom_PIN A1
#define sensorUpper_PIN A2
unsigned char maxBrightness = 50;               
unsigned char pwmFrequency = 75;
unsigned int numRegisters = 3;
unsigned int numOutputs = numRegisters * 8;
unsigned int numRGBLeds = numRegisters * 8 / 3;
unsigned int fadingMode = 0;                     
unsigned long interval = 60000;                
unsigned long previousMillis = 0;               
boolean sensorUpperActive;
boolean sensorBottomActive;
int numberOfstairs = 11;                         
int pause = 20000;                               
//int pause2 = 5000;                           
byte faidoutSpeed = 5;                         
byte faidinSpeed = 5;                           
int val;
int calibrationTime = 60;                        

void setup() {
  Serial.begin(9600);
  // start calibratie pir-sensors
  Serial.print("Calibratie bewegingsmelders (wacht 60 seconden) ");
  for (int i = 0; i < calibrationTime; i++) {
    Serial.print(".");
    delay(1000);                                 
  }
  Serial.println(" gereed");
  Serial.println("BEWEGINGSMELDERS ACTIEF");
  delay(50);
  // einde calibratie pir-sensors
  ShiftPWM.SetAmountOfRegisters(numRegisters);
  ShiftPWM.Start(pwmFrequency, maxBrightness);
  pinMode(sensorBottom_PIN, INPUT);
  pinMode(sensorUpper_PIN, INPUT);
  pinMode(sensorLight_PIN, INPUT);
}
void loop() {
  unsigned long currentMillis = millis();        
  //if ((unsigned long)(currentMillis - previousMillis) >= interval) {    
  if (currentMillis - previousMillis >= interval) {
    val = analogRead(5);                          
    //Serial.print(currentMillis - previousMillis);
    //Serial.print(" ");
    //Serial.println(val);                        
    previousMillis = millis();                    
  }
  //val = analogRead(5);                          
  //Serial.println(val);                          
  if (val < 50) {                                 
    BottomTriggerFire();
    UpperTrigerFire();
    switchONOFFfromdown();
    switchONOFFfromUp();
    switchLEDstandby();                           
  }
  else if (val > 60) {                            
    switchLEDoff();
  }
}
void BottomTriggerFire() {
  if (analogRead (sensorBottom_PIN) >= 550 ) {    
    sensorBottomActive = true;
  }
}
void UpperTrigerFire() {
  if (analogRead(sensorUpper_PIN) >= 550) {
    sensorUpperActive = true;
  }
}
void switchONOFFfromdown() {
  if (sensorBottomActive == true && sensorUpperActive == false) {
    for (int i = 0; i < numberOfstairs; i++) {
      Serial.print("led aan: ");
      Serial.println(i + 1);
      for (int a = 0; a < maxBrightness; a++) {
        ShiftPWM.SetOne(i, a);
        delay(faidoutSpeed);
      }
    }
    delay(pause);
    // start uitdimmen tot brightness 20 trap 1
    Serial.print("led uit: ");
    Serial.println(1);
    for (int a = maxBrightness; a >= 20; a--) {
      ShiftPWM.SetOne(0, a);
      delay(faidinSpeed);
    }
    // einde uitdimmen tot brightness 20 trap 1
    // start uitdimmen tot brightness 0 trap 2 tot 10
    for (int i = 1; i < numberOfstairs - 1; i++) {
      Serial.print("led uit: ");
      Serial.println(i + 1);                      
      for (int a = maxBrightness; a >= 0; a--) {
        ShiftPWM.SetOne(i, a);
        if (a == 0) {
          ShiftPWM.SetOne(i, 0);
        }
        delay(faidinSpeed);
      }
    }
    // einde uitdimmen tot brightness 0 trap 2 tot 10
    // start uitdimmen tot brightness 20 trap 11
    Serial.print("led uit: ");
    Serial.println(11);
    for (int a = maxBrightness; a >= 20; a--) {
      ShiftPWM.SetOne(10, a);
      delay(faidinSpeed);
    }
    // einde uitdimmen tot brightness 20 trap 11
    sensorBottomActive = false ;
  }
}
void switchONOFFfromUp() {
  if ( sensorUpperActive == true && sensorBottomActive == false) {
    for (int i = numberOfstairs - 1; i >= 0; i--) {
      Serial.print("led aan: ");
      Serial.println(i + 1);
      for (int a = 0; a < maxBrightness; a++) {
        ShiftPWM.SetOne(i, a);
        delay(faidoutSpeed);
      }
    }
    delay(pause);
    // start uitdimmen tot brightness 20 trap 11
    Serial.print("led uit: ");
    Serial.println(11);
    for (int a = maxBrightness; a >= 20; a--) {
      ShiftPWM.SetOne(10, a);
      delay(faidinSpeed);
    }
    // einde uitdimmen tot brightness 20 trap 11
    // start uitdimmen tot brightness 0 trap 10 tot 2
    for (int i = numberOfstairs - 2; i >= 1; i--) {
      Serial.print("led uit: ");
      Serial.println(i + 1);
      for (int a = maxBrightness; a >= 0; a--) {
        ShiftPWM.SetOne(i, a);
        if (a == 0) {
          ShiftPWM.SetOne(i, 0);
        }
        delay(faidinSpeed);
      }
    }
    // einde uitdimmen tot brightness 0 trap 10 tot 2
    // start uitdimmen tot brightness 20 trap 1
    Serial.print("led uit: ");
    Serial.println(1);
    for (int a = maxBrightness; a >= 20; a--) {
      ShiftPWM.SetOne(0, a);
      delay(faidinSpeed);
    }
    // einde uitdimmen tot brightness 20 trap 1
    sensorUpperActive = false ;
  }
}
void switchLEDstandby() {
  ShiftPWM.SetOne(0, 20);                  
  ShiftPWM.SetOne(10, 20);                 
}
void switchLEDoff() {
  ShiftPWM.SetAll(0);                      
}

The problem may be being caused or masked by the use of the ShiftPWM library, which I know nothing about. What hardware are you using to drive the LEDs on the stairs and how is it connected to the Arduino ?

Not something as simple as a power saving timeout on the PIR sensor?

UKHeliBob:
The problem may be being caused or masked by the use of the ShiftPWM library, which I know nothing about. What hardware are you using to drive the LEDs on the stairs and how is it connected to the Arduino ?

I used the shield provided by (https://stairsautomation.wixsite.com/home) The shiftregisters are connected to logic mosfets.
I don't think that the problem is in the hardware as everything worked ok before using the millis function.
I'm now trying with millis 60000 in stead of 600000 and everything seems ok as far as the test is running :wink:

Offie:
here is the complete code:

What is the purpose of having a HUGE interval between successive reads of whatever is on pin A5

...R

Robin2:
What is the purpose of having a HUGE interval between successive reads of whatever is on pin A5

...R

I was trying to do a a sort of hysteresis on that analog input: trigger values with a gap between 50 and 60 analog value. That was not enough. The incoming data changed that much in amplitudo so lights keeps triggering on and off when the analog value is in the 50-60 zone. By reding the analog value once a minute i would like to solve the problem. Maybe there is a better way? Working with flags or states?

Try printing out the values of sensorBottomActive and sensorUpperActive as they change. If they both get the value true at the same time then things start to stick...

BTW, switchONOFFfromdown() and switchONOFFfromUp() are very similar functions which can be combined into one parameterised function. I would probably move the if statement outside of said function.

Offie:
I was trying to do a a sort of hysteresis on that analog input:

You don't do that with a timing interval. Just compare the latest value with the previous value and do nothing unless the difference is big enough. Something like this

oldAdcVal = adcVal;
adcVal = analogRead(sensorPin);
if (abs (oldAdcVal - adcVal) > threshold) {
   // do stuff
}

...R

Problem solved. By reading each minute in stead of each 10 minutes I noticed that the "freezing" also lasted for 1 minute.

The following happend: the analog value was just in the gap between <50 and >60..so that was the reason nothing happend. This isn't any problem in the morning or in the evening were a descision point is for lightening the stairs or turning it off....but it is a problem that due to noise suddenly a value between 50 and 60 was read when it is night. Than nothing happened for a minute (no readings on the pir sensors).

How to solve this. I keep using a gap (but made it a little bit smaller) and smoothened the analog input values by using the AnalogSmooth libray. All test are fine now.

Hi,

did you solve this?
because I have the same issue.

Here is the part of my code:

void drawPicture(long long picture, byte duration)
{
  int start = millis();

  while (start + duration*25 > millis())
    for (int y = 0; y < 8; y++)
      for (int x = 0; x < 8; x++)
        if ((picture >> (y * 8 + x)) & 0x01)
          togglePixel(x, y);
}

void togglePixel(int x, int y)
{ 
  digitalWrite(pinx[x], HIGH);
  digitalWrite(piny[y], LOW);
  delayMicroseconds(50);
  digitalWrite(pinx[x], LOW);
  digitalWrite(piny[y], HIGH);
}

after adding millis() my application crashes after seconds. If I alter the value after duration (so millis() wont be called so often) the time till crashing expands.

If I remove millis() completely the crashes vanishes and the arduino works for hours. But I want timing in millis because I multiplex pixels on a display. And pictures with more pixel should have the same duration as pictures with less pixels.

void drawPicture(long long picture, byte duration)
{
  int counter = 0;
  while (counter < duration)
  {
    counter++;
    for (int y = 0; y < 8; y++)
      for (int x = 0; x < 8; x++)
        if ((picture >> (y * 8 + x)) & 0x01)
          togglePixel(x, y);
  }
}

seems for me an internal overflow in millis(). Are there alternatives?

sheepchen:
... I have the same issue.

void drawPicture(long long picture, byte duration)

{
 int start = millis();

while (start + duration*25 > millis())
...
}



seems for me an internal overflow in millis(). Are there alternatives?

The alternative is to use millis() correctly: it returns an unsigned long type, not an int.
And your while loop should be written differently so it behaves correctly upon millis roll-over: while (millis() - start < duration*25)

sheepchen:
did you solve this?
because I have the same issue.

Have you carefully studied the link in Reply #1?

...R