i'm building a little system with my arduino nano and a SIM7670 module.
While i could solve all my hardware problems so far, i'm now having some trouble with the communication to the module.
I'm sending some AT checks in the void setup and they work perfectly fine when sent standalone or up to 3 commands. But as soon as i add a 4th command, the responses start to produce gibberish.
I guess it's some kind of overflow problem, but i don't understand it. I reserved 150 bytes for the array which is more than enough, and i'm also immediately reading out every response after i sent the AT command.
The bad response looks like this (when activating the 4th AT command line):
It's interesting that the gibberish is in the first lines.
The good response looks like this:
I fooled around with different char arr lenghts and delays in between, but nothing seems to help.
Does anyone know how to fix this issue?
Thank you!
The code:
#include <SoftwareSerial.h>
SoftwareSerial SIM7670Serial(2, 3); // RX, TX
char response[150];
void setup() {
Serial.begin(9600);
SIM7670Serial.begin(9600);
sendATCommand("AT+CGMM"); // check communication
sendATCommand("AT+CPIN?");
sendATCommand("AT+CMGF=1");
// sendATCommand("AT+CNMI=2,2,0,0,0"); //Adding this line leads to the overflow! Works fine in standalone and together with 2 other AT commands...
}
void loop() {}
void readResponse(){
char c;
char endMarker = (char)13;
byte j = 0;
if(SIM7670Serial.available()){
while(SIM7670Serial.available()){
c = SIM7670Serial.read(); //gets one byte from serial buffer
if(c != endMarker){
response[j] = c;
j++;
}
delay(2); //slow looping to allow buffer to fill with next character
}
response[strlen(response)]= (char)0;
Serial.println(response);
}
}
void sendATCommand(const char* cmd) {
SIM7670Serial.println(cmd);
readResponse();
}
Second guessing the timing of an asynchronous protocol is also a bad practice. at 9600 bauds you get roughly 1 byte per ms so during that pause 2 characters might have piled-up. You'll loop and get one but one is still pending and then you wait again and 2 more chars arrive. You'll loop and get one but now you have 2 bytes still pending... If the answer is longer than the Serial Rx buffer, after a short while (~130ms), you'll loose data.
I have adapted the code to have the correct termination line.
Also, i added the check from the Serial Input tutorial if the lengths exceeds the maximum char arr length.
void readResponse(){
char c;
char endMarker = (char)13;
byte j = 0;
if(SIM7670Serial.available()){
while(SIM7670Serial.available()){
c = SIM7670Serial.read(); //gets one byte from serial buffer
if(c != endMarker){
response[j] = c;
j++;
if (j >= numChars){
j = numChars - 1;
}
response[j] = '\0';
}
// if(c == endMarker){
// Serial.println("Endmarker found!");
// }
delay(2); //slow looping to allow buffer to fill with next character
}
Serial.println(response);
}
}
Good point.
I wanted to add a check first for the endmarker to make sure it really exists like expected, so i could rewrite the code to look for an endmarker rather than depending on the 2ms buffer.
But when i add the check the Serial Communication is going absolutely bonkers.
I've made a short video of the behaviour:
EDIT: I've just found that apparently the response does contain a carriage return in the beginning as well.
How can i handle this when the Endmarker is also at the start?
so you have a start marker (CR/LF) and an end marker (CR/LF again)
you could ignore all the CR and only deal with LF as start and end marker to make the reading simple.
may be something like this would work (typed here)
const byte maxResponseLength = 50;
char response[maxResponseLength + 1]; // +1 for the trailing null
enum ResponseStatus : byte {WAITING, RECEIVING, RECEIVED, TIMEOUT};
ResponseStatus responseAvailable(const unsigned long timeout = 1000ul) {
static unsigned long chrono;
static byte responseIndex = 0;
static ResponseStatus status = WAITING;
if (status == RECEIVED || status == TIMEOUT) { // assume user has handled the previous response, reset
status = WAITING;
}
if (SIM7670Serial.available()) {
char received = SIM7670Serial.read();
chrono = millis(); // there is activity on the Serial line, reset the timeout period
if (received != '\r') { // we just ignore CR
switch (status) {
case WAITING: // we are still waiting for the start marker
if (received == '\n') {
responseIndex = 0;
response[0] = '\0';
status = RECEIVING;
}
break;
case RECEIVING: // accumulate while there is space in the buffer and until the marker is received
if (received == '\n') {
// this is the end marker, our message is complete
response[responseIndex] = '\0';
status = RECEIVED;
} else {
// store if there is room left otherwise the data is just lost (or you could decide to report an OVERFLOW status)
if (responseIndex < maxResponseLength) response[responseIndex++] = received;
}
break;
default: break;
}
}
} else {
if (millis() - chrono >= timeout) status = TIMEOUT;
}
return status;
}
the function would report its status and you would call it continuously until you get RECEIVED or TIMEOUT.
I did some more debugging and found out that the SIM module just needed some time to give the answer, before being send another command. In my code i asked for all 4 commands in a very short time and the module was unable to keep up.
After adding a 25ms delay between the command and readResponse, the responses worked fine!
The state maschine solution looks awesome, but the code will need some additional adaptions for my application. Because the AT command i'm sending also has the endmarker, so the code would think to start from there and then stop at the beginning of the response.
Also, because of the time problem mentioned above, i need a way to make sure that the controller waits for the whole answer before moving on to the next command.
Ideally you would send a command and await for the answer to come and check if it’s fine before sending the next command. It’s just a while loop calling the function above until you get either RECEIVED or TIMEOUT as an answer, this way you make the function blocking
What you send does not go through that code unless you have the echo on. This code only listens to the answer from the module.
Thanks for the tip, the echo was indeed on. I turned it off now.
Today i was studying how enums work and i was trying to integrate your proposed solution/code with small adaptions. Since i'm doing some AT command checks in the void setup, i need the code to run in a loop until the message is completely received before moving on, otherwise the code would move to the next command without waiting for the response.
So my idea was to trap it on a while loop and basically use the code you were providing. Right now i'm still a little unsure on how to call for the function, so far i can't compile the code (the arguments in the readResponse function are the issue, the rest seems to be fine. How can i fix this and would my generel idea work?
The Code:
//defined globall at beginning:
enum ResponseStatus : byte {WAITING, RECEIVING, RECEIVED, TIMEOUT};
void readResponse(){
responseAvailable(1000); //this doesn't work
if (status == RECEIVED){ //this doesn't work
Serial.println(response);
}
}
ResponseStatus responseAvailable(const unsigned long timeout = 1000){
static unsigned long chrono;
static byte responseIndex = 0;
static ResponseStatus status = WAITING;
if (status == RECEIVED || status == TIMEOUT){
status = WAITING;
}
if(SIM7670Serial.available()){
while(status == WAITING || status == RECEIVING){
char c = SIM7670Serial.read(); //gets one byte from serial buffer
chrono = millis();
if(c != '\r'){
switch (status){
case WAITING:
if (c == '\n'){
responseIndex = 0;
response[0] = '\0';
status = RECEIVING;
}
break;
case RECEIVING:
if (c == '\n'){
response[responseIndex] = '\0';
status = RECEIVED;
} else {
response[responseIndex] = c;
responseIndex++;
}
break;
default: break;
}
}
}
}
else{
if (millis() - chrono >= timeout){
status = TIMEOUT;
}
}
return status;
}
Here you check if a character is available, and then lock yourself reading more characters than there might be.
if(SIM7670Serial.available()){
while(status == WAITING || status == RECEIVING){
char c = SIM7670Serial.read(); //gets one byte from serial buffer
The FSM is in a function meant to be called repeatedly, many times finding no character available. Only when it returns RECEIVED are you to check the reception.
Call it frequently in your loop() function. You may have literally nothing else to do until, but at least you won't be grabbing non-existent characters and handling them uncritically
if(SIM7670Serial.available()){
while(status == WAITING || status == RECEIVING){
char c = SIM7670Serial.read(); //gets one byte from serial buffer Not if there isn't one...
Read @J-M-L's code carefully, put your finger on it and see that a character is never read until there is a character to read.
This is covered in the tutorial linked above, serial input basics. It too is worth a close look.
So after some more hours of trial & error, i think i tinkered a solution that looks like it's working.
The main challenge was the vastly different application of the AT command and readout functions. In the void setup(), i don't care about time and only here i actively need to wait for the finished answer(s) before allowing it to move to the next line of commands.
In the later application, i can just put the readout function in the void loop() to call it repeatedly.
It might not be the most optimized solution, but i decided to split them into separat functions for setup() and loop().
Also, i found out that you can alter the way the GMS module responses. So now i've set it up to respond with (response)(CR)(LF) instead of (CR)(LF)(response)(CR)(LF). And i turned echo back on (for easier debugging).
The reason for the for loop() of 4 iterations is because one command (including echo) will produce exactly 4x(CR), like this:
So we basically have 4 "finished" responses, hence the counter 4.
For the void loop() i'm simply reading it out until i find a CR, without second guessing any timings anymore.
If you have any quick improvement suggestion, i'd gladly hear them. But i don't want to change major things now, since this seems to work and i'd like to move on.
At this point i would like to thank you for the much needed support!
I never imagined that reading out a simple Serial could cause such problems.
It was frustrating at times but i learned quite a bunch here.
Thank you so much, especially @J-M-L !
Here are the relevant parts of the code:
void readResponseStart(){
static byte responseIndex = 0;
static ResponseStatus status = WAITING;
if (status == RECEIVED){
status = WAITING;
}
if(SIM7670Serial.available()){
while(status == WAITING){
char c = SIM7670Serial.read(); //gets one byte from serial buffer
if(c != '\n' && '\r'){
response[responseIndex] = c;
responseIndex++;
}
if (c == '\r'){
response[responseIndex] = '\0';
status = RECEIVED;
responseIndex = 0;
}
}
}
if(status == RECEIVED){
Serial.println(response);
}
}
void sendATCommandStart(const char* cmd) {
SIM7670Serial.println(cmd);
delay(250);
for(int j=1;j<=4;j++){
readResponseStart();
}
}
void readResponse(){
static byte responseIndex = 0;
static ResponseStatus status = WAITING;
if (status == RECEIVED{
status = WAITING;
}
if(SIM7670Serial.available()){
char c = SIM7670Serial.read(); //gets one byte from serial buffer
if(c != '\n' && '\r'){
response[responseIndex] = c;
responseIndex++;
}
if (c == '\r'){
response[responseIndex] = '\0';
status = RECEIVED;
responseIndex = 0;
}
}
if(status == RECEIVED){
Serial.println(response);
}
}
void sendATCommand(const char* cmd) {
SIM7670Serial.println(cmd);
}
Sorry, but i went on with my project and got communication overflows again. Since this still matches this threads topic, i thought it's easiest to continue here.
While most commands work now, unfortunately i'm still getting communication problems on longer commands & responses.
I've printed out every char while reading the response to see what's going on, and in this example it starts producing gibberish at char number 53.
I don't know what's the issue here, i've reserved 100 bytes for the response char array. I'm not second guessing the Serial timing anymore, so i don't think the Serial Buffer is overflowing.
What else could be the issue?
Bad response looks like this (when printed each char individually):
Granted, i picked a bad example, the ReadResponseStart is different with the while loop().
In another example, when using the "normal" sendATCommand() and read it out in my void loop(), this particular command seems to fail for whatever reason:
BECAUSE you are over-running your response[] buffer. You do not check for over-flow, and you are stuffing loads of invalid characters into the buffer becaue you do not check SIM7670Serial.available() FOR EACH CHARACTER before calling SIM7670Serial.read(). You are clobbering whatever data is in memory after than array.
NEVER call read() without making sure a valid character is waiting. You instead check that at least one character is waiting, then you attempt to immediately read the entire message. So, you are reading lots of garbage, with the occassional valid character randomly mixed in.
Unfortunately, the echo i get from the SIM module is garbage, like this:
But i'm still getting an "OK" back from the module.
So i'm guessing that the commandString is transfered just fine, but it seems to have trouble sending a long echo back on the Serial.
Because i can read out other, even longer data strings without problems, i think my code is not the problem.
I think i will just go on and see what happens next..
You still have no protection from a buffer over-run. Each time you receive a character, you store it, and blindlly increment responseIndex, without ever checking if the buffer is already full. If you ever manage to miss the '\r', you will likely start stomping on memory.
