strtok_r spooky action

Here is my current program

char packetBuffer[UDP_TX_PACKET_MAX_SIZE] = "ROW1 255 1 2 3 4";

void loop() {
  char *ptr = NULL;
  char *term[5];
  term[0] = strtok_r(packetBuffer, " ", &ptr);
  for(int i = 1; i <= 5; i++) {
    term[i] = strtok_r(NULL, " ", &ptr);
  }
  char *section = term[0];
  int r = atoi(term[1]);
  int g = atoi(term[2]);
  int b = atoi(term[3]);
  int w = atoi(term[4]);
  int deltaValue = atoi(term[5]);
  Serial.println(section);
  Serial.println(r);
  Serial.println(g);
  Serial.println(b);
  Serial.println(w);
  Serial.println(deltaValue);
  delay(1000);
}

The first time it loops it works great, then it gets weird:

ROW1
255
1
2
3
4
ROW1
0
0
0
0
0

Is something in the program incorrect or is it just a result of me looping through parsing the same thing? If it is the latter, no big deal, I'm just doing testing and of course the final version will only parse it again if the char array changes. Also, is there a slick way for me to simply those repeating
"int r =...
int g =..
etc"
into some sort of loop or not?

Also, this error I'm receiving is annoying:
"iteration 4 invokes undefined behavior"

Thank you all.

is it just a result of me looping through parsing the same thing?

Yes. On second and subsequent passes you are parsing an empty string

"iteration 4 invokes undefined behavior"

Caused by parsing the empty string and warning you that the result cannot be predicted and not to rely on it.

Also, is there a slick way for me to simply those repeating
"int r =...
int g =..
etc"

Why not just use the values in the array ? If you need them as integers then do the atoi() at the same time as the strtok and declare term as an array of ints and save the initial text in a separate variable.

That sketch does not compile, nor would it print anything. You should always post the code that produces the output for your question.

[OK, you edited your post so my answer is quite different. This is called "post vandalism". :-/ It's ok to make minor changes, but completely changing the sketch should have been in a second post.]

I think the main problem is understanding the difference between "a pointer to a char", "a char array" and "a char".

  • a char is one byte of RAM, typically used to store the ASCII value of a character:
    char oneChar = 'A';
    Serial.print( oneChar ); // prints an 'A' on the Serial monitor window
  • a pointer to a char is the address of the char in RAM:
    char oneChar = 'A';
    char *ptr = &oneChar; // the "address" of oneChar
  • a char array is declared as
    char array[ dimension ]; // all zero bytes
    char array[] = "initial value, dimension is length+1";
    char array[ 128 ] = "initial value fills the first length+1 bytes of the array, remainder are zero bytes";

The array's name is, well, a name for the address of the first element. Array indices start at 0, so the first element can be accessed with

    Serial.print( array[0] ); // print one element, a char

Because the array name is also an address, you can get the RAM value stored at that address by "dereferencing" it with the '*' operator:

    Serial.print( *array ); // print one element, a char

Complicating the char/array/pointer topics is the concept of a C string. When you use a "double-quoted" string constant (not the String class), the compiler adds an extra zero byte at the end. That's called the NUL terminator. Many routines can use these "C strings" (e.g., Serial.print). They all know to access sequential elements until a zero byte is encountered. That's the end of the string.

You can pass a char array or a char pointer to Serial.print, and it will print the sequential RAM locations until a zero byte is encountered.

If you pass a char to Serial.print, it will only print one character. Passing *section will only print the first character of the field.

I think you wanted to do this:

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

void loop() {
  const size_t UDP_TX_PACKET_MAX_SIZE = 64;
  char  packetBuffer[UDP_TX_PACKET_MAX_SIZE] = "ROW1 255 1 2 3 4";
  char *ptr;

  char *section = strtok_r(packetBuffer, " ", &ptr);
  Serial.print( F("section = '") );
  Serial.print( section );
  Serial.println( '\'' );

  char *r = strtok_r(NULL, " ", &ptr);
  Serial.print( F("r = '") );
  Serial.print( r );
  Serial.print( "', r value = " );
  Serial.println( atoi(r) );

  char *g = strtok_r(NULL, " ", &ptr);
  char *b = strtok_r(NULL, " ", &ptr);
  char *w = strtok_r(NULL, " ", &ptr);
  char *deltaTime = strtok_r(NULL, " ", &ptr);
  Serial.print( F("deltaTime = '") );
  Serial.print( deltaTime );
  Serial.println( '\'' );

  delay(1000);
}
  1. @/dev, sorry for the post vandilation, thank you for the reply. It is probably the most comprehensive explanation of pointers, chars, and char arrays I have seen.

I updated the program:

#include <Ethernet.h>
#include <DmxSimple.h>

byte mac[] = {
  0xAE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED // arduino mac address
};

IPAddress ip(1, 1, 1, 49); //arduino ip

unsigned int localPort = 8888; //port to listen on

int deltaValue[18]; //channel change per refresh
int finalValue[18];
int currentValue[18];
int refreshRate = 25; //milliseconds

EthernetUDP Udp; //UDP configuration

void setup() {
  for (int i = 3; i <= 18; i++) { //configure all arrays as 0 for 3-18 (the channels on the dmx univers)
    deltaValue[i] = 0;
    finalValue[i] = 0;
    currentValue[i] = 0;
  }
  pinMode(2, OUTPUT);
  digitalWrite(2, HIGH); //configure shield as master
  DmxSimple.usePin(4); //initialize communication with shield on pin 4
  DmxSimple.maxChannel(18); //18 channels in total

  Ethernet.begin(mac, ip); //start ethernet and UDP
  Udp.begin(localPort);
  Serial.begin(9600);
}

void parseAndCalculate() {
  char packetBuffer[UDP_TX_PACKET_MAX_SIZE];  //buffer to hold incoming packet
  int packetSize = Udp.parsePacket();
  if (packetSize) { //if a packet is received, parse it and update the three arrays
    Udp.read(packetBuffer, UDP_TX_PACKET_MAX_SIZE); //read packet into packetBuffer

    //extract the info from the UDP packet
    char *ptr = NULL; //pointer for strtok_r
    char *term[5]; //array to read strtok_r values into
    term[0] = strtok_r(packetBuffer, " ", &ptr);
    for (int i = 1; i <= 5; i++) { //read each packet as a single string into term[]
      term[i] = strtok_r(NULL, " ", &ptr);
    }
    int deltaT = atoi(term[5]);

    //now that the info is extracted from the UDP packet, it is time to begin computation
    int channelStart;
    if (term[0] = "ROW1") {
      channelStart = 3;
    } else if (term[0] = "ROW2") {
      channelStart = 7;
    } else if (term[0] = "ROW3") {
      channelStart = 11;
    } else if (term[0] = "STRS") {
      channelStart = 15;
    }
    for (int i = 0; i <= i + 3; i++) {
      deltaValue[i + channelStart] = (atoi(term[i + 1]) - currentValue[i + channelStart]) / (deltaT * 1000 / refreshRate);
    }
  }
}

  void loop() {
    parseAndCalculate();
  }

It uploads to the board but there are tons of errors:

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino: In function 'void parseAndCalculate()':

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:52:17: warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]

if (term[0] = "ROW1") {

^

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:54:24: warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]

} else if (term[0] = "ROW2") {

^

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:56:24: warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]

} else if (term[0] = "ROW3") {

^

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:58:24: warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]

} else if (term[0] = "STRS") {

^

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino: In function 'setup':

E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:21:22: warning: iteration 15 invokes undefined behavior [-Waggressive-loop-optimizations]

deltaValue = 0;

  • ^*
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:20:3: note: containing loop
  • for (int i = 3; i <= 18; i++) { //configure all arrays as 0 for 3-18 (the channels on the dmx univers)*
  • ^*
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino: In function 'parseAndCalculate()':
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:46:42: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
    term = strtok_r(NULL, " ", &ptr);
    * ^*
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:45:5: note: containing loop
    * for (int i = 1; i <= 5; i++) { //read each packet as a single string into term[]*
    * ^*
    C:\Program Files (x86)\Arduino\hardware\arduino\avr\cores\arduino\main.cpp: In function 'main':
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:46:42: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
    term = strtok_r(NULL, " ", &ptr);
    * ^*
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:45:5: note: containing loop
    * for (int i = 1; i <= 5; i++) { //read each packet as a single string into term[]*
    * ^*
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:21:22: warning: iteration 15 invokes undefined behavior [-Waggressive-loop-optimizations]
    _ deltaValue = 0;
    * ^
    E:\Planetarium\ArduinoDMX\PlanDmxLightingV1\PlanDmxLightingV1.ino:20:3: note: containing loop
    for (int i = 3; i <= 18; i++) { //configure all arrays as 0 for 3-18 (the channels on the dmx univers)
    ^
    [/quote]*

    First, we have the warning that basically comparing pointers to strings is not allowed, I guess. Could this become a problem down the road? What is the correct way to compare these pointers to strings?
    Next is regarding the for loop that assigns all values 3-18 of the three arrays. It says iteration 15 of the deltaValue array "invokes undefined behavior." Again, what does this mean? Why is it only for deltaValue and not the other two arrays in the for loop? Could this become a problem down the road?
    Finally, the next is the same as the last problem. The last term of the strtok_r loop is a problem. I don't know why this is.
    Thank you_
int deltaValue[18]; //channel change per refresh

An array of 18 ints numbered 0 to 17

 for (int i = 3; i <= 18; i++) { //configure all arrays as 0 for 3-18 (the channels on the dmx univers)
    deltaValue[i] = 0;

Initialise elements 3 to 18 of the array

Can you see a problem ?

In any case, global arrays of ints are initialised to zero by default

strtok_r() is the thread-safe version of strtok(). To be thread-safe, it needs a lot more arguments than strtok(), takes up more flash space, and is completely unnecessary on a system that doesn't have threads.

    char *term[5]; //array to read strtok_r values into
    term[0] = strtok_r(packetBuffer, " ", &ptr);
    for (int i = 1; i <= 5; i++) { //read each packet as a single string into term[]
      term[i] = strtok_r(NULL, " ", &ptr);
    }

You are trying to write to 6 elements of a 5 element array.

Before you use the elements of the term array, you should print what term[ i ] points to. You might be surprised.

Thank you both very much for clearing those things up. I got confused by the total number of terms in an array. I mistakenly thought array[18] had terms 0-18, but that would mean it had 19 terms. this is what happened with the strtok function as well.

Thank you PaulS for explaining the functional difference between strtok_r and strtok.

Also, I am wondering, what is the proper thing to compare a char array pointer to to find out what it is. Comparing it to a string brings up errors.

Thank you all very much.

Alright, I figured it out. I just needed to compare the String(char*) to the string. Thank you all.

PureStress:
what is the proper thing to compare a char array pointer to to find out what it is. Comparing it to a string brings up errors... Alright, I figured it out. I just needed to compare the String(char*) to the string.

Gar, you were doing so well... :slight_smile:

Don't use String™. There are many reasons to avoid it. In your case, it is very easy to avoid. Just use the strcmp function and pass it the two strings you want to compare:

     if (strcmp( term[0], "ROW1") == 0) {

     } else if (strcmp( term[0], "ROW2" ) == 0) {

     } else if (strcmp( term[0], "ROW3" ) == 0) {

     } else if (strcmp( term[0], "STRS" ) == 0) {

This will save about 1600 bytes of program space, because it does not need the String library (and associated dynamic memory code). Although speed probably isn't an issue for you, strcmp is also much faster.

If you want to save a little RAM, too, you can use the strcmp_P variant:

         } else if (strcmp_P( term[0], PSTR("ROW3") ) == 0) {

On these processors, double-quoted strings will be copied from FLASH into RAM. Using the PSTR macro avoids this intermediate copy and compares the double-quoted string directly from FLASH.

You can accomplish the same thing with the F macro when printing a double-quoted string:

    Serial.print( F("double-quoted string") ); // saves RAM!

If you have a lot of these strings in your sketch, it can add up quickly.

For my old code I used to have a single = instead of double (==). Now that I simply have if (term[0] == "ROW1") [instead of accidentally assigning term[0] with a single =] it doesn't generate any errors. Will this work, or is strcmp necessary? I haven't had a chance to test it out. Is the == exclusively to compare numbers, or will it work on strings?

is strcmp necessary?

Yes if term is an array of strings (lowercase s) rather than an array of Strings (uppercase S)

Is the == exclusively to compare numbers, or will it work on strings?

It works with numbers and I believe that it works with Strings (uppercase S) but it does not work with strings (lowercase s)

Now that I simply have if (term[0] == "ROW1") [instead of accidentally assigning term[0] with a single =] it doesn't generate any errors. Will this work, or is strcmp necessary? I haven't had a chance to test it out. Is the == exclusively to compare numbers, or will it work on strings?

Like UKHeliBob said, you have to use strcmp to compare C strings (char arrays).

The reason is this: using the "==" operator to compare two C strings will compare their starting RAM addresses, not their contents (i.e., the RAM bytes stored in those addresses).

To compare the contents, you must use strcmp. It compares one byte from each address, then steps to the next address. When it gets a zero byte from one or both of those sequential addresses (array elements), it know that it has reached the end of the C string. That's why the NUL terminator character is there... to let you know the last element of the char array.

Your sketch has two different C strings. One is defined by the double-quoted literal "ROW1" in your code. The compiler allocates a little RAM to hold the contents: 4 characters plus a NUL character.

The second C string, term[0] is a portion of the original packetBuffer variable. Each term array element is pointing into the packetBuffer array. That is, each term element is the address of some characters in the packetBuffer array.

The strtok function starts at the beginning of the packetBuffer array (a specific RAM address), and compares one element (a char at that address) with the delimiters (also an array of characters). After scanning the packetBuffer (and some other things), it eventually returns the address of the token it found. You store that address in the term array. Later, you use it to see what tokens where in the packetBuffer.

The packetBuffer array is stored at a different RAM address than the "ROW1" literal, so comparing their addresses with "==" will always say they are different (not what you want). Using strcmp compares each byte at those addresses. That's what you want.