Arduino Mega + Ethernet shield crashing

Hi,

I’m very new to Arduino so please be understanding if I’ve made some schoolboy errors in my code! I have written a small program to receive a UDP data stream from X-Plane flight simulator, extract some data and then display it on a 6 digit 7-segment LED display using a MAX7219 driver.

It kind of works, but crashes. Here’s the code - please excuse the commented out junk:

#include <LedControl.h>
#include <SPI.h>         
#include <Ethernet.h>
#include <EthernetUdp.h>

//Let's create our program variables

unsigned long LEDdelaytime=500;                            //---- Delay for LED init sequence
byte mac[] = { 
  0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };       //---- MAC Address assigned to Arduino Ethernet Shield
IPAddress ip (192, 168, 120, 49);                          //---- Set IP address for Ethernet shield (any free addr in your local network)
IPAddress ipx (192, 168, 120, 50);                         //---- IP address of the computer with X-Plane installed

unsigned int localPort = 49000;                            //---- Port to listen for UDP packets from X-plane (default)
unsigned int  XP_port = 49001;                             //---- Port to send UDP packets
byte DATA_buf[512];                                        //---- UDP.read buffer (large enough to store all incoming data of one packet)
int packetSize;                                            // size of received packet

/*
Use a union to move data between float and bytes types.
 To move a float to byte - bf.floatvalue = 4500; byte1 = bf.bytevalue[1]; and so on
 */

union {                                    
  char bytevalue[4];                        
  float floatvalue;
  int intvalue;
}  
bf;

//---- Let's set some variables for various XP functions here

int Gear = 2;           //Contact gear_lever (on/off) on Pin 2 Arduino (if using only one input for Gear Up/Dn)
int Com1A;              //Com1 Active Frequency
int Com1A_prev;         //Previous Com1 Active Frequency
int Com1P;              //Com1 Passive Frequency
int Com1P_prev;         //Previous Com1 Passive Frequency

/*
 We need a LedControl to work with.
 - pin 47 is connected to the DataIn 
 - pin 48 is connected to the CLK 
 - pin 49 is connected to LOAD 
 We have only a single MAX72XX.
 */
LedControl lc=LedControl(47,48,49,1);

//---- EthernetUDP instance to send/receive packets over UDP
EthernetUDP Udp;

/*
  This method will bounce a line back and forth across the 7-segemnt displays
 to show the LED control has been initialised.
 */
void scrollInit() {
  for (int i=0;i<6;i++) {
    lc.setChar(0,i,'-',false);
    delay(LEDdelaytime);
    lc.setChar(0,i,' ',false);
  }
  for (int i=4;i>=0;i--) {
    lc.setChar(0,i,'-',false);
    delay(LEDdelaytime);
    lc.setChar(0,i,' ',false);
  }
}

/*
This function will display a Com frequency on the desired MAX72xx Display
 */
void DisplayCom(int LED_disp, int Freq){
  Serial.print("Display function started");
  Serial.println(); 
  //Make it in to a string so we can test the length
  String FreqStr(Freq, DEC); //Freezes here on 3rd run
  Serial.println(FreqStr);
  //define Char[] array
  char FreqCharMsg[FreqStr.length()];
  // Convert integer data to Char array 
  itoa(Freq,FreqCharMsg,10);
  //Now output the array to the 7-Segs
  for (int n=0; n<FreqStr.length(); n++) {
    Serial.print("Loop # ");
   Serial.println(n);
    if (n == 2) lc.setChar(LED_disp,(FreqStr.length()-n),FreqCharMsg[n],true);  // Set decimal point after 3rd digit
    else lc.setChar(LED_disp,(FreqStr.length()-n),FreqCharMsg[n],false);    
  }
}

void setup() {

  // Begin serial session for debugging
  Serial.begin(9600);    //Comment this out before using

  // INIT LCD DRIVER
  /*
   The MAX72XX is in power-saving mode on startup,
   we have to do a wakeup call
   */
  lc.shutdown(0,false);
  /* Set the brightness to a medium values */
  lc.setIntensity(0,8);
  /* Set the number of devices */
  lc.setScanLimit(0,6);
  /* and clear the display */
  lc.clearDisplay(0);
  scrollInit();
  // END INIT LCD DRIVER

  // Start Ethernet class and UDP session
  //  Ethernet.begin(mac, ip, gateway, subnet);                                  // start the Ethernet class
  Ethernet.begin(mac, ip);                                  // start the Ethernet class
  //Lets echo the IP to serial
  // print your local IP address:
  Serial.print("My IP address: ");
  ip = Ethernet.localIP();
  for (byte thisByte = 0; thisByte < 4; thisByte++) {
    // print the value of each byte of the IP address:
    Serial.print(ip[thisByte], DEC);
    Serial.print("."); 
  }
  Serial.println();

  Udp.begin(localPort);                                    //..and begin UDP session:

  // Set port modes for inputs and outputs
  // pinMode(Gear, INPUT_PULLUP);     // --- PULLUP for setting "High" when pin open (LOW when on the GND)

}


void loop() { 
  //------------------------------- RECEIVING   DATA   FROM X-PLANE ---------------------------------------------------
  //------------------------------- UDP-Packet Reading ------------------
  //  lc.setLed(0,0,0,true);
  //  Serial.print("Tick");
  //  Serial.println();
  packetSize = Udp.parsePacket();                          //  Checks for the presence of a UDP packet, and returns its size
  if (Udp.available())                                     //  UDP packet was received and its size defined (if not, this program block skipped)
    //  if (packetSize)
  {  
    Udp.read ( DATA_buf, packetSize );                     // Reads UDP data from the specified buffer.All incoming DATA stores in DATA_buf 
    Serial.print("Got a Packet");
    Serial.println();
    /*
    ----- Code to work with data stored in DATA_buf (bytes array) 
     Start reading buffer skipping the first 5 byte (header),
     1) read 1st and each 36th byte for specific index numbers identificaton
     2) read needed bytes (params) next to the each index# : index+4, index+8, ......  index+32             
     */
    for (int i = 5; i < packetSize; i += 36)    // here we read each 36th byte for looking up Group numbers
    { 

      switch (DATA_buf[i])                     //--- checking presence of incoming data groups (group number)
      {

      case 96:      //------------------------------ Flaps Position  -------------------   

        //Extract Active Com1 frequency
        for (int n=0; n<4; n++) bf.bytevalue[n] = DATA_buf[i+4+n];
        //Turn it in to an int
        Com1A = (bf.floatvalue);
        Serial.print("Com1 Active = ");  
        Serial.println(Com1A);

        //Check if value has changed since last time - if so update display
        //if (Com1A != Com1A_prev) {
        //Update display
        DisplayCom(0,Com1A);
        //Set Com1A_prev
        //Com1A_prev = Com1A;
        //if (Com1ACharMsg[4] == '2'|'7') lc.setChar(0,0,'5',false);
        //else lc.setChar(0,0,'0',false);
        //}

        delay(LEDdelaytime);
        delay(LEDdelaytime);

        //Extract Passive Com1 frequency
        for (int n=0; n<4; n++) bf.bytevalue[n] = DATA_buf[i+8+n];
        //Turn it in to an int
        Com1P = (bf.floatvalue);
        Serial.print("Com1 Passive = ");  
        Serial.println(Com1P);
        //Check if value has changed since last time - if so update display
        //if (Com1P != Com1P_prev) {
        //Update display
        DisplayCom(0,Com1P);
        //Set Com1A_prev
        //Com1P_prev = Com1P;
        //}

        delay(LEDdelaytime);
        delay(LEDdelaytime);
        
        
        break;
      }
    }

  }            // END of UDP packed Reading   

}

The essence of what this is doing is receiving a UDP packet, extracting the interesting data (which in this case is a float). I’m then reading the float in to an INT as I just need a string of numbers I don’t want a decimal in the value I use for calculations. I pass this int over to a function which will display it correctly on a 7-segment display.

The function takes the INT value (and display number which for now is always 0 as I just have one display), I make it a string object so I can use the ‘.length’ function to get it’s length. Next I define a char array of the required length and using itoa() load the array with my int value. Using this I can loop through each digit and output to the display.

This works initially but then locks up - it gets the Com1A value and displays, gets the Com1P value and displays that, then it freezes. I can only assume that something is crashing. Using the serial prints to track down where it is failing I see that the tird time it jumps to the ‘DisplayCom’ function it prints “Display function started” to the serial port and then stops which leads me to believe the problem is with:

String FreqStr(Freq, DEC); //Freezes here on 3rd run

This is where I’m stuck. I suspect I might be running out of memory because of the way I’m using the string functions and itoa - I don’t fully understand pointers or how it’s working, I’ve just found examples from people trying to do similar things and through trial and error put together something that seemed to work.

Hopefully someone can show me the error of my ways?

please excuse the commented out junk:

No. There is no excuse for posting commented out code unless the commented out code, when uncommented IS the problem.

  String FreqStr(Freq, DEC); //Freezes here on 3rd run

So? Quit using Strings. They are known to cause problems.

  char FreqCharMsg[FreqStr.length()];

Really not a good idea. Pick a maximum reasonable size.

        //Turn it in to an int
        Com1A = (bf.floatvalue);

Pretty stupid comment, considering that that is NOT what the code is doing.

(Is) (there) (some) (reason) (for) (the) (parentheses) (?)

        delay(LEDdelaytime);
        delay(LEDdelaytime);

Maybe you need to do this more than twice...

and displays that, then it freezes. I can only assume that something is crashing.

Or that you are writing off the end of an array, CAUSING the crash. Which seems far more likely to me.

I suspect I might be running out of memory because of the way I'm using the string functions

There are things you don't do, like using the F() macro, which could help, but it is probably NOT string functions that are screwing you. It is the misuse of Strings. Strings and strings are NOT the same thing. Not at all.

Well thanks, that was a really constructive and helpful response :(

Your code is too confusing to debug.

I recommend starting with the basic UDP packet receive with the serial monitor. Then you can add the lcd display once you are certain the UDP part is functioning the way you want.

I recommend staying away from the String data type.

I didn't think it was that confusing, I will strip out all of the commented out bits and re-post it.

With regards to the UDP part that is working 100% correctly and does not crash, I fully tested it before starting to try to output to the 7-seg displays. In addition the actual LED control also works fine.

Where I am having issues is taking the float value received via UDP and converting that in to a char array which I can output to the LED display.

I thought:

Com1A = bf.floatvalue;

Is a 'truncating conversion' which is a valid method of converting from a float to an int but PaulS suggests this is not correct?

I just need to take a value like '11552.00' and load '1,1,5,5,2' in to a char array which I can cycle through and output to the correct LED segments. Is there a simple way of doing this that I haven't discovered yet?

OK, good. If you checked them one at a time, that is the correct way to do it.

Strip out the extra stuff and repost your code. I still recommend staying away from the String data type. I use character arrays instead. It makes your code much more stable.

You might need to use dtostrf() to convert the float to a string, then use atoi() to convert the whole number part of the float to an integer. Do you know what the maximum value of the float will be?

Is a 'truncating conversion' which is a valid method of converting from a float to an int but PaulS suggests this is not correct?

The comment is misleading. bt.floatvalue is making the byte array into a float, which is not what the says is happening.

That the float is then stored in an int is not clear from the comment or the variable name.

Hopefully this is cleaner:

#include <LedControl.h>
#include <SPI.h>         
#include <Ethernet.h>
#include <EthernetUdp.h>

//Let's create our program variables

unsigned long LEDdelaytime=500;                            //---- Delay for LED init sequence
byte mac[] = { 
  0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };       //---- MAC Address assigned to Arduino Ethernet Shield
IPAddress ip (192, 168, 120, 49);                          //---- Set IP address for Ethernet shield (any free addr in your local network)
IPAddress ipx (192, 168, 120, 50);                         //---- IP address of the computer with X-Plane installed
unsigned int localPort = 49000;                            //---- Port to listen for UDP packets from X-plane (default)
unsigned int  XP_port = 49001;                             //---- Port to send UDP packets
byte DATA_buf[512];                                        //---- UDP.read buffer (large enough to store all incoming data of one packet)
int packetSize;                                            // size of received packet

/*
Use a union to move data between float and bytes types.
 To move a float to byte - bf.floatvalue = 4500; byte1 = bf.bytevalue[1]; and so on
 */

union {                                    
  char bytevalue[4];                        
  float floatvalue;
  int intvalue;
}  
bf;

//---- Let's set some variables for various XP functions here

int Gear = 2;           //Contact gear_lever (on/off) on Pin 2 Arduino (if using only one input for Gear Up/Dn)
int Com1A;              //Com1 Active Frequency
int Com1A_prev;         //Previous Com1 Active Frequency
int Com1P;              //Com1 Passive Frequency
int Com1P_prev;         //Previous Com1 Passive Frequency

/*
 We need a LedControl to work with.
 - pin 47 is connected to the DataIn 
 - pin 48 is connected to the CLK 
 - pin 49 is connected to LOAD 
 We have only a single MAX72XX.
 */
LedControl lc=LedControl(47,48,49,1);

//---- EthernetUDP instance to send/receive packets over UDP
EthernetUDP Udp;

/*
  This method will bounce a line back and forth across the 7-segement displays
 to show the LED control has been initialised and is working.
 */
void scrollInit() {
  for (int i=0;i<6;i++) {
    lc.setChar(0,i,'-',false);
    delay(LEDdelaytime);
    lc.setChar(0,i,' ',false);
  }
  for (int i=4;i>=0;i--) {
    lc.setChar(0,i,'-',false);
    delay(LEDdelaytime);
    lc.setChar(0,i,' ',false);
  }
}

/*
This function will display a Com frequency on the desired MAX72xx Display
 */
void DisplayCom(int LED_disp, int Freq){
  Serial.print("Display function started");
  Serial.println(); 
  //Make it in to a string so we can test the length
  String FreqStr(Freq, DEC); //Freezes here on 3rd run
  Serial.println(FreqStr);
  //define Char[] array
  char FreqCharMsg[FreqStr.length()];
  // Convert integer data to Char array 
  itoa(Freq,FreqCharMsg,10);
  //Now output the array to the 7-Segs
  for (int n=0; n<FreqStr.length(); n++) {
    Serial.print("Loop # ");
   Serial.println(n);
    if (n == 2) lc.setChar(LED_disp,(FreqStr.length()-n),FreqCharMsg[n],true);  // Set decimal point after 3rd digit
    else lc.setChar(LED_disp,(FreqStr.length()-n),FreqCharMsg[n],false);    
  }
}

void setup() {

  // Begin serial session for debugging
  Serial.begin(9600);    //Comment this out before using

  // INIT LCD DRIVER
  /*
   The MAX72XX is in power-saving mode on startup,
   we have to do a wakeup call
   */
  lc.shutdown(0,false);
  /* Set the brightness to a medium values */
  lc.setIntensity(0,8);
  /* Set the number of devices */
  lc.setScanLimit(0,6);
  /* and clear the display */
  lc.clearDisplay(0);
  scrollInit();
  // END INIT LCD DRIVER

  // Start Ethernet class and UDP session
  Ethernet.begin(mac, ip);                                  // start the Ethernet class
  //Lets echo the IP to serial
  // print your local IP address to serial
  Serial.print("My IP address: ");
  ip = Ethernet.localIP();
  for (byte thisByte = 0; thisByte < 4; thisByte++) {
    // print the value of each byte of the IP address:
    Serial.print(ip[thisByte], DEC);
    Serial.print("."); 
  }
  Serial.println();

  Udp.begin(localPort);                                    //..and begin UDP session:

}


void loop() { 
  //------------------------------- RECEIVING   DATA   FROM X-PLANE ---------------------------------------------------
  //------------------------------- UDP-Packet Reading ------------------
  packetSize = Udp.parsePacket();                          //  Checks for the presence of a UDP packet, and returns its size
  if (Udp.available())                                     //  UDP packet was received (if not, this program block skipped)
  {  
    Udp.read ( DATA_buf, packetSize );                     // Reads UDP data from the specified buffer.All incoming DATA stores in DATA_buf 
    Serial.print("Got a Packet");
    Serial.println();
    /*
    ----- Code to work with data stored in DATA_buf (bytes array) 
     Start reading buffer skipping the first 5 byte (header),
     1) read 1st and each 36th byte for specific index numbers identificaton
     2) read needed bytes (params) next to the each index# : index+4, index+8, ......  index+32             
     */
    for (int i = 5; i < packetSize; i += 36)    // here we read each 36th byte for looking up Group numbers
    { 
      switch (DATA_buf[i])                     //--- checking presence of incoming data groups (group number)
      {
      case 96:      //------------------------------ Comm Frequencies -------------------   
        //Extract Active Com1 frequency
        for (int n=0; n<4; n++) bf.bytevalue[n] = DATA_buf[i+4+n];
        //Store float value in an int
        Com1A = bf.floatvalue;
        Serial.print("Com1 Active = ");  
        Serial.println(Com1A);
		//Display it on display 0 
        DisplayCom(0,Com1A);
		//Wait a bit
        delay(LEDdelaytime);
        delay(LEDdelaytime);
        //Extract Passive Com1 frequency
        for (int n=0; n<4; n++) bf.bytevalue[n] = DATA_buf[i+8+n];
        //Store float value in an int
        Com1P = bf.floatvalue;
        Serial.print("Com1 Passive = ");  
        Serial.println(Com1P);
        //Display it on display 0 
        DisplayCom(0,Com1P);
        //Wait a bit
		delay(LEDdelaytime);
        delay(LEDdelaytime);
        
        
        break;
      }
    }
  }            // END of UDP packed Reading   
}

In terms of the maximum float value it should never me more than “60000.00” and should never be negative. The decimal could potentially be something other than “.00” but the flight simulator documentation on this is very lacking and through trial I have yet to produce that situation. In theory the radio frequencies use 25Khz spacing so the frequency I have been testing with (shown as 115.52 Mhz in the sim and on my LED display) is really “115.525” but I have yet to find an aircraft model in the sim which displays this full frequency so I can see whether it sends “11552.50” via UDP. I doubt it will as the last digit is implied (.02 and .07 will always be followed by 5, .00 and .05 will always be followed by 0). As I haven’t been able to create this situation yet and I’m still just learning I’m working on the assumption that it will only ever send “xxxxx.00” so I can discard anything after the decimal point.

Use dtostrf(). It will convert your float to a string. Then you should be able to send each character of that string to your lcd display. http://www.atmel.com/webdoc/AVRLibcReferenceManual/group__avr__stdlib_1ga060c998e77fb5fc0d3168b3ce8771d42.html

Thanks, I'll give it a go tonight. How do I verify the length of and reference each character of the string? I think that's why I ended up using a String object - to be able to get the length and also move it to a char array I could easily use to reference each individual character.

http://www.cplusplus.com/reference/cstring/strlen/

Then use a for loop to send each character in the character array to the display.

So thanks for the pointers SurferTim. I have something that works…

The display function is now:

void DisplayCom(int LED_disp, char* Freq){
  //Now output the array to the 7-Segs
  for (int n=0; n<strlen(Freq); n++) {
    if (n == 2) lc.setChar(LED_disp,(strlen(Freq)-n),Freq[n],true);  // Set decimal point after 3rd digit
    else lc.setChar(LED_disp,(strlen(Freq)-n),Freq[n],false);    
  }
}

Other Relevant code changes were :

    //Extract Active Com1 frequency
        for (int n=0; n<4; n++) bf.bytevalue[n] = DATA_buf[i+4+n];
        //Load it in to a char array
        dtostrf(bf.floatvalue,1,0,Com1A);

        //Check if value has changed since last time - if so update display
        if (strcmp(Com1A,Com1A_prev) != 0) {
        //Update display
        DisplayCom(0,Com1A);
        //Set Com1A_prev
        for(int i=0; i<strlen(Com1A); i++){
         Com1A_prev[i] = Com1A[i];
        }
        //strcpy(Com1A, Com1A_prev);  //This is too slow! For loop much faster..
        //if (Com1A[4] == 2|| Com1A[4] == 7) lc.setChar(0,0,'5',false);
        //else lc.setChar(0,0,'0',false);
        }

A couple of bit’s I have commented out there. I found that the strcpy() command appeared to slow down processing noticeably whereby it would take a couple of seconds for the LED display to update - with the for loop moving data instead it’s almost instant.

Also I couldn’t get the comparison working (to add the implied digit based on what digit is stored in Com1A[4]. I tried a few different things but the compiler complained about comparing an int and pointer - guess that’s something for another day but now the basics work.

Thanks again for pointing me in the right direction.