Fragmented Heap?

Hi. My alarm clock sketch stops running after five or six days, so I am presuming it is a memory problem. I have cut down the original code but still have a problem. I am using a Arduino Nano Board I have put up as much of the code as I can and attach a schematic.
This is my first project so if I am doing things wrong please let me know.

[code]

//RTC
#include <RTClib.h>
RTC_DS1307 rtc;
int hourupg;
int minupg;
int yearupg;
int monthupg;
int dayupg;

//RotarySwitch
#include <RotaryEncoder.h>
RotaryEncoder encoder(5, 6);
int pos = 0;
int newPos = 0;
int AdjUp;
int AdjDn;

//The Menu
int menuCounter = 0;
int menuState = 0;
int menuLastState = 0;
int menuLive = 0;


//Debounce
#include <Bounce2.h>
Bounce MenuButton = Bounce();
Bounce AlarmSw1 = Bounce();
Bounce AlarmSw2 = Bounce();
Bounce RadioSw1 = Bounce();
Bounce RadioSw2 = Bounce();
Bounce EndButton = Bounce();

//SwitchLogic
int MenuPin = 7;
int AlarmS1Pin = 11;
int AlarmS2Pin = 10;
int RadioS1Pin = 9;
int RadioS2Pin = 8;
int EndPin = 4;
int RelayPin = 12;
int Alarm1;
int SetAlarm1;
int AlarmSw1State;
int AlarmSw2State;
int RadioSw1State;
int RadioSw2State;
int EndButtonState;
int adjTime;
int adjAlarm1;
int relay;
int RelayState;
int LastRelayState;
int LastAlarm1State;
int Alarm1State;


//Alarms
int Alarm1Minupg;
int Alarm1Hourupg;
int displayMinupg;
int displayHourupg;



//Display
#include "Adafruit_LEDBackpack.h"
Adafruit_7segment Display = Adafruit_7segment();
int setBrightness ;
int displayValue;
int brightnessAdj;


//-------------------------------------------------------------Setup------

void setup() {

  //rtc.adjust(DateTime(2020,12,21,11,50,0));
  //Serial.begin(9600);
  MenuButton.attach( MenuPin, INPUT );
  MenuButton.interval(5);
  AlarmSw1.attach( AlarmS1Pin, INPUT );
  AlarmSw1.interval(5);
  AlarmSw2.attach( AlarmS2Pin, INPUT );
  AlarmSw2.interval(5);
  RadioSw1.attach( RadioS1Pin, INPUT );
  RadioSw1.interval(5);
  RadioSw2.attach( RadioS2Pin, INPUT );
  RadioSw2.interval(5);
  EndButton.attach( EndPin, INPUT );
  EndButton.interval(5);
  pinMode (RelayPin, OUTPUT);
  digitalWrite(RelayPin, LOW);
  Display.begin (0x70);

  Alarm1Minupg  = rtc.readnvram(0);
  Alarm1Hourupg = rtc.readnvram(1);


  //  Serial.println(rtc.readnvram(0));
  // Serial.println(rtc.readnvram(1));
  // Serial.println(rtc.readnvram(2));
  //  Serial.println(rtc.readnvram(3));

} //End Setup

//--------------------------------------------------------------Loop-----------

void loop() {

  Rtc();
  Glow();
  SwitchLogic();
  RotarySwitch();
  Clock();
  RelayControl();
  encoder.tick();
  Menu();
  MenuButton.update();
  AlarmSw1.update();
  AlarmSw2.update();
  RadioSw1.update();
  RadioSw2.update();
  EndButton.update();

} //End loop

//----------------------------------------------------------RelayControl------

void RelayControl() {

  if ( SetAlarm1 == HIGH && minupg == Alarm1Minupg && hourupg == Alarm1Hourupg) {
    RelayState = HIGH;// This goes high once at alarm time for 1 minute
  }
  else {
    RelayState = LOW;
  }

  if (RelayState != LastRelayState) {  // A new state Relay High only for 1 loop
    if (RelayState == HIGH) {
      relay = HIGH ;    // used to enable alarm to be reset during first minute
    }
  }
  LastRelayState = RelayState;
  
  if (relay == HIGH && EndButtonState == LOW ) {
    relay = HIGH;            //Relay stays high untill endbutton operated
    digitalWrite (RelayPin, HIGH);
  }
  else {
    relay = LOW;
    digitalWrite (RelayPin, LOW);
  }

}  // End Relay Control

//-------------------------------------------------------Switch Logic---------

void SwitchLogic() {

  AlarmSw1State = AlarmSw1.read();
  AlarmSw2State = AlarmSw2.read();
  RadioSw1State = RadioSw1.read();
  RadioSw2State = RadioSw2.read();
  EndButtonState = EndButton.read();

  if (RadioSw1State == LOW) {   //Relay energised always
    relay = HIGH;
  }
  if (RadioSw2State == LOW ) {   //Radio switch in the Alarm position
    SetAlarm1 = HIGH;           //So alarm can be set
  }
  else {
    SetAlarm1 = LOW;
  }

  if (RadioSw1State == HIGH && RadioSw2State == HIGH) {
    adjTime = HIGH;    //Radio switch in middle position so Time can be adjusted
  }
  else {
    adjTime = LOW;
  }

  if (AlarmSw1State == HIGH && AlarmSw2State == HIGH) {
    adjAlarm1 = HIGH;  //Alarm switch in middle position so Alarm can be adjusted
  }
  else {
    adjAlarm1 = LOW;
  }

  if (EndButtonState == HIGH) {
    menuLive = LOW;
  }

} //EndSwitchLogic

//------------------------------------------------------Menu----------------

void Menu() {

  if (MenuButton.read() == LOW) { //Press of Menu Button
    menuState = HIGH;
    menuLive = HIGH;
  }
  else {
    menuState = LOW;
  }

  if (menuState != menuLastState) { //menuState goes high on button press
    if (menuState == LOW) {         //When buton released, steps
      if (menuCounter < 3) {
        menuCounter++;
      }
    }
  }
  menuLastState = menuState;  // sets the lastState to the state

  if (menuLive == LOW) {      //Resets Menu
    menuCounter = 0;
  }

  if (menuLive == HIGH && adjTime == HIGH) {
    if (menuCounter == 2) {
      SetTimeMinute();
    }
    if (menuCounter == 3) {
      SetTimeHour();
    }
  }

  if (menuLive == HIGH && adjAlarm1 == HIGH) {
    if (menuCounter == 2) {
      SetAlarm1Minute();
    }
    if (menuCounter == 3) {
      SetAlarm1Hour();
    }
  }

  if (EndButtonState == HIGH) {
    menuLive = LOW;
  }

} // End Menu

[/code]

Include all your code. Attach the .ino file to the post.
You don't appear (from the code you have included) to be doing anything that would fragment the heap. That would include using the String class, the new keyword, malloc etc which dynamically allocate memory.

Hi I didn’t think I was using any strings. Do you think any of the libraries could? The rest of the code and attached .ino.

[code]
  }

} // End Menu

//--------------------------------------------------Clock-----------------

void Clock() {

  if (menuLive == HIGH) {
    if (adjAlarm1 == HIGH && adjTime == LOW) { //Alarm switch in middle position & not Adjust time
      displayHourupg = Alarm1Hourupg;          //and Radio switch not in middle
      displayMinupg = Alarm1Minupg;
    }

    if (adjTime == HIGH && adjAlarm1 == LOW) { //Radio switch in middle position
      displayHourupg = hourupg;
      displayMinupg = minupg;
    }
  }
  else {    //Displays time when there are no menu's
    displayHourupg = hourupg;
    displayMinupg = minupg;
  }

  //This writes the time or the alarm time to the display
  Display.writeDigitNum(0, (displayHourupg / 10), false );
  Display.writeDigitNum(1, (displayHourupg % 10), false );
  Display.writeDigitNum(3, (displayMinupg / 10), false );
  Display.writeDigitNum(4, (displayMinupg % 10), false );
  if (displayHourupg <= 9) {          //not sure why the "false" is needed
    Display.writeDigitNum(0, 16, false );
  }

  Display.drawColon(true);

  //This runs the display when setting the time
  if ( adjTime == HIGH ) {  //Radio switch in middle position
    if ( adjTime == HIGH && (menuCounter == 1)) {

      Display.writeDigitRaw (0, 0 );
      Display.writeDigitRaw (1, 0 );
      Display.writeDigitRaw (3, 0 );  // Blanks the display
      Display.writeDigitRaw (4, 0 );
    }

    if ( adjTime == HIGH && (menuCounter == 2)) {
      Display.writeDigitRaw (0, 0 );  //Writes first 2 digits dark for
      Display.writeDigitRaw (1, 0 );  //minute setting
    }
    if (adjTime == HIGH && (menuCounter == 3)) {
      Display.writeDigitRaw (3, 0 );  //Writes last 2 digits dark for
      Display.writeDigitRaw (4, 0 );  //hour setting
    }
  }

  //This runs the display and blanks some digits when setting alarm time
  if ( adjTime == LOW ) {    // Radio switch in position 1 or 3

    if (adjAlarm1 == HIGH && (menuCounter == 1) ) { //Alarm Setting
      Display.writeDigitRaw (0, 119 );
      Display.writeDigitRaw (1, 56 );
      Display.writeDigitRaw (3, 48 );   //writes "AL 1" to display
      Display.writeDigitRaw (4, 0 );
    }
    if (adjAlarm1 == HIGH && (menuCounter == 2) ) {
      Display.writeDigitRaw (0, 0 );    //Writes first 2 digits dark for
      Display.writeDigitRaw (1, 0 );    //alarm 1 minute setting
    }
    if (adjAlarm1 == HIGH && (menuCounter == 3) ) {
      Display.writeDigitRaw (3, 0 );    //Writes second 2 digits dark for
      Display.writeDigitRaw (4, 0 );    //alarm 1 Hour setting
    }
  }



  Display.writeDisplay(); //writes to the 7 segment display


} //End Clock


//-----------------------------------------------SetTimeMinute----------------

void SetTimeMinute() {

  if (AdjUp  == HIGH) {
    if (minupg >= 59) {
      minupg = 1;
    }
    else {
      minupg = minupg + 1;
    }
    rtc.adjust(DateTime(yearupg, monthupg, dayupg, hourupg, minupg, 0));

    AdjUp  = LOW;
  }                    // Adjusts time with output of rotary encoder
  if (AdjDn  == HIGH) { //and writes it to the RTC
    if (minupg <= 1) {
      minupg = 59;
    }
    else {
      minupg = minupg - 1;
    }
    rtc.adjust(DateTime(yearupg, monthupg, dayupg, hourupg, minupg, 0));
    //   Serial.println(minupg);
    AdjDn  = LOW;
  }
} // End SetTimeMinute

//-------------------------------------------------------SetTimeHour-------------

void SetTimeHour() {

  if (AdjUp  == HIGH) {
    if (hourupg >= 23) {
      hourupg = 0;
    }
    else {
      hourupg = hourupg + 1;
    }
    rtc.adjust(DateTime(yearupg, monthupg, dayupg, hourupg, minupg, 0));
    //   Serial.println(hourupg);
    AdjUp  = LOW;
  }
  if (AdjDn  == HIGH) {
    if (hourupg <= 0) {
      hourupg = 23;
    }
    else {
      hourupg = hourupg - 1;
    }
    rtc.adjust(DateTime(yearupg, monthupg, dayupg, hourupg, minupg, 0));
    //   Serial.println(hourupg);
    AdjDn  = LOW;
  }
} // End SetTimeHour



//------------------------------------------------------Set Alarm1-----------

void SetAlarm1Minute() {
  if (AdjUp  == HIGH) {
    if (Alarm1Minupg >= 59) {
      Alarm1Minupg = 1;
    }
    else {
      Alarm1Minupg = Alarm1Minupg + 1;
    }
    AdjUp  = LOW;
  }
  if (AdjDn  == HIGH) {
    if (Alarm1Minupg <= 0) {
      Alarm1Minupg = 59;
    }
    else {
      Alarm1Minupg = Alarm1Minupg - 1;
    }
    AdjDn  = LOW;
  }
  rtc.writenvram(0, Alarm1Minupg); //Saves the setting to nvram
} // End SetAlarm1Minute

void SetAlarm1Hour() {
  if (AdjUp  == HIGH) {
    if (Alarm1Hourupg >= 23) {
      Alarm1Hourupg = 0;
    }
    else {
      Alarm1Hourupg = Alarm1Hourupg + 1;
    }
    AdjUp  = LOW;
  }
  if (AdjDn  == HIGH) {
    if (Alarm1Hourupg <= 0) {
      Alarm1Hourupg = 23;
    }
    else {
      Alarm1Hourupg = Alarm1Hourupg - 1;
    }
    AdjDn  = LOW;
  }
  rtc.writenvram(1, Alarm1Hourupg);//Saves the setting to nvram

} // End SetAlarm1Hour







//------------------------------------------------------Rotary Switch----------

void RotarySwitch () {

  newPos = encoder.getPosition();
  if (pos != newPos) {
    if (pos < newPos) {
      AdjUp = HIGH;
    }
    if (pos > newPos) {
      AdjDn = HIGH;

    }
    pos = newPos ;
  }

}   //End RotarySwitch


//-------------------------------------------------RTC----------------------

void Rtc() {
  DateTime now = rtc.now();
  yearupg = now.year();
  monthupg = now.month();
  hourupg = now.hour();
  dayupg = now.day();
  minupg = now.minute();
} // End Rtc

//------------------------------------------------Glow-----------------------

void Glow() {     // Sets display brightness Range 0 to 15
  if (hourupg > 7 && hourupg < 23) {
    brightnessAdj = 8;
    Display.setBrightness(brightnessAdj);
  }
  else {
    brightnessAdj = 2;
  }
  Display.setBrightness(brightnessAdj);

} //End Glow

[/code]

MyAlarm_Clock19.ino (11.1 KB)

What device is the relay powering?

A small DAB radio was the plan but because the clock is unreliable the radio has been removed so at the moment the relay does nothing.

are you sure your encoder requires pull-ups?

Hi no included resistors, the encoder was a EC11 it was a bare unit not mounted on a PCB.

Ok
I don’t see anything related to heap issue in the code either, so worth looking at power or hardware stuff.

Thanks J-M-L. Can I assume that there are no glaring errors in my code? Five or six days is a long time to run without faulting. A quick power of and on and the time resets and off it goes. It is only driving the 7 segment display energising the relay once a day and I reset the relay once a day. I do get the feeling that something is building up which eventually stopes the code running.

can you post the full Code as an attachment?

One possibility is that your I2C communication is hanging up. The Arduino Wire library (if that's what your RTC or display libraries use) can hang when there is an error on the I2C bus.

There are alternative I2C libraries that include a timeout (see here for example), but you would have to rewrite your libraries to make use of the alternative I2C functions.

I2C bus can also get locked up
The code on this page
https://www.forward.com.au/pfod/ArduinoProgramming/I2C_ClearBus/index.html
tries to clear the bus. Perhaps combine it with the timeouts in the suggested replacement library above @J-M-L.

philconduit:
Hi. My alarm clock sketch stops running after five or six days, so I am presuming it is a memory problem.

How does the "stop running" present?

Hi J-M-L I have attached the .ino to post 2
Hi Professor-Chaos. Very Interesting about the IC2 Library.
Hi drmpf. Thanks for the link.
Hi SteveMann. The clock stops the display stays showing the same time. I don’t think it is just a problem with the display because if the program was still running I would be able to change the time even if the display was stuck. This a cannot do.

I have just had a quick look at the IC2 bus problems. Question, if the bus locks up would it stop the sketch running or Just the IC2 devices? I am also writing the alarm time to the RTC. I think this is only happening when the alarm time is being changed. Could this cause problems?

I'd, whiles trying to figure the mess out, put in print statements.

void loop() {
Serial.println( "! ");
  Rtc();
Serial.println( " @");
  Glow();
Serial.println( "# ");
  SwitchLogic();
Serial.println( " $");
  RotarySwitch();
Serial.println( " %");
  Clock();
Serial.println( " ^");
  RelayControl();
Serial.println( " &");
  encoder.tick();
Serial.println( " *");
  Menu();
Serial.println( " (");
  MenuButton.update();
Serial.println( " )");
  AlarmSw1.update();
Serial.println( " ~");
  AlarmSw2.update();
Serial.println( " _");
  RadioSw1.update();
Serial.println( " +");
  RadioSw2.update();
Serial.println( " =");
  EndButton.update();
Serial.println( " ?");

} //End loop

Now when the program stops the last print statement could be the area to look into.

I had an issue that showed up once in 3 months or so, It was a real bear to find. Once I got the task that was causing the error, it was a quick fix but finding the location of the error took patience.

Seems you are really using a shotgun approach on the Display by calling Glow() and Clock() at every loop and sending commands even if nothing has changed.

It might be useful to try sending Display commands only when change are needed (a new minute, a new hour or only once when the brightness needs to be adjusted).

Hi Idahowalker, please bear with me on this it’s my first project. I like the idea of being able to see where the program stops but surely I would have to leave the alarm clock attached to a pc all the time and I thought even clicking the “serial monitor” button would restart the program and clear the prints.

J-M-L Could what you suggest work by putting the RTC into the loop and use it to call the other functions, say once a second for the Clock()

philconduit:
Hi Idahowalker, please bear with me on this it’s my first project. I like the idea of being able to see where the program stops but surely I would have to leave the alarm clock attached to a pc all the time and I thought even clicking the “serial monitor” button would restart the program and clear the prints.

You could use LEDS. Have a series of LED's that light and turn off after each operation.

void loop() {
blue led on
  Rtc();
red on
  Glow();
yellow on
  SwitchLogic();
green on
  RotarySwitch();
blue off
  Clock();
red off
  RelayControl();
yellow off
  encoder.tick();
green off
  Menu();
  blue and green on
  MenuButton.update();
  blue on green red on
  RadioSw1.update();
blue off green on red on
  RadioSw2.update();
blue on red on green on
  EndButton.update();
all off

} //End loop

or some combination that is unique to each step.

Me. I'd leave it plugged into the monitor but you can use other things as well.