Problem with array scope and array content

I'm trying to send data through FS1000A and XY-MK-5V and today has not been my day. But to focus on one problem at a time, I'd like to understand why the following code doesn't work:

void sendMessage(char* pinCode, char *data) { if (strlen(data) > 0) { int msgSize = (strlen(data) + 10);

char packetData[msgSize]; strcat(packetData, pinCode); strcat(packetData, "."); strcat(packetData, data); vw_send((uint8_t *)packetData, msgSize);

vw_wait_tx(); } }

The problem with this code is that the data is being corrupted and I'm getting some sort of mix between the times the function is called. Here is an example:

Output:
First Call: ó2.1
Second Call: ó2.2.1
Third Call: ó2.2.1

Sometimes the output will give me some variation of that, such as a few more 2's or 1's. I found this post (http://forum.arduino.cc/index.php?PHPSESSID=0jlun4hckc2pg3u1frsrmmlob3&topic=44271.msg320565#msg320565) and for some unknown reason, this worked:

void sendMessage(char* pinCode, char *data) { if (strlen(data) > 0) { int msgSize = (strlen(data) + 10); { char packetData[msgSize]; strcat(packetData, pinCode); strcat(packetData, "."); strcat(packetData, data); vw_send((uint8_t *)packetData, msgSize); } vw_wait_tx(); } }

Now, I'm a PHP Developer starting out in the C/Arduino world (never worked with C/C++ before) and I cannot understand why creating a new scope there solved my problem since the function itself is a limited scope.

Deleu:
Now, I’m a PHP Developer starting out in the C/Arduino world (never worked with C/C++ before) and I cannot understand why creating a new scope there solved my problem since the function itself is a limited scope.

My guess is that you haven’t really solved the problem by doing that but simply masked the failure (or changed the failure mode).

Can you post all the code (and use code tags ala http://forum.arduino.cc/index.php?topic=97455.0)?

Also… when you use strcat I typically start with strcpy so I don’t have to make sure the array is empty first. Something like:

      char packetData[msgSize];

      strcpy(packetData, pinCode);
      strcat(packetData, ".");
      strcat(packetData, data);

Otherwise I’d be worried that you may end up with random garbage in packetData (since you didn’t make sure the array was empty).

Regards,

Brad
KF7FER

Does strcpy clean the array for me? Thanks for the advice.

Here is the full code:

#include <VirtualWire.h>

int senderPort = 7;

int greenButton = 4;
int redButton = 3;
int yellowButton = 2;

const int maxLength = 60;

void setup() {
  // Start Serial
  Serial.begin(9600);

  // Setup Sender
  vw_set_ptt_inverted(true);
  vw_setup(2000);
  vw_set_tx_pin(senderPort);

  // Setup Button Input
  pinMode(greenButton, INPUT);
  pinMode(redButton, INPUT);
  pinMode(yellowButton, INPUT);

  // Define Button Values
  digitalWrite(greenButton, HIGH);
  digitalWrite(redButton, HIGH);
  digitalWrite(yellowButton, HIGH);
}

void loop() {
  // serialListen();
  manualListen();
}

boolean greenState = false;

void manualListen(){
  int reading = digitalRead(greenButton);
  if(reading == LOW && !greenState){
    greenState = true;
    char* a = "2";
    char* b = "1";
    sendMessage(a, b);
  }
  else if(reading == HIGH && greenState){
    greenState = false;
  }
}

void sendMessage(char* pinCode, char *data) {
  if (strlen(data) > 0) {
    int msgSize = (strlen(data) + 10);
    {
      char packetData[msgSize];
      strcat(packetData, pinCode);
      strcat(packetData, ".");
      strcat(packetData, data);
      vw_send((uint8_t *)packetData, msgSize);
    }
    vw_wait_tx();
  }  
}

int msgSize = (strlen(data) + 10);

char packetData[msgSize];

You can't do this in C/C++. The array size must be known at compile time. I am surprised you did not get a compile error. See http://www.cplusplus.com/doc/tutorial/arrays/

DavidOConnor:

int msgSize = (strlen(data) + 10);

char packetData[msgSize];

You can't do this in C/C++. The array size must be known at compile time. I am surprised you did not get a compile error. See http://www.cplusplus.com/doc/tutorial/arrays/

Being that true, I should then declare an array with a constant size. Since we have memory limitation, I am the one responsible for declaring it as small as possible?

Also, that being the case, what about the limited scope for the array? Since the variable size is outside and the array is inside, is that why I'm no longer getting errors?

I'm not familiar with the library you are using, but you could declare the array to hold the largest expected message or you could perhaps eliminate the array and make multiple calls to send the data.

Deleu: Does strcpy clean the array for me? Thanks for the advice.

Well strcpy will replace the destination with the contents of the source (http://www.cplusplus.com/reference/cstring/strcpy/).

strcat will concatenate the source to the destination (http://www.cplusplus.com/reference/cstring/strcat/).

But I believe that DavidOConner is right... I can't believe I missed that.

Regards,

Brad KF7FER

EDIT: Had strcat backwards

Deleu: Being that true, I should then declare an array with a constant size. Since we have memory limitation, I am the one responsible for declaring it as small as possible?

Always. In this case I believe that you can use VW_MAX_MESSAGE_LEN as the largest size you'll need (but you might want to do +1 to include the string terminator).

Regards,

Brad KF7FER

EDIT: Looking at the examples at http://www.pjrc.com/teensy/td_libs_VirtualWire.html, it appears that simply using VW_MAX_MESSAGE_LEN (or 30) is good enough.

strcat() works with strings and char arrays are not necessarily strings. Some compilers do the equivalent of a memset() with zero that would make the array appear to work on the first pass. However, strcat() assumes there is a null-terminated sequence of data to which it adds the new data. Since the first time strcat() is called, all zeros looks like all nulls and things work out great. On the second iteration of the loop, that may not be the case. I'm not sure because I don't know how the compiler allocates memory for local arrays on each pass through a loop. I'm just saying it might be a problem. Brad's suggestion of using strcpy() on the first call is a good one.

DavidOConnor:

int msgSize = (strlen(data) + 10);

char packetData[msgSize];

You can't do this in C/C++. The array size must be known at compile time. I am surprised you did not get a compile error. See http://www.cplusplus.com/doc/tutorial/arrays/

Not quite. GCC, which Arduino uses may be old, however it supports features that are in the C++14 specification. The below code is valid.

void Func( int msgSize ){
  char packetData[msgSize];
}
//...
void setup(){
  Func(strlen(data) + 10);
}

msgSize is not a compile time constant.