tgolla / GCodeParser

The GCode parser is designed to parse GCode in order that it can then be processed.
https://terencegolla.com/
MIT License
18 stars 6 forks source link

ESP32 specific bug: GetWordValue returns wrong value #3

Open refob opened 1 year ago

refob commented 1 year ago

Bug_ESP32.zip Bug_ESP32_fixed.zip

I found an issue with GetWordValue when I ran the provide example on an ESP32 cpu: GetWordValue returns 288 for the input "G0X120". For "G00X120" I get the correct value of 0.

Running the same code on an Arduino I always got the correct value. I fixed this by using:

if ((line[pointer + 1] == '0') && (line[pointer + 2] == 'X'))
  return 0.0;
else
  return (double)strtod(&line[pointer + 1], NULL);

instead of:

return (double)strtod(&line[pointer + 1], NULL);

Please run the code in Bug_ESP32.zip to see this output on an ESP32:

Ready

Command Line: G0X120Z-10F30 Comment(s): Process G code: 288

Command Line: G0X1 Comment(s): Process G code: 1

Command Line: G0X12 Comment(s): Process G code: 18

Command Line: G0X120 Comment(s): Process G code: 288

Command Line: G00X120 Comment(s): Process G code: 0

brmarkus commented 10 months ago

I saw the same - this is an issue to strtod() - there are lots of discussions about flouting-point numbers and their exact representation; "0" seems such a "difficult" case.

For me it helped to just remove/comment-out the part removing spaces and tabs:

      //// Spaces and tabs are allowed anywhere on a line of code and do not change the meaning of 
      //// the line, except inside comments. Remove spaces and tabs except in comments. 
      //if (c == ' ' || c == '\t')
      //{
      //  int removeCharacterPointer = pointer;
      //
      //  while (line[removeCharacterPointer] != '\0')
      //  {
      //    line[removeCharacterPointer] = line[removeCharacterPointer + 1];
      //
      //    removeCharacterPointer++;
      //  }
      //
      //  correctCommentsPointerBy++;
      //}
      //else

This relaxed the parsing and conversion with strtod() - and all returned values from GetWordValue() were correct again.

EDIT: In my case the command look like G0 X120 (a space inbetween), which gets removed by the parser.

JoeLoginIsAlreadyTaken commented 6 months ago

I have the same Issue on a RP2040 when parsing this String "G0 Z0 X300 Y100". GetWordValue of Z returned 768, which is in Hexadecimal 0x300 ;-) I think strtod is using "as much as it can" from the string to get a value, and "0x" starts a HEX value.

brmarkus commented 6 months ago

@JoeLoginIsAlreadyTaken have you found a solution? Did it help to comment-out the section as mentioned above?

JoeLoginIsAlreadyTaken commented 6 months ago

Yes @brmarkus, keeping the spaces had helped. The space i a char where strtod stops.

Here is an other solution.

I added a new function to search from the start-pointer to the next char from "wordLetter" which are the valid gcode letters.

/// <summary>
/// Searches for the end of a word-value by checking for char from wordLetter
/// </summary>
/// <param name="pointer">The position where to start the search.</param>
/// <returns>A pointer to where the word ends.</returns>
int GCodeParser::FindWordEnd( int pointer)
{
    while (line[pointer] != '\0')
    {
        bool isStopChar = false;
        for (int i = 0; wordLetter[i] != '\0'; ++i) {
            if (line[pointer] == wordLetter[i]) {
                isStopChar = true;
                break;
            }
        }       
        if (isStopChar) {
            return pointer;
        }
        pointer++;
    }
    return pointer;
}

Then i changed "GetWordValue" to use this.

/// <summary>
/// Gets the value following the word.
/// </summary>
/// <param name="letter">The letter of the word to look for in the line.</param>
/// <returns>The value following the letter for the word.</returns>
/// <remarks>
/// Currently the parser is not sophisticated enough to deal with parameters, 
/// Boolean operators, expressions, binary operators, functions and repeated items.
/// </remarks>
double GCodeParser::GetWordValue(char letter)
{
    int pointer = FindWord(letter);
    int wordEnd = FindWordEnd(pointer+1);

    if (line[pointer] != '\0' && pointer < wordEnd)
    {
        char temp = line[wordEnd]; // store the original char 
        line[wordEnd] = '\0'; // set a null-Terminator 
        double value = strtod(&line[pointer + 1], nullptr); // convert value
        line[wordEnd] = temp; // reset original char
        return value;
    }

    return 0.0;
}

It is also needed to add the prototype of the new function to the header file.

So far this works for me, but im new to gcode processing at all - so be careful. @tgolla could you please have a look if this solution is ok.

tgolla commented 6 months ago

@brmarkus @JoeLoginIsAlreadyTaken I will look into this.