Problem with parsenumber

Status
Not open for further replies.
S

Swacy

New member
Hello,
please excuse my bad english.

I have a code with which I query and evaluate a serial input.
It works great with the Arduino, but the teensy hangs in the while loop. Why is that?
What do I have to change to use it with the teensy?

Code: 
//G-code parser begin

float parsenumber(char code, float val) {
  char *ptr = buffer;
  while (ptr && *ptr && ptr < buffer + sofar) {
    if (*ptr == code) {
      return atof(ptr + 1);
    }
    ptr = strchr(ptr, ' ') + 1;   // [B]<-- here he hangs up[/B]
  }
  return val;

}

//G-code parser end

Hope you can help me.
Best regards
 
I am not really sure how this is supposed to work... But My guess is there could be something wrong...

That is you did not include a full sketch that demonstrates this so, we have no clue of where buffer is defined or sofar or???

But lets assume you call this like: parsenumber('\n', 0.1f)
And the buffer contained: 0.5\n

You start off set ptr to point to the leading 0 character.
The reasons to abort the while loop is not met.
The *ptr != code so that part is not done.

You then do: ptr = strchr(ptr, ' ') + 1
You don't have a blank so strchr return NULL (0) but then you increment it So now ptr=1
You then go back to test ptr is not NULL and then you try to deference ptr=1 which will probably fault.
 
Thank you very much.

The code should execute the switch / case code via the serial input.
In principle like a 3D printer, only simplified.

Here is the short code:
Code: 
#include "AccelStepper.h"

// Define the Pins used
#define EN_PIN           4 // Enable
#define DIR_PIN          6 // Direction
#define STEP_PIN         5 // Step

#define home_switch 9 // Pin 9 connected to Home Switch (MicroSwitch)

#define MAX_BUF        (64)
#define DEFAULTBAUDRATE 115200

constexpr uint32_t steps_per_mm = 400;

char buffer[MAX_BUF];
int sofar;

// AccelStepper Setup
AccelStepper stepperZ = AccelStepper(1, STEP_PIN, DIR_PIN);   // 1 = Easy Driver interface

//G-code parser begin

float parsenumber(char code, float val) {
  char *ptr = buffer;
  while (ptr && *ptr && ptr < buffer + sofar) {
    if (*ptr == code) {
      return atof(ptr + 1);
    }
    ptr = strchr(ptr, ' ') + 1;
  }
  return val;

}

//G-code parser end

//G-code execution begin
void processCommand() {
  int cmd = parsenumber('G', -1);
  switch (cmd) {

    case  1: //G01 Zxxx Fxxx processing (Z-axis movement)
      stepperZ.setMaxSpeed(steps_per_mm * parsenumber('F', 0) / 60);
      stepperZ.move(steps_per_mm * parsenumber('Z', 0));
      stepperZ.run();
      Serial.write("Z_move_comp\n");
      break;

    case  2: //G02 Zxxx Fxxx Absolute
      break;

    case  4: //G04 Pxxx processing (pause P milliseconds)
      break;

    case 28: //G28 Home Z
      break;

    default:  break;
  }

  cmd = parsenumber('M',-1);
  switch(cmd) {

    case  7: //M07 Sxxx Fan Speed
      break;

    default:  break;
  }
  
}

void setup() {
  Serial.begin(DEFAULTBAUDRATE);
  sofar = 0;

}

void loop() {
    if ( stepperZ.distanceToGo() == 0  ) {
    stepperZ.enableOutputs();
    if (Serial.available() > 0) {
      char c = Serial.read();
      if (sofar < MAX_BUF - 1) buffer[sofar++] = c;
      if (c == '\n') {
        buffer[sofar] = 0;
        processCommand();
        Serial.print("ok\n");
        sofar = 0;
      }
    }
  }
  stepperZ.run();
}


If I now enter G1 Z10 F200, it hangs in the "parsenumber" function.
It works flawlessly via an Arduino Nano / Uno / Mega.
 
Sometimes on different processors you get lucky and bug is not fatal...

Here is a version of your program with lots of debug outputs and does not require anything special hardware or library wise to reproduce the boom...

Code: 
#define MAX_BUF        (64)
#define DEFAULTBAUDRATE 115200

constexpr uint32_t steps_per_mm = 400;

char buffer[MAX_BUF];
int sofar;

float parsenumber(char code, float val) {
  Serial.printf("Parse Number called %s(%c)\n", buffer, code); 
  char *ptr = buffer;
  while (ptr && *ptr && ptr < buffer + sofar) {
    Serial.printf("    ptr: %x(%c)\n", (uint32_t)ptr, *ptr); Serial.flush(); // make it out before boom
    if (*ptr == code) {
      return atof(ptr + 1);
    }
    ptr = strchr(ptr, ' ') + 1;
    Serial.printf("    new ptr: %x\n", (uint32_t)ptr); Serial.flush(); // make it out before boom
  }
  return val;
}

//G-code parser end

//G-code execution begin
void processCommand() {
  int cmd = parsenumber('G', -1);
  float f, z;
  Serial.printf("CMD: %d\n", cmd); Serial.flush();
  switch (cmd) {

    case  1: //G01 Zxxx Fxxx processing (Z-axis movement)

      f = parsenumber('F', 0);
      Serial.print("F: "); Serial.println(f, 2); Serial.flush();
      f = parsenumber('Z', 0);
      Serial.print("Z: "); Serial.println(f, 2); Serial.flush();
      break;

    case  2: //G02 Zxxx Fxxx Absolute
      break;

    case  4: //G04 Pxxx processing (pause P milliseconds)
      break;

    case 28: //G28 Home Z
      break;

    default:  break;
  }

  cmd = parsenumber('M', -1);
  Serial.printf("M: %d\n", cmd);
}

void setup() {
  while (!Serial && millis() < 5000) ;
  Serial.begin(DEFAULTBAUDRATE);
  sofar = 0;
  Serial.println("\n\n *** Test strchr ***"); 
    
}

void loop() {
  if (Serial.available() > 0) {
    char c = Serial.read();
    if (sofar < MAX_BUF - 1) buffer[sofar++] = c;
    if (c == '\n') {
      buffer[sofar] = 0;
      processCommand();
      Serial.print("ok\n");
      sofar = 0;
    }
  }
}
Which verified what I suspected: Here is the output...
Code: 
 *** Test strchr ***
Parse Number called G1 Z10 F200
(G)
    ptr: 20000f1c(G)
CMD: 1
Parse Number called G1 Z10 F200
(F)
    ptr: 20000f1c(G)
    new ptr: 20000f1f
    ptr: 20000f1f(Z)
    new ptr: 20000f23
    ptr: 20000f23(F)
F: 200.00
Parse Number called G1 Z10 F200
(Z)
    ptr: 20000f1c(G)
    new ptr: 20000f1f
    ptr: 20000f1f(Z)
Z: 10.00
Parse Number called G1 Z10 F200
(M)
    ptr: 20000f1c(G)
    new ptr: 20000f1f
    ptr: 20000f1f(Z)
    new ptr: 20000f23
    ptr: 20000f23(F)
    new ptr: 1
Notice the last new ptr = 1...

Simple fix, can do multiple ways, but maybe just change the strchr code to:
Code: 
   ptr = strchr(ptr, ' ');
    if (ptr) ptr++; // don't increment null pointer...
With that change my example worked
 
KurtE said:
You then do: ptr = strchr(ptr, ' ') + 1
You don't have a blank so strchr return NULL (0) but then you increment it So now ptr=1
You then go back to test ptr is not NULL and then you try to deference ptr=1 which will probably fault.
Click to expand...

As Kurt noted, you get a bad ptr value if there is no ' '. To keep from faulting on bad ptr, replace ptr = strchr(ptr, ' ') + 1; with
Code: 
    ptr = strchr(ptr, ' ');
    if (ptr  == NULL ) {
      Serial.println("trouble");
      return val;
    } else  ptr += 1;
The fact that it "works"on UNO etc. is cold comfort. Your coding is bad.
 
Status
Not open for further replies.
Back
Top