Go Down

Topic: Serial Communication / Sending Strings or Char Array (Read 587 times) previous topic - next topic

solveetcoagula07

I have been working on this for a few days and have been unable to figure out what is wrong.  I need to be able to send commands larger than 1 character and have attempted using a 2d char array and a string array as in the code posted below. 

When I run the code with the outer for loop in the gatherData() function set to i < 1, it will run 19 times returning clean data but if I set i < 2, it will run the first command 19 times and the second command 1 time, with random garbage in the returned data. 

I have modified the code slightly (changed the actual commands).

Does anyone have any advice or see what I am doing wrong here?  Thank you for your assistance.

Code: [Select]
#include <SoftwareSerial.h>
SoftwareSerial rs232(10, 11);

int rcvCount = 0;

const byte numChars = 64;

char endStr[numChars];
char receivedChars[numChars];

  String cmds[3] = {"1234", "5678"};



bool RTS = true;
bool newData = false;
bool executed = false;

void setup() {
    Serial.begin(9600);
    rs232.begin(9600);
}

void loop() {
  recvWithEndMarker();
  showNewData();
 
  if(executed == false){
    gatherData();
  }
}

void recvWithEndMarker() {
    static byte ndx = 0;
    char endMarker = '\r';
    char rc, t;

    while (Serial.available() > 0) {
        t = Serial.read();

        if (t == 'A') {
          Serial.println("Caught A");
          executed = false;
          RTS = true;
          rcvCount = 0;
        }
        else {

        }
    }
   
    while (rs232.available() > 0 && newData == false) {
        rc = rs232.read();
        if (rc != endMarker) {
            receivedChars[ndx] = rc;
            ndx++;
            if (ndx >= numChars) {
                ndx = numChars - 1;
            }
        }
        else {
            receivedChars[ndx] = '\0'; // terminate the string
            ndx = 0;
            rcvCount++;
            RTS = true;
            newData = true;
        }
    }
}

void showNewData() {
    if (newData == true) {   
        Serial.println(receivedChars);
        newData = false;
        receivedChars[0] = (char)0;  //clear the char array
      }
}


void gatherData(){
  if(RTS == true){
    int i;
    int j;
    for(i = 0; i < 1; i++){
      char charBuf[cmds[i].length()];
      cmds[i].toCharArray(charBuf, cmds[i].length() + 1);


     
      for(j = 0; j < cmds[i].length(); j++){
          //Serial.print(charBuf[j]);
          rs232.write(charBuf[j]);
      }
        rs232.write('\r');
    }
    RTS = false;
  }
  if(sizeof(cmds) == rcvCount){
    executed = true;
  }
}

MorganS

You have two serial inputs. Split the "receive" function into two functions, each one should only deal with one input. You might call it "checkSerial()" and "checkRS232()" or whatever you like. Details such as the end marker may be different in those two functions.



Quote
Code: [Select]
  String cmds[3] = {"1234", "5678"};
You only populated two items of your three-item array.
"The problem is in the code you didn't post."

Robin2

Have a look at the examples in Serial Input Basics. Just make the sender match what is expected by the receiver. The system in the 3rd example will be the most reliable.

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

solveetcoagula07

Ah, I missed changing the 3 to 2 when I copied the code to the forum.  I looked at Serial Input Basics and used the 3rd example and have it working much better.  Now using the following (modified again) code, when outputting to the serial monitor, there are carriage returns after each commands response.

Would this be due to the '\0' termination byte?  What could I do to get everything on one line, or preferably into an array so that I can format a string to output?
Code: [Select]
#include <SoftwareSerial.h>
SoftwareSerial rs232(10, 11);
const byte numChars = 64;
char receivedChars[numChars];

char endStr[numChars];

boolean newData = false;

int rcvCount = 0;
String cmds[6] = {"****", "****", "****", "****", "****", "****"};
String result = "";

long previousMillis = 0;
long interval = 100;

void setup() {
  Serial.begin(9600);
  rs232.begin(9600);
  Serial.println("<Arduino is ready>");
  int x = 0;
  while (x < 6) {
    unsigned long currentMillis = millis();

    if (currentMillis - previousMillis > interval) {
      previousMillis = currentMillis;

      writeString(cmds[x]);
      rs232.write('\r');
      x++;
    }
  }
}

void loop() {
  recvWithStartEndMarkers();
  showNewData();
  if (rcvCount == 6) {
    for (int y = 0; y < (sizeof(endStr) / sizeof(endStr[0])); y++) {
      if (endStr[y] != '\0') {
        result.concat(endStr[y]);
      }
    }
    Serial.print(result);
    rcvCount = 0;
  }
}

void writeString(String stringData) { // Used to serially push out a String with Serial.write()

  for (int i = 0; i < stringData.length(); i++) {
    rs232.write(stringData[i]);   // Push each char 1 by 1 on each loop pass
  }

}// end writeString

void recvWithStartEndMarkers() {
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char endMarker = '\r';
  char rc;

  while (rs232.available() > 0 && newData == false) {
    rc = rs232.read();

    if (rc != endMarker) {
      receivedChars[ndx] = rc;
      ndx++;
      if (ndx >= numChars) {
        ndx = numChars - 1;
      }
    }
    else {
      receivedChars[ndx] = '\0'; // terminate the string
      recvInProgress = false;
      ndx = 0;
      newData = true;
      rcvCount++;
    }
  }
}

void showNewData() {
  if (newData == true) {
    strcat(endStr, receivedChars);
    if ((sizeof(cmds) / sizeof(cmds[0])) != rcvCount) {
      strcat(endStr, ",");
    } else {
      //endStr[0] = (char)0;  //clear the char array
      //receivedChars[0] = (char)0;  //clear the char array
    }
    newData = false;
  }
}

Robin2

Can you explain what you are trying to achieve (especially with endStr[]) - some examples of the input and the desired output would help. Also some examples of the actual output.

Doesn't this
Code: [Select]
strcat(endStr, receivedChars);
screw up your memory by writing past the end of receivedChars[]

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

solveetcoagula07

I am concerned about the memory issues since this device will be on and gathering data on a regular basis.  I was thinking that making the variables volatile may help with that situation?  As for the commands and response, let's say I want to request six different pieces of data...(I'm using SIM900 AT Commands as an example):
Code: [Select]
cmds[6] = ("AT+CPIN?", "AT+CSQ", "AT+COPS?", "AT+CGMI", "AT+CGMM", "AT+CGSN");

The current response in the serial monitor is:
Code: [Select]
+CPIN:READY,
+CSQ:30,0,
+COPS:0,0,"CHINAMOBILE"
SIMCOM_Ltd
SIMCOM_SIM900A
869988012018905


The desired output would be an array:
Code: [Select]
results[6] = ("+CPIN:READY", "+CSQ:30,0", "+COPS:0,0,'CHINAMOBILE'", "SIMCOM_Ltd" , "SIMCOM_SIM900A", "869988012018905");

This would allow me to format something like a url so that I could send the data to a server.

Robin2

I'm not sure I understand.

Are you saying that you want to send each of the commands in the array cmds[] one at a time and receive the 6 responses and then do something with those 6 responses?

Can you explain the piece you have posted called "response in the serial monitor".
Which command caused that to happen?
Which part of that do you need to use?

I think it would get too complicated to try to say more without those answers.

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

sterretje

#7
May 19, 2017, 05:50 pm Last Edit: May 20, 2017, 05:01 am by sterretje
If your concerned about memory, do not use String (with capital S).

Code: [Select]

// create an array of pointers to text
char *cmds[] = {"AT+CPIN?", "AT+CSQ", "AT+COPS?", "AT+CGMI", "AT+CGMM", "AT+CGSN"};

If there is a maximum size in the replies (of e.g. 20 characters) and you want to store the 6 possible replies in an array
Code: [Select]

// two dimensional arrays (6*(20+1))
char result[6][21];

For c-style strings (nul terminated character arrays), you will need one byte extra space to store the terminating nul character.

// late edit:
// bugfix in cmds array; was (), now {}
If you understand an example, use it.
If you don't understand an example, don't use it.

Electronics engineer by trade, software engineer by profession. Trying to get back into electronics after 15 years absence.

solveetcoagula07

Yes, I want to send the six commands (cmds[]) one at a time, receive the responses and be able to call them individually from an array.  To explain the "response in the serial monitor", those six lines are the respective responses to the cmds[].  So when I send "AT+CPIN?" it returns "+CPIN:READY".  The device that I am communicating with does send a carriage return, but I would imagine that it is dropped since the endMarker variable never gets appended in the else statement.  So I'm not really sure what is causing the linefeed in the serial monitor. 

I am only using String to see what options I have to get it to work the way I want, but even without that would the memory fill up and cause the arduino to stop working until it is reset?

MorganS

Capital-S Strings can fill up the Arduino's memory, if you do certain things with them.

Character arrays or small-s strings can do this too, but you have to work a bit harder and use functions like malloc() to do that. The way that they are normally used, there is no danger of filling up memory.

Small-s strings are an important technique to learn but I don't believe that it's necessary in this case. Just make it work with what you have.
"The problem is in the code you didn't post."

Robin2

Yes, I want to send the six commands (cmds[]) one at a time, receive the responses and be able to call them individually from an array.  To explain the "response in the serial monitor", those six lines are the respective responses to the cmds[].
OK. That's clear.

But you have not said what you want to do with all those responses? Or which one is really of interest to you.

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

solveetcoagula07

All six of the responses are of interest.  I plan on using them to form a URL: http://somesite.com/index.php?one=+CPIN:READY&two=+CSQ:30,0&three=+COPS:0,0,"CHINAMOBILE"&four=SIMCOM_Ltd&five=SIMCOM_SIM900A&six=869988012018905


The webpage would then be able to grab the values from the URL and insert them into a database. 

sterretje

This can be the basis.
Code: [Select]
#include <SoftwareSerial.h>
SoftwareSerial rs232(10, 11);

const byte numChars = 64;
char receivedChars[numChars];

// create an array of pointers to commands
char *cmds[] = {"AT+CPIN?", "AT+CSQ", "AT+COPS?", "AT+CGMI", "AT+CGMM", "AT+CGSN"};

// two dimensional array to hold replies
char replies[sizeof(cmds) / sizeof(cmds[0])][numChars];

bool newData = false;

void setup()
{
  Serial.begin(250000);
  rs232.begin(9600);
  Serial.println("<Arduino is ready>");
}

void loop()
{
  // index of command to send
  static byte cmdIndex = 0;
  // flag indicating thhat we;re waiting for a reply
  bool fWaiting4Reply = false;

  // if not waiting for reply
  if (fWaiting4Reply == false)
  {
    // if not all commands done
    if (cmdIndex < sizeof(cmds) / sizeof(cmds[0]))
    {
      // send command followed by '\r'
      rs232.print(cmds[cmdIndex]); rs232.print("\r");
      fWaiting4Reply = true;
    }
  }

  // if waiting for reply
  if (fWaiting4Reply == true)
  {
    getReply();
    if (newData == true)
    {
      processReply(cmdIndex);
      fWaiting4Reply = false;
      cmdIndex++;
    }
  }

  // if all commands done
  if (cmdIndex == sizeof(cmds) / sizeof(cmds[0]))
  {
    // debug print
    for (int replyCnt = 0; replyCnt < sizeof(cmds) / sizeof(cmds[0]); replyCnt++)
    {
      Serial.print("Reply "); Serial.print(replyCnt);
      Serial.print(" ["); Serial.print(replies[replyCnt]); Serial.println("]");
    }

    // wait forever
    for (;;);
  }
}

/*
  get reply from rs232
  NOTE: was recvWithStartEndMarkers
*/
void getReply()
{
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char endMarker = '\r';
  char rc;

  while (rs232.available() > 0 && newData == false)
  {
    rc = rs232.read();

    if (rc != endMarker)
    {
      receivedChars[ndx] = rc;
      ndx++;
      if (ndx >= numChars)
      {
        ndx = numChars - 1;
      }
    }
    else
    {
      receivedChars[ndx] = '\0'; // terminate the string
      recvInProgress = false;
      ndx = 0;
      newData = true;
    }
  }
}

/*
  process reply
  NOTE: was showNewData
*/
void processReply(byte cmdNum)
{
  // copy data to reply buffer
  strcpy(replies[cmdNum], receivedChars);
  // indicate we're ready for next reply
  newData = false;
}

The code demonstrates how to send commands in sequence, wait for a reply on each command, process the reply (add to replies array) and at the end displays all replies.

Tested using pure HardwareSerial; afterwards modified for SoftwareSerial.
If you understand an example, use it.
If you don't understand an example, don't use it.

Electronics engineer by trade, software engineer by profession. Trying to get back into electronics after 15 years absence.

solveetcoagula07

sterretje thank you for the code.  I tried it and I am having the same issue with your code and the code below.  When going from a reset state the first commands response is missed, on the second data call, the last commands response is missing but on the third and greater data calls, all of the data is there except that the last commands response gets pushed in front of all others...see code and example output below.
Code: [Select]
#include <SoftwareSerial.h>
SoftwareSerial rs232(12, 14);
const byte numChars = 64;
char receivedChars[numChars];

char endStr[numChars];

boolean newData = false;

int rcvCount = 0;
char *cmds[] = {"AT+CPIN?", "AT+CSQ", "AT+COPS?", "AT+CGMI", "AT+CGMM", "AT+CGSN"};

long previousMillis = 0;
long interval = 100;

void setup() {
  Serial.begin(9600);
  rs232.begin(9600);
}

void loop() {
  recvWithStartEndMarkers();
  showNewData();
}

void gatherData() {
  int x = 0;
  while (x < (sizeof(cmds) / sizeof(cmds[0]))) {

    unsigned long currentMillis = millis();

    if (currentMillis - previousMillis > interval) {
      previousMillis = currentMillis;
      Serial.write(cmds[x]);
      Serial.write('\r');
      x++;
    }
  }
}

void recvWithStartEndMarkers() {
  char t;
  while (rs232.available() > 0) {
    t = rs232.read();

    if (t == 'A') {
      gatherData();
      rcvCount = 0;
    }
  }

  static boolean recvInProgress = false;
  static byte ndx = 0;
  char endMarker = '\r';
  char rc;

  while (Serial.available() > 0 && newData == false) {
    rc = Serial.read();

    if (rc != endMarker) {
      receivedChars[ndx] = rc;
      ndx++;
      if (ndx >= numChars) {
        ndx = numChars - 1;
      }
    }
    else {
      receivedChars[ndx] = '\0'; // terminate the string
      recvInProgress = false;
      ndx = 0;
      newData = true;
      rcvCount++;
    }
  }
}

void showNewData() {
  if (newData == true) {
    // copy data to reply buffer
    strcat(endStr, receivedChars);
    receivedChars[0] = (char)0;  //clear the char array

    if (((sizeof(cmds) / sizeof(cmds[0])) - 1) != rcvCount) {
      strcat(endStr, ",");
    } else {
      rs232.print("http://127.0.0.1:8080/get.php?somevar=");
      rs232.println(endStr);
      rcvCount = 0;
      endStr[0] = (char)0;  //clear the char array
    }
    newData = false;
  }
}


I have changed the responses to A,B,C,D,E,F,G to simplify things.


Responses (code ran multiple times to show error):
Code: [Select]

The commands and their respective responses:
AT+CPIN? = A, B
AT+CSQ = C
AT+COPS? = D
AT+CGMI = E
AT+CGMM = F
AT+CGSN = G

What I am getting:

http://127.0.0.1:8080/get.php?somevar=C,D,E,F,G
http://127.0.0.1:8080/get.php?somevar=A,B,C,D,E,F
http://127.0.0.1:8080/get.php?somevar=G,A,B,C,D,E,F
http://127.0.0.1:8080/get.php?somevar=G,A,B,C,D,E,F
http://127.0.0.1:8080/get.php?somevar=G,A,B,C,D,E,F


This has been tested on an Arduino MEGA, UNO and an ESP8266, all have the same result.

sterretje

OK, the code that I presented in reply #12 had one bug; the fWaiting4Reply should be static.

Below new code (some debugging added so I could follow which command was send)
Code: [Select]
#include <SoftwareSerial.h>
SoftwareSerial rs232(10, 11);

const byte numChars = 64;
char receivedChars[numChars];

// create an array of pointers to commands
char *cmds[] = {"AT+CPIN?", "AT+CSQ", "AT+COPS?", "AT+CGMI", "AT+CGMM", "AT+CGSN"};

// two dimensional array to hold replies
char replies[sizeof(cmds) / sizeof(cmds[0])][numChars];

bool newData = false;

void setup()
{
  Serial.begin(250000);
  rs232.begin(9600);
  Serial.println("<Arduino is ready>");
}

void loop()
{
  // index of command to send
  static byte cmdIndex = 0;
  // flag indicating thhat we;re waiting for a reply
  static bool fWaiting4Reply = false;

  // if not waiting for reply
  if (fWaiting4Reply == false)
  {
    // if not all commands done
    if (cmdIndex < sizeof(cmds) / sizeof(cmds[0]))
    {
      Serial.print("Sending: "); Serial.println(cmds[cmdIndex]);
      // send command followed by '\r'
      rs232.print(cmds[cmdIndex]); rs232.print("\r");
      fWaiting4Reply = true;
    }
  }

  // if waiting for reply
  if (fWaiting4Reply == true)
  {
    getReply();
    if (newData == true)
    {
      processReply(cmdIndex);
      fWaiting4Reply = false;
      cmdIndex++;
    }
  }

  // if all commands done
  if (cmdIndex == sizeof(cmds) / sizeof(cmds[0]))
  {
    // debug print
    for (int replyCnt = 0; replyCnt < sizeof(cmds) / sizeof(cmds[0]); replyCnt++)
    {
      Serial.print("Reply "); Serial.print(replyCnt);
      Serial.print(" ["); Serial.print(replies[replyCnt]); Serial.println("]");
    }

    // wait forever
    for (;;);
  }
}

/*
  get reply from rs232
  NOTE: was recvWithStartEndMarkers
*/
void getReply()
{
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char endMarker = '\r';
  char rc;

  while (rs232.available() > 0 && newData == false)
  {
    rc = rs232.read();

    if (rc != endMarker)
    {
      receivedChars[ndx] = rc;
      ndx++;
      if (ndx >= numChars)
      {
        ndx = numChars - 1;
      }
    }
    else
    {
      receivedChars[ndx] = '\0'; // terminate the string
      recvInProgress = false;
      ndx = 0;
      newData = true;
    }
  }
}

/*
  process reply
  NOTE: was showNewData
*/
void processReply(byte cmdNum)
{
  // copy data to reply buffer
  strcpy(replies[cmdNum], receivedChars);
  // indicate we're ready for next reply
  newData = false;
}



I tested it against another arduino (also using SoftwareSerial) that simply echoed the received commands; code below
Code: [Select]
#include <SoftwareSerial.h>
SoftwareSerial rs232(10, 11);

const byte numChars = 64;
char receivedChars[numChars];
bool newData = false;

void setup()
{

  Serial.begin(250000);
  rs232.begin(9600);
  Serial.println("<Slave is ready>");

}

void loop()
{
  static int cnt = 1;
  getReply();
  if(newData == true)
  {
    Serial.print("Received: "); Serial.println(receivedChars);
    rs232.print(receivedChars); rs232.print("\r");
    newData = false;
  }
}

void getReply()
{
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char endMarker = '\r';
  char rc;

  while (rs232.available() > 0 && newData == false)
  {
    rc = rs232.read();

    if (rc != endMarker)
    {
      receivedChars[ndx] = rc;
      ndx++;
      if (ndx >= numChars)
      {
        ndx = numChars - 1;
      }
    }
    else
    {
      receivedChars[ndx] = '\0'; // terminate the string
      recvInProgress = false;
      ndx = 0;
      newData = true;
    }
  }
}


The output on the sending arduino
Code: [Select]
<Arduino is ready>
Sending: AT+CPIN?
Sending: AT+CSQ
Sending: AT+COPS?
Sending: AT+CGMI
Sending: AT+CGMM
Sending: AT+CGSN
Reply 0 [AT+CPIN?]
Reply 1 [AT+CSQ]
Reply 2 [AT+COPS?]
Reply 3 [AT+CGMI]
Reply 4 [AT+CGMM]
Reply 5 [AT+CGSN]


I strongly suggest that you do not modify / butcher the original Serial Input Basics code as you did

I have no idea of the real replies that you get back so can't help further.
If you understand an example, use it.
If you don't understand an example, don't use it.

Electronics engineer by trade, software engineer by profession. Trying to get back into electronics after 15 years absence.

Go Up