Problems with strcpy

Hi all!

My first post on this great forum :slight_smile:

First of all, I am a beginner so do not be too hard on me ...
I have a program that writes strings to a LED matrix. These come via a UDP server. I look at the first character in the string to estimate the string that is to be updated.
My problem is that strcpy seems to destroy something which is demonstrated in the if (readString.substring (0,1) ==" 1 ") can only be run once.

Debug from first time a string is sent:

This is the data! 
1Hi! 
This is the read string! 
1Hi! 
This is one substring 
1 
IF 1 
This is the charBuf: 
Hi!

Debug second time a string is sent:

This is the data! 
1Hi! 
This is the read string! 
1Hi! 
This is one substring 
1

The program will not not get into the if statement though it's a 1 as the first character.
Would be grateful for any tips on how I should do to resolve this.

Regards, Rickard

Here is the code

#include <MD_Parola.h>
#include <MD_MAX72xx.h>
#include <SPI.h>
#include <EtherCard.h>
#include <IPAddress.h>


#define STATIC 1  // set to 1 to disable DHCP (adjust myip/gwip values below)

#if STATIC
  // ethernet interface ip address
  static byte myip[] = { 192,168,17,182 };
  // gateway ip address
  static byte gwip[] = { 192,168,17,31 };
#endif

// ethernet mac address - must be unique on your network
static byte mymac[] = { 0x70,0x69,0x69,0x2D,0x30,0x31 };

byte Ethernet::buffer[500]; // tcp/ip send and receive buffer




#define	MAX_DEVICES	2
#define	CLK_PIN		3  // or SCK
#define	DATA_PIN	4  // or MOSI
#define	CS_PIN		5  // or SS


MD_Parola P = MD_Parola(DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);

#define	PAUSE_TIME		1000	// in milliseconds
#define	SPEED_DEADBAND	5		// in analog units

// Global variables


String readString = ""; //string for fetching data from address

textEffect_t effect[] =
{
  PRINT,
  SLICE,
  WIPE,
  WIPE_CURSOR,
  OPENING,
  OPENING_CURSOR,
  CLOSING,
  CLOSING_CURSOR,
  BLINDS,
  DISSOLVE,
  SCROLL_UP,
  SCROLL_DOWN,
  SCROLL_LEFT,
  SCROLL_RIGHT,
  SCROLL_UP_LEFT,
  SCROLL_UP_RIGHT,
  SCROLL_DOWN_LEFT,
  SCROLL_DOWN_RIGHT,
  SCAN_HORIZ,
  SCAN_VERT,
  GROW_UP,
  GROW_DOWN,
};



uint8_t	curString = 0;
char	*pc[] = 
{ 
  "1",
  "2"
};

char	*startup[] = 
{ 
  "Display Serv"
};


#define	NEXT_STRING	((curString + 1) % ARRAY_SIZE(pc))

//callback that prints received packets to the serial port
void udpSerialPrint(word port, byte ip[4], const char *data, word len) {
  IPAddress src(ip[0], ip[1], ip[2], ip[3]);
  Serial.println(src);
  Serial.println(port);
  Serial.println(data);
  Serial.println(len);
  
  Serial.println("This is the data!");
  Serial.println(data);
  readString = data;
  Serial.println("This is readString!");
  Serial.println(readString);
  
  Serial.println("This is substring 1");
  Serial.println(readString.substring(0,1));   
  
   
  if (readString.substring(0,1) == "1")
  {
    Serial.println("If 1");
    char charBuf[10];
    readString.substring(1).toCharArray(charBuf, 8);
    Serial.println("This is the charBuf: ");
    Serial.println(charBuf);
    strcpy(pc[0], charBuf);
  }

  if (readString.substring(0,1) == "2")
  {
    Serial.println("If 2");
    char charBuf[20];
    readString.substring(1).toCharArray(charBuf, 8);
    Serial.println("This is the charBuf: ");
    Serial.println(charBuf);
    strcpy(pc[1], charBuf);
  }

  
}


void setup(){
  Serial.begin(57600);
  Serial.println(F("\n[Displayserver UDP 328]"));

  if (ether.begin(sizeof Ethernet::buffer, mymac, 10) == 0)
    Serial.println(F("Failed to access Ethernet controller"));
  #if STATIC
    ether.staticSetup(myip, gwip);
  #else
    if (!ether.dhcpSetup())
      Serial.println(F("DHCP failed"));
  #endif

  ether.printIp("IP:  ", ether.myip);
  ether.printIp("GW:  ", ether.gwip);
  ether.printIp("DNS: ", ether.dnsip);

  //register udpSerialPrint() to port 8888
  ether.udpServerListenOnPort(&udpSerialPrint, 8888);




  Serial.println("Display-Server started");

  // parola object
  P.begin();
  P.displayText(startup[0], CENTER, P.getSpeed(), PAUSE_TIME, PRINT, PRINT);
  delay(2000);
  P.displayText(pc[curString], CENTER, P.getSpeed(), PAUSE_TIME, PRINT, PRINT);
  curString = NEXT_STRING;

  P.setTextEffect(effect[10], effect[10]);
  P.displayReset();
  P.setSpeed(10);
  P.setPause(2000);
  P.setIntensity(0);
  }

void loop(){
  //this must be called for ethercard functions to work.
  ether.packetLoop(ether.packetReceive());


  if (P.displayAnimate()) 
    {
      P.setTextBuffer(pc[curString]);
      P.displayReset();
      curString = NEXT_STRING;

    }
  }

My problem is that strcpy seems to destroy something which is demonstrated in the

if (readString.substring (0,1) ==" 1 ")

can only be run once.

How can a 1 element substring equal a 3 element string?

    strcpy(pc[0], charBuf);

How big is pc[0]? Not 10 chars. Quit shitting on other memory.

Something was apparently wrong with the formatting when I copied the bit of code. It looks like this:

if (readString.substring(0,1) == "1")

You are still overwriting memory you don't own. pc[0] should NOT be a pointer. It should be an array.

My lack of knowledge ...
Has now set the length of charBuf to "len", but no difference...

if (readString.substring(0,1) == "1")
  {
    Serial.println("If 1");
    char charBuf[len];
    readString.substring(1).toCharArray(charBuf, len);
    Serial.println("This is the charBuf: ");
    Serial.println(charBuf);
    strcpy(pc[0], charBuf);

How should I write the pc[] array??

Hi Gus,

I too have a similar problem with strcpy.

I declare a global 2 dimensional character array :

char Mp3Files[25][50];

So this should hold 25 strings of up to 50 characters.

But later on if I do this :

strcpy(Mp3Files[0],"hello there");

The whole this crashes out - it works if I remove the strcpy line !?!?!?

If I define a single character area :

char tmpstr[50];

then do strcpy(tmpstr,"hello") then no problems.

My question is if I declare a global 2 dimensional character array is the memory actually allocated or do I need to malloc it ???

char Mp3Files[25][50];

Let's hope you've got 1250 bytes of RAM to spare.

How should I write the pc[] array??

As a 2 dimensional array. It is up to you to determine the values of each dimension (how many records, how long is each record, for instance).

@AWOL - yes, I wish this was the issue but even if I change the array to be Mp3Files[3][10] the strcpy still writes over stuff it shouldn't.

Then your arrays are not nul ('\0')terminated.

but even if I change the array to be Mp3Files[3][10] the strcpy still writes over stuff it shouldn't.

And, how many characters are there in "hello there"?

PaulS:

How should I write the pc[] array??

As a 2 dimensional array. It is up to you to determine the values of each dimension (how many records, how long is each record, for instance).

Managed to get it to work now!

char pc[4][10]={
{1},
{2},
{3},
{4}
};

did the trick :slight_smile:

Thank's for guiding me right.

/Rickard

Or even better:

char	*startup[] =

and startup strings

  strcpy(pc[0], "1");
  strcpy(pc[1], "2");
  strcpy(pc[2], "3");
  strcpy(pc[3], "4");
    strcpy(pc[0], charBuf);

Here 'pc[0]' is not the address of the first character in the array. It is the content of that character. If there's a linefeed there, then strcpy is trying to copy charBuf to memory starting at address 0x0A.
You should use either

    strcpy(&pc[0], charBuf);

or

    strcpy(pc, charBuf);

Pete

Here 'pc[0]' is not the address of the first character in the array.

In the original code, pc was an array of pointers, so pc[0] is a pointer, and is perfectly correct to use, as long as it actually points to enough memory (which it didn't).

My issue has been SOLVED :slight_smile:

@AWOL was correct - I ran out of memory.

Here's the thing - I thought I had plenty but didn't realize the difference between SRAM and Flash memory - Doh !!

Wish I'd read the documentation :

Notice that there's not much SRAM available in the Uno. It's easy to use it all up by having lots of strings in your program. For example, a declaration like:
char message[] = "I support the Cape Wind project.";
puts 33 bytes into SRAM (each character takes a byte, plus the '\0' terminator). This might not seem like a lot, but it doesn't take long to get to 2048, especially if you have a large amount of text to send to a display, or a large lookup table, for example.
If you run out of SRAM, your program may fail in unexpected ways; it will appear to upload successfully, but not run, or run strangely. To check if this is happening, you can try commenting out or shortening the strings or other data structures in your sketch (without changing the code). If it then runs successfully, you're probably running out of SRAM.

Thanks to all for the input and help

To check if this is happening, you can try commenting out or shortening the strings or other data structures in your sketch (without changing the code). If it then runs successfully, you're probably running out of SRAM.

There are, of course, more robust methods...

http://playground.arduino.cc/Code/AvailableMemory

And, there are ways to keep literals out of SRAM, too:
Serial.print("This string occupies SRAM");
Serial.print(F("This one does not"));

My problem is solved !!!

@AWOL was right I had used up all my memory - I hadn't realized the difference between SRAM and Flash memory.

Thanks all for the help