what causes arduinos to freeze /lockup

I have built an aquarium controller using an Arduino pro mini clone from ebay and I seem to be having some issues with it locking up it will run between 2-4 hours and lock up I have cropped out some of the extra features that I dont need such as iremote and softserial. I am running on 12 hours no issues yet time will see maybe I have used up my hardware resources or perhaps its a wiring problem i have no idea where to start

Let me know if you have any ideas to what is causing it to lock up this is my first arduino project and programming experience should i be looking at hardware or my code for issues

I have attached my code

Controller_v1_3addediremote.ino (11.6 KB)

Hi,

How locked up is it? Can you upload the program again, or do you have to power down/up??

What external devices are you controlling?? Do they have any optical or other isolation??

Where is Ground?? Where do different grounds come together??

See theArduinoInfo.Info WIKI HERE: and the section: http://arduino-info.wikispaces.com/RelayIsolation

Let us know what you see....

Let me know if you have any ideas to what is causing it to lock up

You are using a LOT of SRAM for the literal constants print to the LCD. How you measured how much free memory you have? I'm guessing that it is nowhere near enough.

You can reclaim some using the F() macro:
lcd.print(F("Temp "));

Your code is very hard to read. Using Tools + Auto Format would benefit everyone.

  if(menubutton.uniquePress()||irmenu) menustate++, lcd.clear();
  if (menustate > 6) save(), menustate = 0;

What is this mess? Have you researched what the comma operator does? It does NOT do what you seem to think, as evidenced by this code, what it does.

You should fix every single statement like this. How depends on what you think this code is supposed to do.

I had a brief review of the code and couldn't see any dangerous constructs. I suspect that PaulS has hit the nail on the head with his suggestion of memory exhaustion.

The review turned up some other curiosities that you might want to look at.

  (!sensors.getAddress(Sump, 0));

Huh?

PH = sum/15/46;

It would be better to use the variable samples you have already defined, rather than repeating the value 15 here. The expression has a different value depending on the order of precedence of the / operators. It may well do what you want, but personally I'd prefer to make that precedence explicit since it does matter and is not immediately obvious.

I noticed that save() writes to the EEPROM so I wondered how often you call it. Mostly it looked OK but I wasn't sure how often this expression would be true:

  if (menureset > 200) {menustate = 0; save(); lcd.clear();}

I don't like the use of the comma operator to execute several statements after an if. It would be much better to use a block statement instead, which you have already done in many other cases. I'd go one step further and suggest that you should always use a block statement whenever you have conditional code i.e. in the body of an if, else, do, while, for and do on - even if it only contains a single statement. It prevents a whole host of logic bugs when statements get added/removed later on, or where you are mistaken about which 'else' pairs up with which 'if' when you have nested logic.

I was also intrigued by the choice of name for printTemperature(). :slight_smile:

The first parameter 'setInterval' takes a 'long'.

Undecorated integer literals are of type 'signed int' with a smaller maximum range (?32768 to 32767). Your second use below is out of the range of an 'integer' and must have the suffix 'L' added in order make it a signed 'long' as required by the library.

timer.setInterval(1000L, SecondPulse);
timer.setInterval(3600000L, hourpulse);

I will go through the code with suggestions made and see what I can eliminate to free up sram

for answers to questions

lcd shows time when it stops I know it has locked up and a reset brings it back to life

ground every thing is 5v so I have 1 common ground

components consist of 4 relays optically isolated, ds18b20 temp sensor, lcd on i2c, rtc on i2c, 2 float switches connected to ground and input pins I thought these where the problem because this started when I installed them but stayed after I took them out, and a bluetooth serial board (hc-06).

I did auto format does this just organize spacing etc what does this do

if (menustate > 6) save(), menustate = 0; this would be the same as the following as I understand it or am i wrong

if (menustate > 6){
save()
menustate = 0;
}

(!sensors.getAddress(Sump, 0)); this is a copy of someone elses work and I incorporated it into my own not sure how it works but it defines Sump as the first sensor it gets a address for

PH = sum/15/46; not used I can take it out I havent got PH yet this was the way I was going to do it but now I found a i2c circuit and will need to change the code anyways

save(); is called up anytime I do something in the menu after I get back to the main menu it compares what is in the eeprom to what the controller has if I changed anything it notices and writes my new value to the eeprom

timer.setInterval(1000L, SecondPulse);
timer.setInterval(3600000L, hourpulse); I have timed this and it was working without the long or L and wouldnt it be the hourpulse that you are saying is out of range because second pulse of 1000 is in the range of (?32768 to 32767) or am I reading you wrong

THANKS FOR THE HELP!!!!! I appreciate it this is my first time doing anything more complex then blink

as far as where I should be putting my energy into fixing this should I look at code or hardware and looking at sram I found PROGMEM is this something I should be learning and using

Judging by how many literal strings are in your code, PROGMEM could very well save you a lot of SRAM. If running out of RAM is what is making your Arduino lock up, it's a good thing to try.

However, I think that's pretty much what the F() macro suggested by PaulS does. Did you try that yet? I didn't see any other large chunks of memory in your code.

I havent used the f macro yet I am still working on applying the suggestions I did take out 4 float and 2 int. this takes the control of being able to change the temp set point away from me in the menu but I doubt I will need to change it anyways. This should free up some sram and now I have ran the controller for 16 hrs no problems yet so it seems it is a SRAM problem I will work on fixing other issues as I have free time

is there a better way of doing this I would think there would be a way of saying if eepromread is not equal to alksp then eeprom write alksp

Ralksp = EEPROM.read(0);

if (Ralksp != alksp) {
EEPROM.write(0, alksp);
}

So... does this actually work? Did you mean to use the same address in the EEPROM for both alksp and calsp?

The stuff I read says EEPROM.read() returns a byte. Does that auto-cast into the floats?

 float alksp, calsp;
...
 int Ealksp = 0;
 int Ecalsp = 0;
 int Rcalsp;
 int Ralksp;
...
  alksp = EEPROM.read(Ealksp);
  calsp = EEPROM.read(Ecalsp);
...
  if (Ralksp != alksp) {
  EEPROM.write(Ealksp, alksp);
...
  if (Rcalsp != calsp) {
  EEPROM.write(Ecalsp, calsp);

yes it works although I didnt realize that they where both 0 I changed that already today I have been setting them to the same so it didnt appear to be wrong I have changed the code to the following to reduce SRAM

float alksp, calsp;
...

 int Rcalsp;
 int Ralksp;
...
  alksp = EEPROM.read(0);
  calsp = EEPROM.read(1);
...
  if (Ralksp != alksp) {
  EEPROM.write(0, alksp);
...
  if (Rcalsp != calsp) {
  EEPROM.write(1, calsp);

I have been using the f macro I was wondering if an instance where I only call 1 character such as lcd.print(":"); will it benifit to use lcd.print(F(":"));

will this work

if (alksp != EEPROM.read(0)){ EEPROM.write(0, alksp)}

Does it compile (in your sketch)?

Does it do what you want?

yes it works

can someone with F macro experience look over my code and see if I used it right I am not totally familiar with it also I have reduced several things trying to save resources

Controller_v1_4.ino (11 KB)