Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESP32 specific bug: GetWordValue returns wrong value #3

Open
refob opened this issue Mar 2, 2023 · 5 comments
Open

ESP32 specific bug: GetWordValue returns wrong value #3

refob opened this issue Mar 2, 2023 · 5 comments

Comments

@refob
Copy link

refob commented Mar 2, 2023

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
Copy link

brmarkus commented Dec 4, 2023

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
Copy link

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
Copy link

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

@JoeLoginIsAlreadyTaken
Copy link

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
Copy link
Owner

tgolla commented Apr 1, 2024

@brmarkus @JoeLoginIsAlreadyTaken I will look into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants