Go Down

Topic: Program freezes after a while (Read 1 time) previous topic - next topic

giorgisp

Feb 03, 2013, 01:10 pm Last Edit: Feb 04, 2013, 02:59 pm by giorgisp Reason: 1
I am using a standalone atmega8 on a simple custom made pcb, and programming it with the arduino software.

The device I am making is a controller for relays and can also read sensors such as thermistors and ldr's. It is connected to another arduino (the master controller), via half-duplex RS-485 (using the MAX485 RS-232/RS-485 converter IC), and listens for commands from it (to turn on/off an output, to read a sensor etc.). It also has buttons to manually toggle an output.

Everything works perfectly (it accepts commands, executes the requested action and replies back), until the microcontroller freezes. It can't possibly be an SRAM problem, since the code uses no strings, large arrays or other ram eating stuff. This is driving me nuts. I mean, after 100-200 commands it freezes completely. There are also times where it stops responding to serial input but keeps responding to the buttons. Most of the times though it freezes completely (I can tell because there's a heartbeat led which normally blinks every 2 seconds, and even that stops blinking).

I'd really appreciate any help. The program is attached below.

wildbill

Do you have any way to get debug info out of the system? If not, I'd be inclined to use softwareserial to talk to the other arduino and leave the hardware serial port free for debugging. I'd be interested in the value of switch_count. Depending on the setup in EEPROM, it looks like there is potential for it to be greater than max_switches.

robtillaart

I looked quickly at your code and I had a hard time to understand the code so I cannot give the answer to your question.

your original code has
Code: [Select]
if (EEPROM.read((command_in[2] + 30)) == 'i') { // convert pin number to the EEPROM address its mode is saved at & ensure that the pin is an output
do you see the mismatch between code and comment?


You need to spend some time to refactor the code so the names of the variables and functions become more meaningful.
That way you do not need so much comments to explain what everything does.

Also sometimes code looks better when using the switch statement iso multiple ifs

To show what I mean, I have rewritten on of the functions
Code: [Select]

byte write_output()
{
 int pin = command_in[2];
 if (pinNotValid(pin)) return 'e';

 int address = pin + 30;
 if (EEPROM.read(address) != 'o')  return 'e';  

 switch(command_in[3])
 {
 case 'h' : digitalWrite(pin, HIGH);
   break;
 case 'l' : digitalWrite(pin, LOW);
   break
 case 't': digitalWrite(pin, !digitalRead(pin));
   break;
 default:
   return 'e';
 }
 return 's';
}

// can be reused
boolean pinNotValid(int nr)
{
 return (pin< 5 || pin > 13);
}


Next step in readability is use symbolic error names (so you can differentiate between types of errors)

Code: [Select]

// somewhere in the top of your sketch
#define SUCCES                    0
#define INVALID_PIN_ERROR  -1
#define INVALID_COMMAND -2
#define INVALID_PINMODE  -3
//etc

...

int write_output()
{
 int pin = command_in[2];
 if (pinNotValid(pin)) return INVALID_PIN_ERROR  ;

 int address = pin + 30;
 if (EEPROM.read(address) != 'o')  return INVALID_PINMODE;  

 switch(command_in[3])
 {
 case 'h' : digitalWrite(pin, HIGH);
   break;
 case 'l' : digitalWrite(pin, LOW);
   break
 case 't': digitalWrite(pin, !digitalRead(pin));
   break;
 default:
   return INVALID_COMMAND ;
 }
 return SUCCES;
}

This way the code needs almost no comment as it becomes quite readable in itself

Try to rewrite your code in this way and see

OK, one extra
Code: [Select]

int read_digital()
{
 int pin = command_in[2];
 if (pinNotValid(pin)) return INVALID_PIN_ERROR  ;

 int address = pin + 30;
 if (EEPROM.read(address) != 'i')  return INVALID_PINMODE;  

 if (digitalRead(pin) == HIGH) return 'h';
 return 'l';
}


Hope this helps


Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

giorgisp

#3
Feb 03, 2013, 03:01 pm Last Edit: Feb 03, 2013, 03:46 pm by giorgisp Reason: 1

Do you have any way to get debug info out of the system? If not, I'd be inclined to use softwareserial to talk to the other arduino and leave the hardware serial port free for debugging. I'd be interested in the value of switch_count. Depending on the setup in EEPROM, it looks like there is potential for it to be greater than max_switches.


I am testing the code with a configuration of 3 relays (and 3 buttons to manually control them), 1 temp sensor and 1 ldr. I don't think the problem is caused because of an overflow of that array, since there are 3 of them, and also the problem would probably appear immediately as the array was populated at the beginning of the sketch. There are 9 free pins on the arduino so there can be up to 4 relay/button pairs, so that's why I have put 4 as the maximum number of switches.

Unfortunately I can't use the hardware port for debugging because I can't change the hardware design that easily (it's a homemade etched pcb).


I looked quickly at your code and I had a hard time to understand the code so I cannot give the answer to your question.

your original code has
Code: [Select]
if (EEPROM.read((command_in[2] + 30)) == 'i') { // convert pin number to the EEPROM address its mode is saved at & ensure that the pin is an output
do you see the mismatch between code and comment?

...
Hope this helps


I copied and pasted some parts and probably forgot to change the comments so that's why. I am indeed getting a bit confused while reading through this code and I have already decided to re-write it to make it more readable. Thank you very much for your tips.

Update: robtillaart, I'm applying the changes you suggested and the code already seems much better  :)

I am still hoping that someone can spot the cause of the freezing.

robtillaart

When the code is rewritten and better readable (and you post it) I will have a 2nd look
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Go Up