Go Down

Topic: Serial Input Basics: example 5, question (Read 2223 times) previous topic - next topic

Deva_Rishi

Would it not be much more elegant to simply add a ',' as a terminating character here
Code: [Select]
if (recvInProgress == true) {
            if (rc != endMarker) {
                receivedChars[ndx] = rc;
                ndx++;
                if (ndx >= numChars-1) {
                    ndx = numChars - 2;             // and forcefully leave 1 extra character of space here
                }
            }
            else {
                receivedChars[ndx] = ',';          // put an extra separator
                receivedChars[ndx+1] = '\0';   // terminate the string
                recvInProgress = false;
                ndx = 0;
                newData = true;
            }
To 'Correct' you have to be Correct. (and not be condescending..)

brice3010

Can you make a short program that illustrates the problem? I suspect it would help you as well as us.

...R
PS ... I have not tested my examples on an ESP8266.
Sortened bare-bones version included.

To clarify, here is the code used by the transmitter in the TX code section:
Code: [Select]

    Serial.print('<'); // this section for HC-12 transmission
    Serial.print(cycleValue);  // ref Robin2's Serial Output Basics
    Serial.print(',');   // http://forum.arduino.cc/index.php?topic=396450.0
    //    Serial.print(temperatureT1); // send temperature read from analog input temperaturePin
    Serial.print(get3231Temp()); // read DS3231 temperature
    Serial.print(',');
    Serial.print(batteryVoltage); // send battery voltage
    Serial.print(',');
    Serial.print(now.minute(), DEC); // send minute
    Serial.print(',');
    Serial.print(now.hour(), DEC); // send hour
    Serial.print(",A>"); // send sensor identifier
    Serial.println();


The pending test (ref cattledog) with use of A. is not yet shown here.

brice3010

Would it not be much more elegant to simply add a ',' as a terminating character here
Code: [Select]
if (recvInProgress == true) {
            if (rc != endMarker) {
                receivedChars[ndx] = rc;
                ndx++;
                if (ndx >= numChars-1) {
                    ndx = numChars - 2;             // and forcefully leave 1 extra character of space here
                }
            }
            else {
                receivedChars[ndx] = ',';          // put an extra separator
                receivedChars[ndx+1] = '\0';   // terminate the string
                recvInProgress = false;
                ndx = 0;
                newData = true;
            }

The issue with that I think is that , are used as separator, and termination is done with endmarker: this allows the program to differentiate between "continuous" data and "last" data.
What advantage would the use of , as endmarker have?

Deva_Rishi

#18
Jan 15, 2019, 07:10 pm Last Edit: Jan 15, 2019, 07:12 pm by Deva_Rishi
Quote
What advantage would the use of , as endmarker have?
what was suggested earlier just so strtok() is looking for something that it can find.

Actually this might be a clue if you have done this correctly. Did you only replace the last use of strtok for the message and the printing of the 6. The earlier cases should have been left unchanged.  If that is the situation for this test, then there must be something wrong with the null termination.

Code: [Select]

  strtokIndx = strtok(NULL, NULL);// this continues where the previous call left off


Actually, I have higher hopes for the addition of another token after the A.  It could be a ',' or a '.' --just another character that strtok is looking for instead of the end of the string when looking for a non existent ','.
my point also, when a '>' endmarker is received do not just terminate with a NULL but add a ',' separator as well. I can't see anything being wrong with the terminating NULL other then strtok() not finding it !
To 'Correct' you have to be Correct. (and not be condescending..)

brice3010

what was suggested earlier just so strtok() is looking for something that it can find.
my point also, when a '>' endmarker is received do not just terminate with a NULL but add a ',' separator as well. I can't see anything being wrong with the terminating NULL other then strtok() not finding it !
Indeed, just to recap: now A is being sent without anything behind it, so placing a . or a , after the A and have strtok identify this character: strtokIndx = strtok(NULL, "."); in case of A.
Right?

cattledog

Quote
Indeed, just to recap: now A is being sent without anything behind it, so placing a . or a , after the A and have strtok identify this character: strtokIndx = strtok(NULL, "."); in case of A.
Right?
I think the suggestion was to add the final token character in the receiving code instead of in the transmission. In Robin's code, the end marker is replace by '\0' and it was suggested that a separator be added before the Null. You could probably send the final separator in the transmission and add another at the receive.

I think the bigger issue is why strtok with the ESP8266 appears to not find the null terminator when parsing something at the end of the character string.

brice3010

I think the suggestion was to add the final token character in the receiving code instead of in the transmission. In Robin's code, the end marker is replace by '\0' and it was suggested that a separator be added before the Null. You could probably send the final separator in the transmission and add another at the receive.

I think the bigger issue is why strtok with the ESP8266 appears to not find the null terminator when parsing something at the end of the character string.

Thanks for that clarification cattledog. Tomorrow afternoon tests will be done, parameters and results I post asap.

brice3010

#22
Jan 16, 2019, 05:34 pm Last Edit: Jan 16, 2019, 05:37 pm by brice3010
Actually this might be a clue if you have done this correctly. Did you only replace the last use of strtok for the message and the printing of the 6. The earlier cases should have been left unchanged.  If that is the situation for this test, then there must be something wrong with the null termination.

Code: [Select]

  strtokIndx = strtok(NULL, NULL);// this continues where the previous call left off
 //strtokIndx = strtok(NULL, ","); // this continues where the previous call left off
  strcpy(messageFromPC, strtokIndx); // this is a text string: copy it to messageFromPC:
  // sensor identification, sensor "A" or sensor "B", or C or D
  Serial.println(F(" 6 "));
  Serial.println();


Actually, I have higher hopes for the addition of another token after the A.  It could be a ',' or a '.' --just another character that strtok is looking for instead of the end of the string when looking for a non existent ','.
So far so good, I have been simulating several hours of transmissions/receptions, no crashes so far (in the previous setup there would have been several crashes). I keep this running another 24 hours (RX every 5 seconds) and let you know the result.

brice3010

I think the suggestion was to add the final token character in the receiving code instead of in the transmission. In Robin's code, the end marker is replace by '\0' and it was suggested that a separator be added before the Null. You could probably send the final separator in the transmission and add another at the receive.

I think the bigger issue is why strtok with the ESP8266 appears to not find the null terminator when parsing something at the end of the character string.

After 24h testing (RX every 5 seconds instead of every 20 minutes: simulating 10 days in each hour of the past 24h), 2 crashes occured. An order of magnitude better than before the change according to cattledog suggestion.

But what can be done to eliminate these spurious crashes completely? The test is now run in ideal lab conditions, and I suspect the same issue being present because the crash still occurs when parsing the last part: the transmitter ID (A.. to D).
Is there a fallback for this last parsing instruction, ie building in a condition to somehow verify invalid ID characters?
Or would the consensus be that this result is "good enough"?


Robin2

Is there a fallback for this last parsing instruction, ie building in a condition to somehow verify invalid ID characters?
Or would the consensus be that this result is "good enough"?
Why not check the tail end of the data before starting to parse. Then you can add a suitable terminator if needed. Just make sure that the array into which you receive the data is ALWAYS a few characters larger than the largest message that may be received.


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

brice3010

#25
Jan 17, 2019, 05:06 pm Last Edit: Jan 17, 2019, 06:30 pm by brice3010
Why not check the tail end of the data before starting to parse. Then you can add a suitable terminator if needed. Just make sure that the array into which you receive the data is ALWAYS a few characters larger than the largest message that may be received.


...R
Good idea. Can you give some clue how this can be done without parsing please?
Currently the data to be parsed sits in tempchars[numChars] where numChars is 32. The data sent is in this format: <47.50,12.50,6.50,5,10,A> (hence: 3 floats, 2 integers and one character A or B or C or D).

EDIT: the format currently tested is with A. hence: <47.50,12.50,6.50,5,10,A.>

cattledog


Quote
After 24h testing (RX every 5 seconds instead of every 20 minutes: simulating 10 days in each hour of the past 24h), 2 crashes occured
In your testing, did you add the extra terminator to the transmission and again at the receive when the end marker is found?

Quote
<47.50,12.50,6.50,5,10,A>
Are the transmitted values always that length, or can the number of digits in the floats or integers change?

If the length is constant, and I count correctly, tempchars[23] should be the letter, tempchars[24] should be the final separator(',' or '.'), and tempchars[25] should be '\0'.


brice3010

#27
Jan 17, 2019, 06:21 pm Last Edit: Jan 17, 2019, 06:24 pm by brice3010
In your testing, did you add the extra terminator to the transmission and again at the receive when the end marker is found?
Yes.

This is the code in the transmitter used to send the packet of data:
Code: [Select]

    Serial.print('<'); // this section for HC-12 transmission
    Serial.print(cycleValue);  // ref Robin2's Serial Output Basics
    Serial.print(',');   // http://forum.arduino.cc/index.php?topic=396450.0
    //    Serial.print(temperatureT1); // send temperature read from analog input temperaturePin
    Serial.print(get3231Temp()); // read DS3231 temperature
    Serial.print(',');
    Serial.print(batteryVoltage); // send battery voltage
    Serial.print(',');
    Serial.print(now.minute(), DEC); // send minute
    Serial.print(',');
    Serial.print(now.second(), DEC); // send hour
    Serial.print(",A.>"); // send sensor identifier
    Serial.println();


This is the code at the receiver to parse the last section (the identifier A, or B, or C or D):

Code: [Select]

  strtokIndx = strtok(NULL, "."); // identifier parsing



Are the transmitted values always that length, or can the number of digits in the floats or integers change?

If the length is constant, and I count correctly, tempchars[23] should be the letter, tempchars[24] should be the final separator(',' or '.'), and tempchars[25] should be '\0'.
Yes, the length always is constant.

Ok, I try that suggestion; the reason I kept 32 is because at the time this was still in prototype testing. By now I know this length will not alter anymore.





cattledog

It's interesting that there are no other errors with the data except for the tail end. More robust error checking with check sums, could be incorporated, and might not be a bad idea.

As a quick test with your current code, to both fix the error and see where it is, you could try the following

Code: [Select]

if (newData == true) {
  if (receivedChars[24] != '.')
  {
    Serial.println("Missing .");
    receivedChars[24] = '.';
  }
  if (receivedChars[25] != '\0')
  {
    receivedChars[25] = '\0';
    Serial.println("Missing NULL");
  }
  strcpy(tempChars, receivedChars);
  // this temporary copy is necessary to protect the original data
  //   because strtok() used in parseData() replaces the commas with \0
  //
  // Serial.println(receivedChars); // serial monitor output; not usefull on ESP8266
  // Serial.println();
  parseData();
  // useParsedData(); // still issue with execution and timing (9/5/2018); solved: use in irrigationCommand();
  // thingSpeak();
  newData = false;
}

brice3010

It's interesting that there are no other errors with the data except for the tail end. More robust error checking with check sums, could be incorporated, and might not be a bad idea.

As a quick test with your current code, to both fix the error and see where it is, you could try the following

Code: [Select]

if (newData == true) {
  if (receivedChars[24] != '.')
  {
    Serial.println("Missing .");
    receivedChars[24] = '.';
  }
  if (receivedChars[25] != '\0')
  {
    receivedChars[25] = '\0';
    Serial.println("Missing NULL");
  }
  strcpy(tempChars, receivedChars);
  // this temporary copy is necessary to protect the original data
  //   because strtok() used in parseData() replaces the commas with \0
  //
  // Serial.println(receivedChars); // serial monitor output; not usefull on ESP8266
  // Serial.println();
  parseData();
  // useParsedData(); // still issue with execution and timing (9/5/2018); solved: use in irrigationCommand();
  // thingSpeak();
  newData = false;
}

Checksums: indeed may become necessary.
Your suggestion above: will be done now, checking back in a few days.
Thank you cattledog.

Go Up