string to variable help

Im trying to parse a comma delimited serialread string to use as variables or act on the result.
In my searching around I found a post that seems to be helpful and while not exactly what I was looking should work until I figure out how to use a variable from a function outside that function and will cleanup the code.
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1231961642
this test snippet works fine.

#include <string.h>

char *record = "name:bob";
char *p, *i;

void setup() {

   Serial.begin(9600);
   Serial.println("Starting..");

    //  First strtok iteration
    p = strtok_r(record,":",&i);
    Serial.print(p);
    Serial.print(" = ");

    //  Second strtok iteration
    p = strtok_r(NULL,":",&i);
    Serial.print(p);
    Serial.println("");

}

void loop () {
}

Now when I Frankenstein it into my current sketch I get compiler errors because I'm mixing types char*, char and int. Problem being I'm about 2 steps over my head on this and I havent been able to find the syntax to convert it.
p = strtok(record,":",&i);

g:/dev/arduino/arduino-0022/hardware/tools/avr/lib/gcc/../../avr/include/string.h: In function 'void doSensorcheckanddisplay()':
g:/dev/arduino/arduino-0022/hardware/tools/avr/lib/gcc/../../avr/include/string.h:145: error: too many arguments to function 'char* strtok(char*, const char*)'
GroPi5000v0_2:228: error: at this point in file
GroPi5000v0_2:233: error: cannot convert 'int*' to 'char**' for argument '3' to 'char* strtok_r(char*, const char*, char**)'
GroPi5000v0_2:236: error: ISO C++ forbids comparison between pointer and integer
GroPi5000v0_2:248: error: jump to case label
GroPi5000v0_2:227: error: crosses initialization of 'char* record'
GroPi5000v0_2:252: error: jump to case label
GroPi5000v0_2:227: error: crosses initialization of 'char* record'

 int availableBytes = Serial.available();
  if (availableBytes > 0)
  {
    for(int i=0; i<availableBytes; i++) 
    {
      int ch = Serial.read();
      switch(ch)
      {
      case '#':
        char *record = string;
        p = strtok(record,":",&i);
        Serial.print(p);
        Serial.print(" = ");

        //  Second strtok iteration
        p = strtok_r(NULL,",",&i);
        Serial.print(p);
        Serial.println("");       
        if (CO2LevelValue < p)
        {
          digitalWrite(co2relaypin, HIGH);   // turn the co2 on
        }
        else {
          digitalWrite(co2relaypin, LOW);   // set the LED on
        }
        Serial.println(string);

        curWritePos=0;
        string[curWritePos]=0;
        break;
      case '

** edited for viewing pleasure
what this does is get check for a string with $ startbit and # stopbit and parses the csv in between. eg $123,112.4,1000,1,2,6.5#
And then each strtok iteration lets me act on that value.

Of course ideally I would just be creating variables with each strtok but I could never get a variable from inside a function to work outside it or in another function. But alas a topic for another day.
I think I gave enough code and detail but let me know if more clarification is needed.

:
        curWritePos=0;
        string[curWritePos]=0;
        break;
      default:
        if (curWritePos < sizeof(string)-1)
        {
          string[curWritePos++]=ch;
          string[curWritePos]=0;
        }
        else
        {
          //      Serial.println("Overflow\n");
        }
        break;
      }
    }
  }


** edited for viewing pleasure
what this does is get check for a string with $ startbit and # stopbit and parses the csv in between. eg $123,112.4,1000,1,2,6.5#
And then each strtok iteration lets me act on that value. 

Of course ideally I would just be creating variables with each strtok but I could never get a variable from inside a function to work outside it or in another function. But alas a topic for another day.
I think I gave enough code and detail but let me know if more clarification is needed.

jointtech:
Now when I Frankenstein it into my current sketch I get compiler errors ...

You do what to it?

yea exactly. I take bits and pieces from the forum graveyard and splice them together into "The Monster". :wink:
and here is the left leg I just attached.

   char *record = string;
        p = strtok(record,":",&i);
    Serial.print(p);
    Serial.print(" = ");

    //  Second strtok iteration
    p = strtok_r(NULL,",",&i);
    Serial.print(p);
    Serial.println("");

I see. So you are using undead code, is that it?

Putting that aside, this looks wrong:

        for(int i=0; i<availableBytes; i++) 
        {
           int ch = Serial.read();
           switch(ch)
           {
               case '#':
               char *record = string;
        p = strtok(record,":",&i);
    Serial.print(p);
    Serial.print(" = ");

Where does the switch terminate? Your indentation suggests you got a bit bored with it and moved on.

the switch statement looks like this. Yes my cut and paster didnt format well. The switch works fine when just serial.print ing the result it gets (echo)

case '#':
        char *record = string;
        p = strtok(record,":",&i);
        Serial.print(p);
        Serial.print(" = ");

        //  Second strtok iteration
        p = strtok_r(NULL,",",&i);
        Serial.print(p);
        Serial.println("");       
        if (CO2LevelValue < p)
        {
          digitalWrite(co2relaypin, HIGH);   // turn the co2 on
        }
        else {
          digitalWrite(co2relaypin, LOW);   // set the LED on
        }
        Serial.println(string);

        curWritePos=0;
        string[curWritePos]=0;
        break;
        char *record = string;
        p = strtok(record,":",&i);
        Serial.print(p);
        Serial.print(" = ");

        //  Second strtok iteration
        p = strtok_r(NULL,",",&i);

We can't tell from this snippet what string is, so it's impossible to say if the pointer assignment is correct. The third argument to strtok is optional. If you don't care that the value is, leave it out.

Why are you using strtok() and strtok_r()? The _r version is the thread safe version. Hardly a concern in an environment where threads can not exist. It carries a lot of overhead that you don't need. Using a thread-safe version to manipulate the data set up by a non-thread-safe version defeats all the safety features that the thread-safe version adds.

Finally, you have not explained what it is that you expect the code to do, whether or not it does exactly that, and, if not, how what it does do differs from what you want it to do.

i cant get it to compile so I dont really know what it does yet.
In theory from the code I have posted currently it should take this string $123,456,789#
and during the second iteration of strtok p should = 123

I just noticed that I've been posting code incorrectly.
p = strtok(record,":",&i);
should be
p = strtok(record,",",&i);

i replaced my comma with a colon just to see if it was somehow choking on that.

Im posting the full code here. Sorry its long.

//Arduino Dingle Dongle for GroPI5000  (CO2, Temp, Humidity, Flood Alert, Light, water temp, PPM, PH)

// Library Includes
#include <SoftwareSerial.h>
#include <SparkFunSerLCD.h>
#include <SHT1x.h>
#include <OneWire.h>
#include <stdlib.h>
//Pin Assignments
//A0
int PHPin = 0;  // Analog input pin that the phidget ph sensor uses
//A1
// WATER LEVEL SENSOR
//A2
int lightsensorpin = 2;
//A3
int TDSPin = 3;  // Analog input pin that the PSC-154 uses
//A4
int CO2Pin = 4;                //CO2 Sensor Pin
//A5

//D0 - RX
// TO DDWRT
//D1 - TX
// TO DDWRT
//D2
int waterflowmeterpin = 2;            //The pin location of the sensor
//D3
int co2relaypin = 3;//RELAY CO2 SOLINOID 
//D4
//RELAY AUTO TOP OFF
//D5
//RELAY PH PUMP
//D6
OneWire ds(6);  // DS18S20 Temperature chip i/o
//D7
SparkFunSerLCD led(7,2,16); // desired pin, rows, cols
//D8
int FloodPin = 8;              //Flood Sensor Pin
//D9
//RELAY ADD NEUTS PUMP
//D10
//RELAY WATER PUMP
//D11
#define sht15clockPin 11
//D12
#define sht15dataPin  12
//D13

//init vars
SHT1x sht1x(sht15dataPin, sht15clockPin);  //SHT15 temp/humidity sensor
int CO2SensorValue = 0;        //CO2 Sensor Value
int CO2LevelValue = 0;         //CO2 Level Value
int FloodValue = 0;            //Flood Sensor value
volatile int NbTopsFan;        //measuring the rising edges of the signal
int Calc;                               
int count = 0;
int phsensorvalue = 0;        // value read from the phidget PH sensor
float PHValue = 0;        // PH value output to serial
int TDSsensorValue = 0;        // value read from the PSC-154
int TDSValue = 0;        // TDS value output to serial
float temp_f;
float humidity;
char string[32];
int curWritePos;
char *p, *i;
//run program
void setup()                    // run once, when the sketch starts
{
  Serial.begin(115200);
  pinMode(waterflowmeterpin, INPUT); //initializes digital pin 2 as an input
  pinMode(co2relaypin, OUTPUT); 
  attachInterrupt(0, rpm, RISING); //and the interrupt is attached
  curWritePos=0;
  string[curWritePos]=0;
  led.setup();
  led.off();
  led.on();
  led.cursorOff();

}

void loop()        {             // run over and over again
  doSensorcheckanddisplay();
}

void rpm ()     //This is the function that the interupt calls for water flow meter
{ 
  NbTopsFan++;  //This function measures the rising and falling edge of the hall effect sensors signal
} 

void doSensorcheckanddisplay()
{
  // Read values from the SHT15
  temp_f = sht1x.readTemperatureF();
  humidity = sht1x.readHumidity();
  //Water Flow Meter
  NbTopsFan = 0;	//Set NbTops to 0 ready for calculations
  //  sei();		//Enables interrupts didnt seem to work properly
  delay (1000);	//Wait 1 second
  //  cli();		//Disable interrupts didnt seem to work properly
  Calc = (NbTopsFan * 60 / 7.5); //(Pulse frequency x 60) / 7.5Q, = flow rate 
  Calc = (Calc * 0.264172052); //Convert to Gallons.  Dirty eurotrash sensors 
  //CO2 Sensor
  CO2SensorValue = analogRead(CO2Pin);
  CO2LevelValue = map(CO2SensorValue, 0, 1023, 0, 2000); 
  //Light Sensor
  int lightsensorValue;
  lightsensorValue = analogRead(lightsensorpin);     // read analog input pin 0
  // PH
  phsensorvalue = analogRead(PHPin);   
  PHValue= (7 - (2.5 - phsensorvalue / 200) / (0.257179 + 0.000941468 * 22.7));
  //TDS
  TDSsensorValue = analogRead(TDSPin);            
  // map it to the range of the analog out:
  TDSValue = map(TDSsensorValue, 202, 1023, 0, 10000);  
  //Water Temp
  int HighByte, LowByte, TReading, SignBit, Tc_100, Whole, Fract;
  byte i;
  byte present = 0;
  byte data[12];
  byte addr[8];
  if ( !ds.search(addr)) {
    //       Serial.print("No more addresses.\n");
    ds.reset_search();
    return;
  }

  //     Serial.print("R=");
  for( i = 0; i < 8; i++) {
    //       Serial.print(addr[i], HEX);
    //      Serial.print(" ");
  }

  if ( OneWire::crc8( addr, 7) != addr[7]) {
    //        Serial.print("CRC is not valid!\n");
    return;
  }

  if ( addr[0] == 0x10) {
    //        Serial.print("Device is a DS18S20 family device.\n");
  }
  else if ( addr[0] == 0x28) {
    //        Serial.print("Device is a DS18B20 family device.\n");
  }
  else {
    //        Serial.print("Device family is not recognized: 0x");
    //        Serial.println(addr[0],HEX);
    return;
  }

  ds.reset();
  ds.select(addr);
  ds.write(0x44,1);         // start conversion, with parasite power on at the end

  delay(1000);     // maybe 750ms is enough, maybe not
  // we might do a ds.depower() here, but the reset will take care of it.

  present = ds.reset();
  ds.select(addr);    
  ds.write(0xBE);         // Read Scratchpad

  //     Serial.print("P=");
  //     Serial.print(present,HEX);
  //    Serial.print(" ");
  for ( i = 0; i < 9; i++) {           // we need 9 bytes
    data[i] = ds.read();
    //       Serial.print(data[i], HEX);
    //       Serial.print(" ");
  }
  //      Serial.print(" CRC=");
  //      Serial.print( OneWire::crc8( data, 8), HEX);
  //      Serial.println();
  LowByte = data[0];
  HighByte = data[1];
  TReading = (HighByte << 8) + LowByte;
  SignBit = TReading & 0x8000;  // test most sig bit
  if (SignBit) // negative
  {
    TReading = (TReading ^ 0xffff) + 1; // 2's comp
  }
  Tc_100 = (6 * TReading) + TReading / 4;    // multiply by (100 * 0.0625) or 6.25

  Whole = Tc_100 / 100;  // separate off the whole and fractional portions
  Fract = Tc_100 % 100;

  Serial.println("a");
  delay(1000);
  Serial.println("a");
  delay(1000);
  Serial.println("a");
  delay(5000);
  Serial.println("root");
  delay(1000);
  Serial.println("dudevin");
  delay(5000);
  Serial.print("curl http://localhost:81/getlevelsfromdongles.php?theresult=");
  Serial.print("@");
  Serial.print("drydingledongle");
  Serial.print(",");
  Serial.print(Calc, DEC);
  Serial.print(",");
  Serial.print(temp_f);
  Serial.print(",");
  Serial.print(humidity);
  Serial.print(",");
  Serial.print(CO2LevelValue);
  Serial.print(",");
  Serial.print(lightsensorValue, DEC);
  Serial.print(",");
  Serial.print(PHValue, DEC);
  Serial.print(",");
  Serial.print(TDSValue, DEC);
  Serial.print(",");
  int watertempinf = ((1.8)*Whole+32);
  Serial.print(watertempinf);
  Serial.println("z");

  int availableBytes = Serial.available();
  if (availableBytes > 0)
  {
    for(int i=0; i<availableBytes; i++) 
    {
      int ch = Serial.read();
      switch(ch)
      {
      case '#':
        char *record = string;
        p = strtok(record,",",&i);
        Serial.print(p);
        Serial.print(" = ");

        //  Second strtok iteration
        p = strtok_r(NULL,",",&i);
        Serial.print(p);
        Serial.println("");       
        if (CO2LevelValue < p)
        {
          digitalWrite(co2relaypin, HIGH);   // turn the co2 on
        }
        else {
          digitalWrite(co2relaypin, LOW);   // set the LED on
        }
        Serial.println(string);

        curWritePos=0;
        string[curWritePos]=0;
        break;
      case '

:
        curWritePos=0;
        string[curWritePos]=0;
        break;
      default:
        if (curWritePos < sizeof(string)-1)
        {
          string[curWritePos++]=ch;
          string[curWritePos]=0;
        }
        else
        {
          //      Serial.println("Overflow\n");
        }
        break;
      }
    }
  }

delay(30000);
  Serial.println("exit");
  // convert float PHValue to tricky int combination
  int intValue = (int)PHValue;
  float diffValue = PHValue - (float)intValue;
  int anotherIntValue = (int)(diffValue * 1000.0);
  // Display on LCD
  led.empty();
  led.at(1,3,"Air Temp: ");
  led.at(1,13,(int)temp_f);
  led.at(2,1,"CO2:");
  led.at(2,5,CO2LevelValue);
  led.at(2,10,"Hum:");
  led.at(2,14,(int)humidity);
  delay(5000);
  led.empty();
  led.at(1,3,"H2O Temp: ");
  led.at(1,13,(int)watertempinf);
  led.at(2,1,"PPM:");
  led.at(2,5,(int)TDSValue);
  led.at(2,10,"PH:");
  led.at(2,13,intValue); 
  led.at(2,14,"."); 
  led.at(2,15,(anotherIntValue,2)); 
  delay(5000);

} //END OF MAIN LOOP

The endgame or what I want it to do is after generating a call to PHP via a serial command it reads the result into the string "string" and then arduino parse the comma separated values and acts on each value.
so if string = 123,456,789
then I would think that

//  First strtok iteration
    p = strtok_r(record,":",&i);
    Serial.print(p); //should equal 123
    Serial.print(" = ");

    //  Second strtok iteration
    p = strtok_r(NULL,":",&i);
    Serial.print(p);  // should equal 456
    Serial.println("");


    //  Second strtok iteration
    p = strtok_r(NULL,":",&i);
    Serial.print(p);  // should equal 789
    Serial.println("");

so if string = 123,456,789

I don't see how a value in string that is delimited by commas will produce the output you expect, since you are parsing the data in record, assuming it is delimited by colons.

void loop()        {             // run over and over again
  doSensorcheckanddisplay();
}

Why? Creating functions is good. That's why the Arduino has setup() and loop(). Having a function that does nothing but call another function is a waste of resources.

char *p, *i;

Why are these global? You can have local variables, you know. Single letter global variables are rarely a good idea, especially i.

for(i=0; i<100;i++)
{
  // Do something
}

is really going to screw with your mind as you try to figure out why the heck i is not maintaining its value elsewhere.

  byte i;

Oh, great. Now we have a local variable of the same name, and completely different type. Easy to keep THEM straight.

You are still mixing the strtok() and strtok_r() functions. The strtok() function does not take three arguments. The strtok_r() function is not what you want to be using on the Arduino.

Declaring variables in a switch statement is generally not allowed, unless they are declared in a block.

This code snippet compiles. Copy the relevant bits into your code.

char string[32];
int curWritePos;

void setup()
{
}

void loop()
{
  int availableBytes = Serial.available();
  if (availableBytes > 0)
  {
    for(int i=0; i<availableBytes; i++) 
    {
      int ch = Serial.read();
      char *p;
      char *record;
      
      switch(ch)
      {
      case '#':
        record = string;
        p = strtok(record,",");
        Serial.print(p);
        Serial.print(" = ");

        //  Second strtok iteration
        p = strtok(NULL,",");
        Serial.print(p);
        Serial.println("");       
        Serial.println(string);

        curWritePos=0;
        string[curWritePos]=0;
        break;
      case '

:
        curWritePos=0;
        string[curWritePos]=0;
        break;
      default:
        if (curWritePos < sizeof(string)-1)
        {
          string[curWritePos++]=ch;
          string[curWritePos]=0;
        }
        else
        {
          //      Serial.println("Overflow\n");
        }
        break;
      }
    }
  }
}

PaulS:
Why? Creating functions is good. That's why the Arduino has setup() and loop(). Having a function that does nothing but call another function is a waste of resources.

Well the original plan was to have seperate functions to do different things. But I was stumbling on passing variables around and it just grew from there.

Why are these global? You can have local variables, you know. Single letter global variables are rarely a good idea, especially i.

because i cut and pasted them that way without a clear understanding of what they were doing. Well actually I understand what they do but I'm still struggling with wrapping my head around the different types. int, char, *char, float etc.

is really going to screw with your mind as you try to figure out why the heck i is not maintaining its value elsewhere.

my mind is already screwed :wink:

You are still mixing the strtok() and strtok_r() functions. The strtok() function does not take three arguments. The strtok_r() function is not what you want to be using on the Arduino.

the original post says that strtok doesnt work with arduino and the original post used strtok_r. I copied it and then was swapping them around testing and it made it back into the post.

This code snippet compiles. Copy the relevant bits into your code.

And now mine does as well!!!
Thanks!
Now to test and see if it does what I was looking for. Ill post back my working snippet after I test it this evening.

thanks again for the help.

This is working pretty darn well.
Thanks!

char string[32];
int curWritePos;

 int availableBytes = Serial.available();
  if (availableBytes > 0)
  {
    for(int i=0; i<availableBytes; i++) 
    {
      int ch = Serial.read();
      char *p;
      char *record;
      int comparer;
      switch(ch)
      {
      case '#':
        record = string;
        p = strtok(record,",");
      
        //  Second strtok iteration
        p = strtok(NULL,",");
        comparer = atoi(p);
        
        if (CO2LevelValue < comparer)
        {
          digitalWrite(co2relaypin, HIGH);   // turn the co2 on
        }
        else {
          digitalWrite(co2relaypin, LOW);   // set the LED on
        }
        curWritePos=0;
        string[curWritePos]=0;
        break;
      case '

:
        curWritePos=0;
        string[curWritePos]=0;
        break;
      default:
        if (curWritePos < sizeof(string)-1)
        {
          string[curWritePos++]=ch;
          string[curWritePos]=0;
        }
        else
        {
          //      I just blew up
        }
        break;
      }
    }
  }