0

This problem is on an Arduino Due. I am trying to populate a char array buffer that is receiving information over a serial port. I am reading the information off of the serial port and storing it into a buffer array and passing it back to the original function call.

First is the code that initializes the buffer and writes the data that the Serial requires in order to issue a response. This function compares many types of expected responses (uint32_t expect) and returns a confirmation as an int.

//Receive a cmd to send over serial and expect a certain response
int SendCommand(char * cmd, uint32_t expect){
  int cmdSuccess = 0, j=0;
  char buffer[500];
  memset(&buffer,0,sizeof(buffer)); //0 out the buffer
  Serial3.print(F(cmd));
  delay(500);

  Response(buffer);

  if (expect == EXPECT_OK){
    for(int i=0;i<sizeof(RESP_OK);i++){
      if(buffer[i]==RESP_OK[i]){ //RESP_OK is an example of a response of length 6
        j++;
    }
  }
    if(j==sizeof(expect)-1){Serial.print(F("\nGOOD RESPONSE!\n")); return 1;}
  }
  else{return 0;}
  delete [] buffer;
}

void Response(char buffer[500])
{
  int index = 0; //message index;
  int count = 0; //used for timeouts
  while(1)
  {
  if(Serial3.available()) //Data will be coming across serial3
  {
    char data = Serial3.read();
    buffer[index]=data;
    index++;
  }
    count++;
    delay(100);
    if(count == 50){
    Serial.println(F("\nEnd of Response"));
    break;
  }

}

Edit: fixed the braces.

Right now, this code is hanging at the last part of Response right before break;. Am I having memory issues by the time it reaches the println at the end? That last println was actually working until I began passing the buffer as a pointer and not returning it a char* and declaring void Response as char* Response. But the problem with this approach was that the buffer would reduce to sizeof(buffer) = 4 and would strip off the rest of the information when returned.

Edit: As Matt suggested down below, I declared a static const size_t bufferSize and pass it into the Response() call as well as passing in buffer as char*. Then I reset the buffer size by buffer = (char*)malloc(bufferSize).

What I still don't understand curious is the sizeof(buffer) after Response which is returning 4. However, 6 characters are stored into the array during Response. I've changed the pointer call to Response(char* buffer, const int size) with the following:

  if(index<size){
  buffer[index]=data;
  index++;
  }
  else{Serial.print(F("Buffer length exceeded!"));}

Thanks so much for your time!

  • What is that delete command for? 1) delete is c not c++ and should only be used to free something allocated with new and 2) that line is unreachable. Secondly you never check whether you're going past the end of your buffer (not an issue right now because of the incorrect } meaning you only ever get 1 byte) What if 501 bytes arrive before you time out? – Andrew Sep 8 '16 at 12:07
  • 1
    Also when you pass a char* sizeof will always return 4 (or 8 on 64 bit systems) rather than the size of the buffer, you're asking it for the size of the pointer not the size of the buffer it's pointing to. The normal way to cope with this is to pass a pointer and also pass an integer value of the buffer size. That makes the buffer size known to the function without having to hard code it as part of the function parameters. – Andrew Sep 8 '16 at 12:10
  • The 500 in the function's argument is meaningless and ignored by the compiler. If your timeout is supposed to be an inter-character timeout, then you're doing it wrong. You should clear 'count' in the IF block, if that is your intention. – TisteAndii Sep 8 '16 at 14:27
  • I tried putting it in as @Andrew mentioned, by passing the buffer as a pointer separate to its buffer size. However, it's still only returning with a sizeof = 4, leaving off some of the chars that are being read into it. – Andrew Sep 8 '16 at 17:21
  • Of course it's still returning a sizeof of 4, I said it will always do that. That's why you pass the size as a second parameter. Use that within the function and don't use sizeof. – Andrew Sep 9 '16 at 8:02
1

Sorry, but there are quite a few issues with your code any of which may be contributing to your problem.

  1. memset(&buffer,0,sizeof(buffer)); //0 out the buffer you are taking the address of a pointer and passing it to memset(). buffer is already a pointer of type char*. Its working, because address of the pointer is the same address as buffer[0], but it is wrong and won't always work.
  2. Serial3.print(F(cmd)); I have never used the F macro, but my understanding is it tells the compiler to store the string in flash memory. cmd is a variable on the stack, a parameter to the function, so my guess would be the compiler is ignoring the F macro.
  3. 500 using a literal value in more than one place is opening you up to problems. I think you should declare static const size_t BufferSize = 500;
  4. void Response(char buffer[500]) It would be better to pass in a pointer to a char and the size as the parameter, (const char* pBuffer, const int& size) you can treat it the same within your function or you could pass a pointer to the next available item and then return the updated pointer.
  5. 1for(int i=0;i strlen(buffer))` then buffer can't contain RESP_OK (its quicker too).

Also the Response function won't compile, because you have too many closing braces (I assume its a copy and paste error). This is the cause of you code hanging if(Serial3.available()) is inside a while(1) and there is no way of breaking out. So if you just fix that you code will run.

  • About point 1: buffer is not a pointer, it's an array. The & does not make the array identifier decay to a pointer, thus &buffer is not the address of a pointer, it's the address of the array. – Edgar Bonet Sep 8 '16 at 15:34
  • Yes, these are all good points. First, you are right about the copy-paste error and the hanging bracket, otherwise I'm sure the code wouldn't compile. So it no longer freezes, and I modifies the buffer call to Response by passing it in as 'char*' (not a pointer) and 'size'. However, I'm still getting the problem when I call 'sizeof(buffer)' right after the call to response. It is reading in 6 characters into the array during 'Response', and when it resumes 'SendCommand' buffer has only a length of 4! – Andrew Sep 8 '16 at 16:35
  • @EdgarBonet In c the name of an array without an index specifier is a pointer to the start of the array not the entire array. That's why sizeof(array) doesn't work within a function if the array is passed as a parameter, all that gets passed is a pointer. – Andrew Sep 9 '16 at 8:04
  • @Andrew sizeof(buffer) will NEVER tell you how many bytes of data are in the buffer. Depending upon the scope it will either tell you the number of bytes allocated to buffer when it was created (500 in this case) or it will tell you the size of a pointer. If you want to know how many bytes you've written to that buffer then you need to track the maximum index you have used to write data. – Andrew Sep 9 '16 at 8:09
  • @Andrew (user 24459, not the OP) wrote: “In c the name of an array without an index specifier is a pointer to the start of the array”. This is incorrect. An array identifier represents the whole array. However, when you use the identifier in an expression, it does decay to a pointer to the first element, save for three exceptions. Taking the address of the array is one of these exceptions: there is no decay and you get the address of the array. C.f. the C FAQ about the equivalence of pointers and arrays. – Edgar Bonet Sep 9 '16 at 8:27
0

I am having difficulties to follow your code due to indentation and additional braces which don't pair up correctly, but to me you are breaking out of the if(count == 50) statement through to the whole function as the while(1) statement was already closed.

0

Ok, thanks to the responses in this thread, I believe I have the solved the problem. There is a problem using sizeof(char*) when passing through arrays, so I used strlen(char*) instead. This code properly stores the messages of various sizes coming off of Serial3 and correctly compares them to the expected responses. The F() macro in Arduino is very useful in keeping the buffer free and, in general, I use it to print constants during debugging.

int SendCommand(char * cmd, bool resp_type, uint32_t expect){
delay(500);
  int cmdSuccess = 0, j=0, msgSize = 0;
  char * buffer;
  Serial3.print(F(cmd));
  Serial.print(F(cmd)); //Print to serial for debug purposes only
  delay(500);
  buffer = (char *)malloc(bufferSize);
  msgSize = Response(buffer, bufferSize);

  Serial.println("Buffer is: ");
  if (expect == EXPECT_OK){
    for(int i=0;i<msgSize;i++){
      if(buffer[i]==RESP_OK[i]){
        Serial.print(buffer[i],HEX);
        j++;
      }
    }
    Serial.print("\n");
    if(j==sizeof(expect)-1){Serial.print(F("\nGOOD RESPONSE!\n")); return 1;}
  }  

  return cmdSuccess;
}

int Response(char* buffer, const int size)
{
  int index = 0; //message index;
  int count = 0; //used for timeouts
  while(1)
  {
  if(Serial3.available()) //Message will be coming across serial3
  {
    char data = Serial3.read();
      if(index<size){
        buffer[index]=data;
        index++;
      }
      else{Serial.print(F("Buffer length exceeded!"));}
  }
    count++;
    if(count == 50){
    Serial.println(F("\nEnd of Response"));
    break;
    }
  } return index;
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.