Simple program hangs after long time - tried a lot, please help

Hi,

I wrote a simple program that does three things:

  1. turns a remote switch 433MHz on and off for two timer settings
  2. turns this switch on if the temperature rises beyond a previous average
  3. lets a button turn on / off the switch also

It uses an Arduino Nano, a 433Mhz transmitter and a RTC

Unfortunately the Arduino hangs (not restarts!) after a few days of operation - I thought the millis function could be responsible - but I guess I accounted for that.

After several recursions and tests I’m out of ideas - can anybody please give me a hint?

/*
 02/2017 by BjFo

 */
#include <dht.h>

#include <Wire.h>
#include <Rtc_Pcf8563.h>
 
#define dht_dpin A0 //channel of sensor

dht DHT;
Rtc_Pcf8563 rtc;
float versionNo = 0.89;

//
// Config Logger and Go Alarm Times -------------------------------------------------------------------------
//
const unsigned long logEveryMS = 60000; // delay between each log - 
int maxGoSets = 2; // NUmber of automatic run times (timer settings below)
int goHour[] = {5, 16};
int goMinute[] = {15, 18};

// Confi Transmitter ---------------------------------------------------------------------

int transPin = 8; //for Transmitter
int remoteCode = 67083688;
int remoteUnit = 1;
int remotePeriod = 260;

int buttPIN = 9;

#include <NewRemoteTransmitter.h>
NewRemoteTransmitter InterTechnoTransmitter(remoteCode, transPin, remotePeriod);

unsigned long onTimeManual = 9000000; // 150 miutes
unsigned long onTimeTemperature = 9000000; // 150 miutes
float aveTemp = 20.0; // average temperature - will be set with dht later and then slowly follows the current temp.
float switchThresholdTemp = 3.0; // Switch temperature above average

unsigned long holdTime = 0;

unsigned long safetyTime = 600000;  // repeat on or off after x ms
unsigned long minTime = 120000;  // 2 Minimum on or off time in ms
unsigned long loopTime = 5000;
unsigned long lastCycleEndTime = 0;
unsigned long lastSwitched = 0;
boolean switchStat = false;
boolean manSetTo = false;
boolean setTo = false;

long logID = 0;

void setup()
{
  //int sensorDataPIN = 8; 
  // Open serial communications and wait for port to open:
  Serial.begin(9600);
  
  Serial.print("Humidity and Temperature Version: ");
  Serial.print(versionNo);
  Serial.println("\n\n");
  Serial.println("SWITCH ON in 2 sec");
  delay(2000);
  Serial.println("now");
  InterTechnoTransmitter.sendUnit(remoteUnit, true);
  delay(3000);//Let system settle
  Serial.println("SWITCH OFF in 2 sec");
  delay(2000);
  Serial.println("now");
  InterTechnoTransmitter.sendUnit(remoteUnit, false);

  Serial.println("all done");
  delay(1000);
  
  DHT.read11(dht_dpin);  
  delay(2000);
  aveTemp = float(DHT.temperature); // get start point
}

void(* resetFunc) (void)=0; // decalre Reset Function

void loop()
{
  unsigned long timeComp = millis();

  DHT.read11(dht_dpin);  
  int curTemp = DHT.temperature;
  int curHumi = DHT.humidity;
  aveTemp = ((float)curTemp + (999 * aveTemp)) / 1000; // calculate average Temp over last 1000 minutes
 
  if ( (rtc.getHour() == 3) && (rtc.getMinute() == 58) ) {
    delay(59000); // wait for one minute to avoid another reset
    InterTechnoTransmitter.sendUnit(remoteUnit, 0); // make sure everything is off
    delay(1000);
    resetFunc(); // reset Arduino at 03:59 - unfortunately this did not help!
  }
  
  for (int ii=0; ii < maxGoSets; ii++ ) {  // TIMER CHECK AND RUN
    if ( (rtc.getHour() == goHour[ii]) && (rtc.getMinute() == goMinute[ii]) ) {
      delay(59000); // wait for one minute to avoid another reset
      manSetTo = 1;
      holdTime = lastSwitched + onTimeManual;
      delay(1000);
    }
  }
  
  if ( (millis() > holdTime) && (aveTemp + switchThresholdTemp <= (float)curTemp) ) {  // if temperature is larger than average (by THRESHOLD) then switch ON
    Serial.println("temp switch");
    setTo = 1;
    lastSwitched = millis();
    holdTime = lastSwitched + onTimeTemperature;
  }
  
  if (millis() > holdTime) {
    setTo = 0;
  }
  
  // SWITCH!
  if ( (setTo != switchStat) || (millis() - lastSwitched > safetyTime) ) {
    InterTechnoTransmitter.sendUnit(remoteUnit, setTo);
    switchStat = setTo;
    lastSwitched = millis(); 
  }
  
  digitalWrite(13, setTo);
  
  timeComp = millis() - timeComp; // compensate for measuring time
  unsigned long mbrTime = timeComp;
  
  if ( (timeComp >= logEveryMS) || (timeComp <= 0) ) { 
    timeComp = logEveryMS - 1000; // if things go south...
  }

  manSetTo = switchStat;
  int cLoops = 0;
  for (int i=0; i < 1000; i++ ) {
    delay((logEveryMS - timeComp) / 1000);
    if ( cLoops++ > 10 ) {
      Serial.print("."); // mainly this is done to blink the TX LED to show the switch is still running
      cLoops =0;
    }
    if (digitalRead(buttPIN) == 0){  // Manual button pressed - to turn on/off the switch
      manSetTo = !switchStat;
      holdTime = lastSwitched + onTimeManual;
      digitalWrite(13, manSetTo);
      Serial.print("Button switch to ");
      Serial.println(manSetTo);
    }
  }
  setTo = manSetTo;
  Serial.println(".");
}

What an unholy mess of millis() and delay()s

    holdTime = lastSwitched + onTimeTemperature;
  }
  
  if (millis() > holdTime) {

This will give you problem after roughly 40 days when holdtime overflow before millis.

Does it always hang after the same time or is it different time? If time is different I could suspect power supply. Always the same time you could have a look in the library files for bad coding.

First get rid of all delay()s and use millis() to manage the timing without blocking. And see the proper way to use millis() in Several Things at a Time. Then you will have a consistent system of timing that you (and we) can make some sense of.

Organizing your code into functions as illustrated in Planning and Implementing a Program will also greatly help with debugging.

A common cause of program crashes is writing beyond the end of an array.

...R

int remoteCode = 67083688; Hum . . .

.

Yeah - as you probably guessed this evolved over time into something not really beautiful (not functional as it turns out)

Thanks for your input - I'll rewrite the code from scratch.

@Gabriel_swe - sometimes it lasts for 2-5 days - sometimes just for a couple of hours. But took the advice and started to do it right this time.

Now this is the new code - I’ll report if it works.

/*
 07/2017 by BjF

 */
#include <dht.h>

#include <Wire.h>
#include <Rtc_Pcf8563.h>
 
#define dht_dpin A0 //channel of sensor

dht DHT;
Rtc_Pcf8563 rtc;
float versionNo = 0.91;

//
// Set Timers -------------------------------------------------------------------------
//

int maxGoSets = 2; // Number of automatic run times (timer settings below)
int goHour[] = {5, 16};
int goMinute[] = {15, 18};
int timeInterval = 15; // how long to keep running in minutes

// Confi Transmitter ---------------------------------------------------------------------

int transPin = 8; //for Transmitter
long remoteCode = 67083688;
int remoteUnit = 1;
int remotePeriod = 260;

int buttPIN = 9;

int offHour, offMinute;
float aveTemp = 20.0;
float switchThresholdTemp = 3.0; // Switch Temperature above Average

#include <NewRemoteTransmitter.h>
NewRemoteTransmitter InterTechnoTransmitter(remoteCode, transPin, remotePeriod);

int switchStatus = 0;
int switchStatusOld = 0;

void setup() {
  Serial.begin(9600);
  Serial.print("Temperature Switch Version: ");
  Serial.println(versionNo);

  DHT.read11(dht_dpin);  
  delay(2000);
  aveTemp = float(DHT.temperature); // get start point
}

void loop() {
  checkButton();
  checkTimer();
  checkTemperature();

  sendSwitch();

  Serial.print("Average temperature: ");
  Serial.print(aveTemp);
  Serial.print(" - switch status: ");
  Serial.println(switchStatus);
  delay(100); 
}

void sendSwitch()
{
  if (switchStatus != switchStatusOld) { 
    switchStatusOld = switchStatus;
    InterTechnoTransmitter.sendUnit(remoteUnit, switchStatus);
    digitalWrite(13, switchStatus);
    delay(1000); // for f sake - I know!
  };
}

void checkTimer()
{
  if (rtc.getSecond() == 0) {
    for (int ii=0; ii < maxGoSets; ii++ ) {  // TIMER CHECK AND RUN
      if ( (rtc.getHour() == goHour[ii]) && (rtc.getMinute() == goMinute[ii]) ) {
        switchStatus = 1;
        setOffTime(goHour[ii], goMinute[ii]);
      }
    }
    if ( (rtc.getHour() == offHour) && (rtc.getMinute() == offMinute) ) {
      switchStatus = 0;
    }
  }
}

void checkButton()
{
  if (digitalRead(buttPIN) == 0){
    if (switchStatus == 1) { switchStatus = 0; }
    else  { switchStatus = 1; }
    Serial.print("Manual button switches to ");
    Serial.println(switchStatus);
    if (switchStatus == 1) { setOffTime(rtc.getHour(), rtc.getMinute()); }
  }
}

void checkTemperature()
{
  DHT.read11(dht_dpin);  
  int curTemp = DHT.temperature;
  int curHumi = DHT.humidity;
  if (rtc.getSecond() == 0) {aveTemp = ((float)curTemp + (999 * aveTemp)) / 1000; } // this runs about 10 times per minute - calculates average Temp over last 100 minutes

  if (aveTemp + switchThresholdTemp <= (float)curTemp) {
    switchStatus = 1;
    Serial.println("Temperature switches to 1");
    setOffTime(rtc.getHour(), rtc.getMinute());
  }
}

void setOffTime(int startHour, int startMinute)
{
  offHour = startHour;
  offMinute = startMinute + timeInterval;
  if (offMinute >= 60) {
    offMinute = offMinute - 60;
    offHour++;
    if (offHour >=24) { offHour = offHour - 24;}
  }
  Serial.print("Run until ");
  Serial.print(offHour);
  Serial.print(":");
  Serial.println(offMinute);
}

It still hangs - same symptoms as mentioned above - the thing just stops, the TX LED won't flash and the button won't do anything. It ran well for about 2 to 6 hours before stopping.

I avoided millis() alltogether and relied on the rtc for timing - from my point of view only the rtc requests remain to be to source of the hangs.

Did anybody experience this or something similar before? Could it be a defect in the rtc?

What does this variable represent?

int remotePeriod = 260;

If that was my program I would convert all the times to minutes (ie. hr * 60 + min) or seconds and then all the tests can be simplified.

I would also get the RTC values once at the start of loop() and use those values everywhere rather than reading the clock several times and possibly getting different values.

I don't believe your problem has anything to do with millis() or the RTC - either should do the job perfectly.

...R

Robin2:
I would also get the RTC values once at the start of loop() and use those values everywhere rather than reading the clock several times and possibly getting different values.

Talking about RTC values, OP, are your rtc values what you expect?
If this is the library that you are using then I would have thought that you would have needed to call getDateTime(), or some other function that calls it implicitly, to update the RTC values in rtc.
But maybe I have misunderstood something?

remotePeriod is used for the 433MHz transmitter triggering a radio controlled Intertechno wall socket - I use this for all my Arduino / Intertechno applications but never had any problems connected to the transmitters.

I'll store the time in a global variable as you suggested - but I kinda doubt that it will remedy the problem.

The RTC values are what I'd expect - but I will investigate your point.

I'll also replace the RTC module just to be sure is is not an instable hardware. Probably will also be trying a different power supply.

I think it will be a memory leak. You know when you flash it over, it will say how much memory is left. If you add in another variable as a dummy 'memory block', say an array of strings for example (as a test), that value will go down significantly. If you do it, does the program crash a lot earlier? If it does, you've probably got a memory leak.

The way to get around it is to leave the dummy 'memory block' in the code so that there's barely any memory left, and then comment out the rest of your code. Bit by bit, reintroduce more code, then let it run, repeat until it crashes. The chances are that the last bit of code reintroduced will be responsible for the memory leak. It's probably just something simple which you've missed.

:slight_smile:

what are you trying todo here:

  manSetTo = switchStat;
  int cLoops = 0;
  for (int i=0; i < 1000; i++ ) {
    delay((logEveryMS - timeComp) / 1000);
    if ( cLoops++ > 10 ) {
      Serial.print("."); // mainly this is done to blink the TX LED to show the switch is still running
      cLoops =0;
    }
    if (digitalRead(buttPIN) == 0){  // Manual button pressed - to turn on/off the switch
      manSetTo = !switchStat;
      holdTime = lastSwitched + onTimeManual;
      digitalWrite(13, manSetTo);
      Serial.print("Button switch to ");
      Serial.println(manSetTo);
    }
  }
  setTo = manSetTo;
  Serial.println(".");

what’s magic about 1000 iterations?

that was a very ugly way to read a button while waiting - I scrapped this code.

beejayf:
that was a very ugly way to read a button while waiting - I scrapped this code.

you should update your code, if it still isn't working.

Here is a simple button press example:

const byte buttonPin = 9;

void setup() 
{
  Serial.begin(9600);
  pinMode(buttonPin, INPUT_PULLUP);
}

void loop() 
{
  if(buttonPressed())
  {
    Serial.println(F("Pressed"));
  }
}

bool buttonPressed()
{
  static unsigned long lastMillis = 0;
  static byte lastState = HIGH;
  const byte newState = digitalRead(buttonPin);
  if(newState != lastState)
  {
    if(millis() - lastMillis > 100)
    {
      lastMillis = millis();
      if(newState == LOW) // pressed
      {
        return true;
      }
    }
  }
  return false;
}

beejayf:
remotePeriod is used for the 433MHz transmitter triggering a radio controlled Intertechno wall socket - I use this for all my Arduino / Intertechno applications but never had any problems connected to the transmitters.

I was hoping you would explain what the number is for - the name suggests it is an amount of time? if so what are the time units and how is the time value used?

I'll store the time in a global variable as you suggested - but I kinda doubt that it will remedy the problem.

Even if it does not the simplification of the code should hep to identify the problem wherever it may be.

...R

Holy cow! It appears to have been the power supply - huge 2.4A adapter I use the same type in about 5 or 6 applications - never would have suspected. Still don't know the exact circumstances but it is up and running since about 24 hours.

Thanks everybody!