[SOLVED]Breaking down a string won't work right

Hello!

I'm new to the forum, so please don't shoot me if I'm not exactly following procedures for posting a problem :blush:

But here it is:
I'm trying to control KlikAanKlikUit-reiceivers with an arduino uno talking (over IP, UDP protocol) to a vb6 project on my PC

Here's the sketch, in fact, it is the udpsendreceivestring example, where I added a few snippets found on this forum, plus a few extra ' debug' serial.println commands...

#include <NRT.h>
#include <Wire.h>
#include <SPI.h>         // needed for Arduino versions later than 0018
#include <Ethernet.h>
#include <EthernetUdp.h>         // UDP library from: bjoern@cs.stanford.edu 12/30/2008
#include <string.h>

byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
unsigned int localPort = 8888;      // local port to listen on

byte group = 0;
byte unit = 0;


// buffers for receiving and sending data
char packetBuffer[UDP_TX_PACKET_MAX_SIZE]; //buffer to hold incoming packet,
char ReplyBuffer[] = "acknowledged";       // a string to send back

// An EthernetUDP instance to let us send and receive packets over UDP
EthernetUDP Udp;
NRT transmitter(538,5,260,3);

void setup() {
  // start the Ethernet and UDP:
  Ethernet.begin(mac);
  Udp.begin(localPort);
  Serial.begin(9600);
}

void loop() {
  // if there's data available, read a packet
  int packetSize = Udp.parsePacket();
  if(packetSize)
  {
    Serial.print("Received packet of size ");
    Serial.println(packetSize);
    Serial.print("From ");
    IPAddress remote = Udp.remoteIP();
    for (int i =0; i < 4; i++)
    {
      Serial.print(remote[i], DEC);
      if (i < 3)
      {
        Serial.print(".");
      }
    }
    Serial.print(", port ");
    Serial.println(Udp.remotePort());

    // read the packet into packetBufffer
    char *Command[10];
    Udp.read(packetBuffer,UDP_TX_PACKET_MAX_SIZE);
    Serial.println("Contents:");
    Serial.println(packetBuffer);
    Serial.println("Split:");
    
    // Split the command
    char *p = packetBuffer; //point to *p to the string in inData
    char *str;        //declaring *str
    int counter = 0; //initialise the counter
    while (str = strtok_r(p, ";", &p)) // delimiter is the semicolon
            {
      Command[counter] = str; //use the counter as an index to add each value to the array
      Serial.println(Command[counter]);
      counter++; //increment the counter
             }
      char* konijn=Command[0];
      Serial.println(konijn);
      int sLen = sizeof(konijn);
for(int s=0; s<sLen; s++)
{
   Serial.print("konijn[");
   Serial.print(s);
   Serial.print("] is {");
   Serial.print(konijn[s]);
   Serial.print("} which has an ascii value of ");
   Serial.println(konijn[s], DEC);
}       
    if (Command[0]=="SWITCH"){

      Serial.println("Switching");//This line never shows up in the serial monitor....!
      transmitter.sendUnit(atoi(Command[1]),atoi(Command[2]),Command[3]=="1");
    }
      

    // send a reply, to the IP address and port that sent us the packet we received
    Udp.beginPacket(Udp.remoteIP(), Udp.remotePort());
    Udp.write(ReplyBuffer);
    Udp.endPacket();
  }
  delay(10);
}

And this is the serial output:

Received packet of size 13
From 192.168.0.108, port 8888
Contents:
SWITCH;1;3;1
Split:
SWITCH
1
3
1
SWITCH
konijn[0] is {S} which has an ascii value of 83
konijn[1] is {W} which has an ascii value of 87

The fact that it never prints the line Switching tells me the comparison Command[0]=="SWITCH" doesn't work...
The fact that sizeof(konijn) seems to be 1 confirms that.

Am I doing something wrong in the strtok_r routine?

This not work for strings (vectors of char) but only for two String objects:

if (Command[0]=="SWITCH"){

for two string or text you need a compare function like strcmp() for strings (vectors of char)
http://www.cplusplus.com/reference/cstring/strcmp/?kw=strcmp

or compareTo() for String objects.

Ok, so replacing that with if(strcmp(Command[0],"SWITCH")) { should do the trick?

I will test it tonight, and give feedback, thanks!

char *Command[10];

An array by it self is alread a pointer to position 0.Here you are creating 10 pointers

if(strcmp(Command[0],"SWITCH")) {

This should be something like:

if(strcmp(*Command[0],"SWITCH") == 0) {

But pay attention in order this to work each string must be correct terminated with a '\0' to be a valid string

While you are rewriting your code, get rid of strtok_r(). Use the much simpler strtok(), instead. The _r version is the thread-safe, re-entrant version of strtok(). On a single-threaded system, thread safety is not an issue.

HugoPT, I don't get your first remark. What I'm trying to do is to put my message in an array, part for part. I've learned that a string is in fact an array of characters, so I created a 2-dimensional char array. The message will never have more than 10 parts.
Command[0] will be the real command to execute, and Command[1] to [9] will be its arguments.

Is that not correct?

And I noticed my mistake on the comparison when I took a good look at the C++ reference... :blush:

Blackys_Boss:
Ok, so replacing that with if(strcmp(Command[0],"SWITCH")) { should do the trick?

Almost.

char * Command[] = {"SWITCH", "1", "3"};
// ...
  if(strcmp(Command[0],"SWITCH") == 0) {
    char* p1 = Command[1];
    int i = atoi(p1);  // string to int
    Serial.print("Found SWITCH;");Serial.print(i);
  }

strcmp() returns 0 when equal

Edit: finding bugs even before compiling is a bad sign.

michael_x:
Edit: finding bugs even before compiling is a bad sign.

u-huh :blush: :blush:

But it works perfect now!
Here's (the important part of) the new code: //I also cleaned it up a bit :grin:

    // read the packet into packetBufffer
    Udp.read(packetBuffer,UDP_TX_PACKET_MAX_SIZE);
    Serial.print("Contents: ");
    Serial.println(packetBuffer);
        
    // Split the packetBuffer
    char *Command[10]; //declare the command array
    char *p = packetBuffer; //point to *p to the string in packetBuffer
    char *str;        //declaring *str
    int counter = 0; //initialise the counter
    while (str = strtok_r(p, ";",&p)){ // delimiter is the semicolon
      Command[counter] = str; //use the counter as an index to add each value to the array
      counter++; //increment the counter
    }
    
    //Execute the command
    if (strcmp(Command[0],"SWITCH")==0){
      Serial.print("Switching unit ");
      Serial.print(Command[2]);
      Serial.print(" in group ");
      Serial.print(Command[1]);
      Serial.print(" to ");
      Serial.println(strcmp(Command[3],"1")==0);
      transmitter.sendUnit(atoi(Command[1]),atoi(Command[2]),strcmp(Command[3],"1")==0);
    }
    if (strcmp(Command[0],"DIM")==0){
      Serial.print("Dimming unit ");
      Serial.print(Command[2]);
      Serial.print(" in group ");
      Serial.print(Command[1]);
      Serial.print(" to level ");
      Serial.println(Command[3]);
      transmitter.sendDim(atoi(Command[1]),atoi(Command[2]),atoi(Command[3]));
    }
    if (strcmp(Command[0],"GROUP")==0){
      Serial.print("Switching group ");
      Serial.print(Command[1]);
      Serial.print(" to ");
      Serial.println(strcmp(Command[3],"1")==0);
      transmitter.sendGroup(atoi(Command[1]),strcmp(Command[2],"1")==0);
    }
    // send a reply, to the IP address and port that sent us the packet we received
    Udp.beginPacket(Udp.remoteIP(), Udp.remotePort());
    Udp.write(ReplyBuffer);
    Udp.endPacket();

@PaulS, I tried the strtok() (no _r), but it instantly f*#ked up everything, and if the _r version does work, at the cost of 4 whole bytes... Never change a winning team 8)

Thnx to all, now for the next part, implementing RTC...

@PaulS, I tried the strtok() (no _r), but it instantly f*#ked up everything, and if the _r version does work, at the cost of 4 whole bytes... Never change a winning team

I've never had a problem using strtok(). The strtok_r() version is much more than 4 bytes larger than the strtok() version, if strtok() is used properly.