Run Time error with this code

YairYakov · December 25, 2014, 1:38pm

Hi,
I compiled this code with no errors but for some reason I get run-time error (the device gets stuck):

// ErrorLog.h
#ifndef ERRORLOG
#define ERRORLOG
#define ERR_BUFF_SIZE 8
#define ERR_DISABLE 0
#define ERR_ENABLE 1
enum {
** ERR_NOIP = 0,**
** ERR_CONNECTION,**
** ERR_NTPsync,**
** ERR_SD,**
** ERR_WEB,**
** ERR_ANALOG,**
** ERR_DIGITAL,**
** ERR_ONE_WIRE**
};
char ERR_LOG_TXT[ERR_BUFF_SIZE][100] = { "No IP adress", "Ethernet connection Error", "NTP server sync Error", "SD card Error", "Web server Error", "Analog input Error", "Digital Input Error", "One Wire Error" };
typedef struct
{
** unsigned char status; // enabled/disabled**
** unsigned char counter;**
** unsigned char errorCode;**
** unsigned char trigger;**
} ERR_TYPE;
ERR_TYPE Error[ERR_BUFF_SIZE];
void initERR(void)
{
** char i;**
** //ENABLE all timers**
** for (i = 0; i < ERR_BUFF_SIZE; i++) {**
Error*.status = ERR_ENABLE;
_ Error.counter = 0;
Error.trigger = 0;
Error.errorCode = i;
// Serial<<F(i);_
// Serial<<F(":")<<(ERR_LOG_TXT)<<("- Enable -");
_ }
}
void ErrorHandler(){
char i;
//_
for (i = 0; i < ERR_BUFF_SIZE; i++){
if (Error_.status && Error.trigger){
// Serial << F("HELLO");
// Serial.print("Logging Error Code#:");
// Serial.println(Error.errorCode);_
// Serial.println(ERR_LOG_TXT);
_ Error.counter++;
//do the logging to SDcards here*
Error*.trigger=false; //reset the trigger*
* }
}
}
#endif*
[/b][/quote]
The problem seems to be in void initERR(void) where Error*.status is causing the problem (and any other array calling).*
can anyone help ?
Thank you_

SurferTim · December 25, 2014, 1:45pm

Which Arduino are you using? If an Uno, you are probably running out of SRAM. You are using 800 bytes here.

#define ERR_BUFF_SIZE   8

char ERR_LOG_TXT[ERR_BUFF_SIZE][100] = { "No IP adress", "Ethernet connection Error", "NTP server sync Error", "SD card Error", "Web server Error", "Analog input Error", "Digital Input Error", "One Wire Error" };

YairYakov · December 25, 2014, 2:04pm

I'm using Mega

Binary sketch size: 78,554 bytes (of a 258,048 byte maximum)

SurferTim · December 25, 2014, 2:17pm

The Error structure variable is an array, so I would access it like an array.

typedef struct
{
  unsigned char status;   // enabled/disabled
  unsigned char counter;
  unsigned char errorCode;
  unsigned char trigger;
} ERR_TYPE;

ERR_TYPE Error[ERR_BUFF_SIZE];

  //ENABLE all timers
  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Error[i].status = ERR_ENABLE;
    Error[i].counter = 0;
    Error[i].trigger = 0;
    Error[i].errorCode = i;
   //  Serial<<F(i);
  //   Serial<<F(":")<<(ERR_LOG_TXT)<<("- Enable -");
  }

system · December 25, 2014, 2:30pm

I compiled this code with no errors

loop()?
setup()?

YairYakov · December 25, 2014, 5:07pm

initERR() is being called from setup{}
ErrorHandler() is being called from loop{}

I commented everything related to this header and left only initERR()
the problem is in "Error*.status = ERR_ENABLE;"*
for example this code dosn't cause problem:
typedef struct
{

unsigned char status; // enabled/disabled*
unsigned char counter;*
unsigned char errorCode;*
unsigned char trigger;*
} ERR_TYPE;
ERR_TYPE ErrorOne;
void initERR()
{
ErrorOne.status = ERR_ENABLE;*
ErrorOne.counter = 0;*
ErrorOne.trigger = 0;*
}

system · December 25, 2014, 6:24pm

for example this code dosn't cause problem:

Why the italics?

YairYakov · December 25, 2014, 6:29pm

dont know

system · December 25, 2014, 6:35pm

Well, stop the vague hand-waving, and let's see the source.

SurferTim · December 26, 2014, 1:10am

In this code, the ErrorOne variable is not an array, just a structure, and you are accessing it correctly.

typedef struct
{
  unsigned char status;   // enabled/disabled
  unsigned char counter;
  unsigned char errorCode;
  unsigned char trigger;
} ERR_TYPE;

ERR_TYPE ErrorOne;

void initERR()
{

    ErrorOne.status = ERR_ENABLE;
    ErrorOne.counter = 0;
    ErrorOne.trigger = 0;
}

In this code, the Error variable is an array of structures and you are not accessing it correctly. See reply #3 above.

typedef struct
{
  unsigned char status;   // enabled/disabled
  unsigned char counter;
  unsigned char errorCode;
  unsigned char trigger;
} ERR_TYPE;

ERR_TYPE Error[ERR_BUFF_SIZE];

void initERR(void)
{
  char i;

  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Error.status = ERR_ENABLE;
    Error.counter = 0;
    Error.trigger = 0;
    Error.errorCode = i;
  }
}

YairYakov · December 26, 2014, 3:03pm

I'm sorry - this is the code that causing me problems

typedef struct
{
unsigned char status; // enabled/disabled
unsigned char counter;
unsigned char errorCode;
unsigned char trigger;
}
ERR_TYPE;

ERR_TYPE Error[ERR_BUFF_SIZE];

void initERR(void)
{
char i;
//ENABLE all ErrorLogs
for (i = 0; i < ERR_BUFF_SIZE; i++) {
Error*.status = ERR_ENABLE;*
_ Error*.counter = 0;_
_ Error.trigger = 0;
Error.errorCode = i;
}
}*
In this case Error is an array pf structures and I'm accessing each one inside the for loop
again - what is the problem ?_

SurferTim · December 26, 2014, 3:08pm

That is correct. You are accessing Error as a structure, not an array of structures. Are you not seeing the correction I suggested in reply #3? Are you not scrolling the code box down to see how to access an array of structures? Here it is:

void initERR(void)
{
  char i;

  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Error[i].status = ERR_ENABLE;
    Error[i].counter = 0;
    Error[i].trigger = 0;
    Error[i].errorCode = i;
  }

ShapeShifter · December 26, 2014, 4:11pm

SurferTim has the correct answer. But I don't understand how the original code even compiles.

Error is declared as an array of structures. So, when Error is referenced by itself without an array subscript, it is treated as a pointer to the first element of the array. So, with Error being a pointer to a structure, Error.status should be a compile time error, because the proper way to dereference the pointer would be Error->status.

(Of course, this would only ever access the first element of the array, so using the "->" notation is NOT the solution to the OP's problem where he wants to access every array element in the loop. I only bring up this notation as a contrast to the erroneous "."notation of a pointer.)

This code is definitely erroneous:

    Error.status = ERR_ENABLE;
    Error.counter = 0;
    Error.trigger = 0;
    Error.errorCode = i;

So why does it even compile? It should be a compile time error, and have never made it to a run time error.

_{The forum has two of almost every smiley, but it doesn't have a confused smiley that's scratching its head or has questions marks... Pity, that would be perfect for this post...}

system · December 26, 2014, 4:35pm

The forum has two of almost every smiley, but it doesn't have a confused smiley that's scratching its head or has questions marks... Pity, that would be perfect for this post...

But it does have code tags. . .

YairYakov · December 26, 2014, 4:38pm

Oh I get it now, he he he he
there is i index in Error.Status but the HTML page dosn't show it..
that explains the italic hehhe.. here let me try again uploading my code:

typedef struct
{
  unsigned char status;	// enabled/disabled
  unsigned char counter;
  unsigned char errorCode;
  unsigned char trigger;
} 
ERR_TYPE;

ERR_TYPE Error[ERR_BUFF_SIZE];

void initERR(void)
{
  char i;
  //ENABLE all ErrorLogs
  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Error[i].status = ERR_ENABLE;
    Error[i].counter = 0;
    Error[i].trigger = 0;
    Error[i].errorCode = i;
  }
}

As you can see I have i index and still it causes problem

SurferTim · December 26, 2014, 5:18pm

No it doesn't. It must be something else in your sketch. I tried this test code and it works fine. I stored and then displayed the errorCode.

#define ERR_BUFF_SIZE   8
#define ERR_DISABLE      0
#define ERR_ENABLE      1

typedef struct
{
  unsigned char status;	// enabled/disabled
  unsigned char counter;
  unsigned char errorCode;
  unsigned char trigger;
} 
ERR_TYPE;

ERR_TYPE Error[ERR_BUFF_SIZE];

void initERR(void)
{
  char i;

  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Error[i].status = ERR_ENABLE;
    Error[i].counter = 0;
    Error[i].trigger = 0;
    Error[i].errorCode = i;
  }
}

void printERR(void)
{
  char i;

  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Serial.println(Error[i].errorCode);
  }
}

void setup() {
  Serial.begin(115200);
  initERR();
  printERR();
  
}

void loop() {
}

It displayed this

YairYakov · December 26, 2014, 5:31pm

So the good news is that the code if fine
The bad news is that I have no Idea where is the problem..

ShapeShifter · December 26, 2014, 5:32pm

YairYakov:
Oh I get it now, he he he he
there is i index in Error.Status but the HTML page dosn't show it..
that explains the italic hehhe.. here let me try again uploading my code:

And that's why it's important to use the code tags, and not quote tags or simply placing the code inline.

As you can see I have i index and still it causes problem

OK, that explains why it compiled.

You say you think the error is in the initErr() function. Why do you say that?

It's been stated before that it might be an out of memory error (stack colliding with other memory data values.) It could be. This construct seems to be particularly wasteful of precious RAM:

char ERR_LOG_TXT[ERR_BUFF_SIZE][100] = { "No IP adress", "Ethernet connection Error", "NTP server sync Error", "SD card Error", "Web server Error", "Analog input Error", "Digital Input Error", "One Wire Error" };

This takes up 800 bytes (8 * 100) but only stores 147 useful bytes. That's about a quarter of your available memory wasted on unused space. A more efficient way of storing it would be:

char* ERR_LOG_TXT[ERR_BUFF_SIZE] = { "No IP adress", "Ethernet connection Error", "NTP server sync Error", "SD card Error", "Web server Error", "Analog input Error", "Digital Input Error", "One Wire Error" };

Note that it was changed from an array of 100 character arrays, to now be an array of character pointers. This means that ERR_LOG_TXT is now an array of 8 pointers, each of which points to a character string. It would seem that adding that extra array of pointers would use even more memory, but it ends up using only 163 bytes to store the same information as opposed to 800. With such a limited memory budget, that saving of 637 bytes can make a difference.

The original code allocated 100 bytes for each string, even though the longest one only needs 26 bytes. You could make that array index smaller than 100, but then you have to remember to make it bigger if you ever put in a longer message, plus you are still wasting memory for all of the strings that are shorter than the longest one.

By adding the array of pointers, each string takes up only as much space as needed; they do not have to be padded out to some arbitrary maximum length. The memory savings is well worth adding the extra array of pointers.

Note that the only thing that needs to change is the declaration. The ERR_LOG_TXT array is still accessed the same as it was before since a pointer to a character is logically the same as a character array.

ShapeShifter · December 26, 2014, 5:36pm

With the importance of code tags now understood, would you please post the entire sketch in code tags? Perhaps there have been other bits of code that got swallowed up by the forum formatter.

YairYakov · December 26, 2014, 6:02pm

ShapeShifter:
You say you think the error is in the initErr() function. Why do you say that?

Because I've commented any other calling to this file and left only initERR() and it caused an error
and after I commented initERR() it was fine

ShapeShifter:
It's been stated before that it might be an out of memory error (stack colliding with other memory data values.) It could be. This construct seems to be particularly wasteful of precious RAM:

You are right, i'll change that

Again here is the whole code - I'm calling these functions from the main code

// ErrorLog.h

#ifndef ERRORLOG
#define ERRORLOG

#define ERR_BUFF_SIZE	8

#define ERR_DISABLE		0
#define ERR_ENABLE		1

enum {
  ERR_NOIP = 0,
  ERR_CONNECTION,
  ERR_NTPsync,
  ERR_SD,
  ERR_WEB,
  ERR_ANALOG,
  ERR_DIGITAL,
  ERR_ONE_WIRE
};

char ERR_LOG_TXT[ERR_BUFF_SIZE][100] = { "No IP adress", "Ethernet connection Error", "NTP server sync Error", "SD card Error", "Web server Error", "Analog input Error", "Digital Input Error", "One Wire Error" };

typedef struct
{
  unsigned char status;	// enabled/disabled
  unsigned char counter;
  unsigned char errorCode;
  unsigned char trigger;
} 
ERR_TYPE;

ERR_TYPE Error[ERR_BUFF_SIZE];

void initERR(void)
{
  char i;
  //ENABLE all ErrorLogs
  for (i = 0; i < ERR_BUFF_SIZE; i++) {
    Error[i].status = ERR_ENABLE;
    Error[i].counter = 0;
    Error[i].trigger = 0;
    Error[i].errorCode = i;
  }
}

void ErrorHandler(){
  char i;
  //
  for (i = 0; i < ERR_BUFF_SIZE; i++){
    if (Error[i].status && Error[i].trigger){
	//	Serial << F("HELLO");
   //   Serial.print("Logging Error Code#:");
    //  Serial.println(Error[i].errorCode);
   //   Serial.println(ERR_LOG_TXT[i]);
      Error[i].counter++;
      //do the logging to SDcards here
      Error[i].trigger=false; //reset the trigger

    }
  }
}
#endif

This code is not original coded by me and I'm continuing an existing code.
The project contains one *.ino file and some *.h files but no *.c files at all
inside the *.h there are some functions that are being called
Since this is my first project in arduino I wanted to know if this is the common way to code in arduino.
I know from C that there should be *.h for each *.c file and the *.h should contain definitions and function statements and the *.c should have the function itself.
Maybe this causing some problems ?

Topic		Replies	Views
Run time error Programming	15	2032	May 12, 2021
Runtime error on esp 8266 while reloading client multiple times is restarting the board Programming	1	84	September 5, 2024
Errors in previously compiled code Programming	7	1023	May 5, 2021
Code Error Programming	2	526	May 6, 2021
Error Messages in Arduino IDE [SOLVED] Programming	11	2317	May 5, 2021

Run Time error with this code

Related topics