Stuck with program flow and freezing programm

I know, asking for help I would need to post some code but to show exactly what happens and where, I would need to post some code exceeding the allowed 9500 characters. Let me try to explain the problem:

I am working on a intelligent lighting project. The system is monitoring various PIR's in different rooms and a few momentary switches to override the PIR's. Since the project became far larger than thought of in the beginning and I got into the need of implementing a menu system, I needed to clean up my main code a bit.

For example to check the PIR's and determine if the lights are to be switched on or not I am running the PIR feedback and some sensor readings of a photocell through a set of if statements and pass everything through a loop in the end to produce a binary to control a set o two 74HC595 shift registers controlling 15 relays and a monitoring LED. Up til here everything is working fine.

Since I was pretty lazy and just repeated the same bit of code controlling the PIR's for every room I was using up quite a lot of memory which I need to implement the menu.
Now I was taking the two parts of code controlling the PIR's and put them into functions and replaced that respective part of code with the function call in the main program loop.
After doing this the program stops after 2 to 3 minutes apart from using the reset button the system does not respond to any input and the connected LCD usually showing date and time spills out a few random characters and freezes with the rest of the system. My guess is that I might have broken the program flow some where or for some reason I am running out of RAM even the current Ram usage after upload is 56%.
If anybody knows what to look for and where I might be missing something, any help would be appreciated.

I would need to post some code exceeding the allowed 9500 characters.

Use the Additional Options... link to attach the code.

Thanks for the advice.
I attached the parts of code in question. The first part is the bit checking the PIR's and the output as it is working without any problems.
The second attachment is showing the changes I have made to convert the bit of code into functions and how it is called.
If you need more code or what ever information please let me know.

And thanks for having a look at.

working_parts_of_code.ino (5.83 KB)

after_changed_to_function.ino (4.97 KB)

If you suspect that you are running out of memory, you are probably right. Meanwhile, though, you are wasting our time by not posting ALL of your code.

I, for one, won't even read any of it knowing that it isn't complete. Where you think the problem is may not be where it is at all.

No problem, here is the whole lot attached. However, to look at the first two attachments show the changes I made when the problems started to turn up.

And thanks again

blog_RMU_1_2_4.ino (56.5 KB)

unsigned int hourAc1On = 18;
unsigned int minuteAc1On = 0;
unsigned int hourAc1Off = 24;
unsigned int minuteAc1Off = 0;

//////////////////////holliday timer settings//////////////////////

////////////Room 1 (Bed 1) ///////////
byte room1MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room1OnM[2] = {5, 35};        //Time to switch on the lights 5
unsigned int room1OffM[2] = {6, 5};       //Time to switch off the lights 6

byte room1O1Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room1On1[2] = {19, 35};        //Time to switch on the lights
unsigned int room1Off1[2] = {20, 15};       //Time to switch off the lights

byte room102Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room1On2[2] = {21, 5};        //Time to switch on the ;ights
unsigned int room1Off2[2] = {21, 15};       //Time to switch off the lights

////////////Room 2 (Bed 2) ///////////
byte room2MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room2OnM[2] = {6, 30};         //Time to switch on the lights
unsigned int room2OffM[2] = {6, 50};        //Time to switch off the lights

byte room201Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room2On1[2] = {19, 30};        //Time to switch on the lights
unsigned int room2Off1[2] = {20, 10};       //Time to switch off the lights

////////////Room 3 (Bed 3) ///////////
byte room3MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room3OnM[2] = {5, 50};        //time to switch on the lights
unsigned int room3OffM[2] = {6, 20};       //time to switch off the lights

byte room301Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room3On1[2] = {18, 10};        //time to switch on the lights
unsigned int room3Off1[2] = {18, 25};       //time to switch off the lights

byte room302Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room3On2[2] = {19, 15};        //Time to switch on the lights
unsigned int room3Off2[2] = {19, 40};       //Time to switch off the lights

byte room303Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room3On3[2] = {23, 20};        //Time to switch on the lights
unsigned int room3Off3[2] = {23, 35};       //Time to switch off the lights

///////////Room 4 {Living)
byte room401Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room4On[2] = {17, 30};         //Time to switch on the lights
unsigned int room4Off[2] = {23, 30};        //Time to switch off the lights

//////////Room 5 (bath 1)//////////
byte room5MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room5OnM[2] = {5, 40};         //Time to switch on the lights
unsigned int room5OffM[2] = {5, 45};        //Time to switch off the lights

byte room501Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room5On[2] = {19, 55};         //Time to switch on the lights
unsigned int room5Off[2] = {20, 10};        //Time to switch off the lights

//////////Room 6 (bath 2)//////////
byte room6MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room6OnM[2] = {6, 35};         //Time to switch on the lights
unsigned int room6OffM[2] = {6, 45};        //Time to switch off the lights

byte room601Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room6On[2] = {19, 50};         //Time to switch on the lights
unsigned int room6Off[2] = {20, 05};        //Time to switch off the lights

//////////Room 7 (bath 3)//////////
byte room7MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room7OnM[2] = {6, 5};         //Time to switch on the lights
unsigned int room7OffM[2] = {6, 25};        //Time to switch off the lights

byte room701Active = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room7On[2] = {22, 50};         //Time to switch on the lights
unsigned int room7Off[2] = {23, 15};        //Time to switch off the lights

//////////Room 8 (bath 4)//////////
byte room8MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room8On[2] = {22, 5};         //Time to switch on the lights
unsigned int room8Off[2] = {22, 20};       //Time to switch off the lights

//////////Room 9 (Kitchen)//////////
byte room9MActive = 1;                     //Set to 1 if you want to process
                                           //Timer will be ignored when set to 0
unsigned int room9OnM[2] = {5, 50};         //Time to switch on the lights
unsigned int room9OffM[2] = {6, 45};        //Time to switch off the lights

byte room901Active = 1;                     //Set to 1 if you want to process
                                            //Timer will be ignored when set to 0
unsigned int room9On[2] = {17, 45};         //Time to switch on the lights
unsigned int room9Off[2] = {18, 30};        //Time to switch off the lights

Do these REALLY need to be ints?

unsigned int switchState[25] = {0};           //array holding the state of each switch

On or Off needs an int?

  if(switchState[21] == 1){
    Serial.println("Setup");
  }

But, Serial.begin() is commented out.

  if(analogRead(lightSensor) > 0 && 
     analogRead(lightSensor) < 1024){ //checking if the reading

Since analogRead() returns values in the range 0 to 1023, this is useless test.

    int reading[15] = {0};            //declaring a local var to hold 
                                      //the readings
     for(int i=0; i<15; i++){         //take 10 readings
      reading[i] += analogRead(lightSensor);
    }
    //average the readings
    sensorValue = (reading[0] + reading[1] + reading[2] + reading[3] +
                   reading[4] + reading[5] + reading[6] + reading[7] +
                   reading[8] + reading[9] + reading[10] + reading[11] +
                   reading[12] + reading[13] + reading[14]) / 15;

There's a waste of 30 bytes. Set sensorValue to 0, then increment it by the reading divided by 10 in the loop. Then, divide by 15. There is no reason to store the data in an array.

Thanks for the suggestions.
to point 1, the timers are now all set to byte with a few other variables only holding a status 0 or 1.
to point 2 changed to byte as well
to point 3 the Serial.println statement, I must have missed it when I commented out all the other Serial print statements, done as well.
to point 4

 if(analogRead(lightSensor) > 0 && 
     analogRead(lightSensor) < 1024){ //checking if the reading

I took it out. The idea was if something goes wrong with the photocell and some readings come which are off the scope, the lights be switched on anyway.
point 5 I am currently working on.
Uploading the changed sketch changed the behavior in so far that everything is resetting itself every 30 seconds.

I guess saving another few bytes on point 5 might get everything working again.
What I still not understand is the fact that everything was working before I started to do the indicated changes. And is there a way of managing the memory since I am still in need to implement a menu to change the timers?

And is there a way of managing the memory

You "manage the memory" by not wasting it. No variables larger than they need to be. No arrays larger than they need to be. No unnecessary strings in SRAM. No Strings AT ALL.

If these still cause you to run out of memory, you need an Arduino with more memory.

If these still cause you to run out of memory, you need an Arduino with more memory.

Point taken.

However, I am still looking for a explanation why everything was working fine with having this bit of code 9 times in the main loop:

     if(switchState[0] == 1 && lightStatus[16] == 1) {            //checking if PIR in Room 1 was
                                                                   //activated (bed 1)
         lightStatus[16] = 0;                                      //resetting master off
         digitalWrite(doorMonitor, LOW);                           //resetting the door Monitor LED
      }
     
      if(switchState[1] == 0 && sensorValue <= photoCellCutOff) {  //checking if S2 priority off was
                                                                   //set bed 1
        if(switchState[0] == 1 && priorityStatus[0] == 0) {        //check if the PIR in bed 1 was
                                                                   //activated and no priority was set
           //Serial.println("We switch in the lights in bedroom 1");  //Debug only
           lightOutput[0] = 1;                                     //switching on the lights – binary
                                                                   //000000000000000000000001
           lightStatus[0] = 1;                                     //setting the light status for bed 1
           lightOutput[14] = 16384;                                //make sure the master relay
                                                                   //stays on
           lightStatus[14] = 1;                                    //setting the master yelay status
           roomTimer[0] = millis();                                //setting the timer
        }
        else if(switchState[0]  == 0 && lightStatus[0] == 1) {     //the PIR not activated but the
                                                                   //lights are on
           //Serial.println("We are checking the timer");            //Debug only
           currentTime = millis();                                 //setting time reference
           endTime = currentTime - roomTimer[0];                   //calculating the inactive time
           if(endTime >= delayTime[0]) {                           //comparing inactive time with
                                                                   //allowed delay time
              //Serial.println("Time is up switching off the lights");  //Debug only
              lightOutput[0] = 0;                                  //switching off the lights
              lightStatus[0] = 0;                                  //resetting the light status
              roomTimer[0] = 0;                                    //resetting the room timer
           }
        }
     }
     else if(switchState[1] == 1 && lightStatus[0] == 1
             && switchState1Old != 1) {                            //if priority is activated and the
                                                                   //lights are on
        //Serial.println("Priority switch activated switching off the lights");  //Debug only
        lightOutput[0] = 0;                                        //switching off the lights
        lightStatus[0] = 0;                                        //resetting the light status
        roomTimer[0] = 0;                                          //resetting the room timer
        priorityStatus[0] = 1;                                     //setting the priority status bed 1
     }
     else if(switchState[1] == 1 && lightStatus[0] == 0
             && switchState1Old != 1) {                               //if priority was activated and the 
                                                                   //lights are off
        //Serial.println("Priority switch deactivated switching on the lights");    //Debug only
        lightOutput[0] =1;                                         //switching on the lights
        lightStatus[0] = 1;                                        //setting the light status
        roomTimer[0] = millis();                                   //setting the room timer
        priorityStatus[0] = 0;                                     //setting the priority for bed 1 back  
     }
     switchState1Old = switchState[1];                             //passing on the switch state

and after I placed it in a function, having it only once and the respective function calls I am getting the memory problems.

What happens when a function is called? Arguments are, usually, passed by value. How does that happen? The arguments are pushed onto the stack. What happens when the stack meets the heap? Bad things.

What happens when a function is called? Arguments are, usually, passed by value. How does that happen? The arguments are pushed onto the stack. What happens when the stack meets the heap? Bad things.

That means I have to live with having the bit of code repeating 9 times and fill up the program memory or using the function and dealing with a stack overflow.

Is there some tutorials dealing with that so I can plan the next project a bit better in that respect.

int reading[15] = {0}; //declaring a local var to hold
//the readings
for(int i=0; i<15; i++){ //take 10 readings
reading += analogRead(lightSensor);

  • }*
  • //average the readings*
  • sensorValue = (reading[0] + reading[1] + reading[2] + reading[3] +*
  • reading[4] + reading[5] + reading[6] + reading[7] +*
  • reading[8] + reading[9] + reading[10] + reading[11] +*
  • reading[12] + reading[13] + reading[14]) / 15;*
    There's a waste of 30 bytes. Set sensorValue to 0, then increment it by the reading divided by 10 in the loop. Then, divide by 15. There is no reason to store the data in an array.[/quote]
    I changed this part to
    ```
  • sensorValue = 0;
        int reading = 0;                                  //the readings
        for(int i=0; i<15; i++){                      //take 15readings
          int value = analogRead(lightSensor);
          reading = reading + value;
        }
        //average the readings
        sensorValue = reading / 15;
      //Serial.print("Sensor value: ");*
    ```
    It looks like everything is stable again. It's running since about 10 minutes without visible problems.
    Thank you very much for the help.
    However, if you have a hint how to get the menu into that without running into memory problems again, I would be very happy to hear about.
    Again, thanks for your help

You seem to have a lot of LONG variables to store millisecs and I suspect the things you want to time are really only measured in seconds or minutes.

If you write a small function(s) to count seconds or minutes then maybe all those longs could be reduced to integers or bytes.

...R

    int reading = 0;                                  //the readings
     for(int i=0; i<15; i++){                      //take 15readings
      int value = analogRead(lightSensor);
      reading = reading + value;
    }

Have you ever wondered why the language we use to program the Arduino wasn't called C=C+1? Why do you need a local variable in the loop?

     int reading = 0;                                  //the readings
     for(int i=0; i<15; i++)
     {                      //take 15readings
         reading += analogRead(lightSensor);
     }

Have you ever wondered why the language we use to program the Arduino wasn't called C=C+1? Why do you need a local variable in the loop?

I have tried that approach once and I forgot why but I couldn't get it to work and so I fell back in my old php programming habits. I changed it again and it works.

Again, thanks a lot for your great help

You seem to have a lot of LONG variables to store millisecs and I suspect the things you want to time are really only measured in seconds or minutes.

If you write a small function(s) to count seconds or minutes then maybe all those longs could be reduced to integers or bytes.

...R

You are right, I have a look at it right away

Thanks

byte getSensorValue(int sensorReading, int switchValue, 
                    int switchLimit, byte switchStatus){
  byte onStatus = 0;
  
  if(switchStatus == 0){
  [...]
    if(sensorReading <= switchValue){
  [...]
  else if(switchStatus = 1){
  [...]

Might want to fix that else if.

Regards,

Brad
KF7FER

That means I have to live with having the bit of code repeating 9 times and fill up the program memory or using the function and dealing with a stack overflow.

It doesn't really mean that, at all.

When you convert bits of duplicated code to a function, you might need to pay attention to what variables the function is modifying.

When you convert bits of duplicated code to a function, you might need to pay attention to what variables the function is modifying.

How do you mean? Do I have to change variables from global to local var's or is there any rule what to be changed?

Might want to fix that else if.

Thank you very much, I honestly missed that one.