Adjust parsing of HPGL.

[just-jason]

Currently, the HPGL parser is only looking for ';' as a terminator. The formatting of HPGL allows for sending larger blocks of co-ordinates without using a terminator after each co-ordinate pair eg
PD3112,3513,3825,3566,4558,3718,4657,3698,4758,3667,4846,3605,4970,3460,5046,3373,5146,3372,6937,4569,9296,6216,9208,6409,9089,6585,8949,6744,8781,6876,8592,6981,8394,7056,8183,7100,7971,7109,7760,7087,7544,7030,7412,6937,6162,6105,2778,3949,2720,3905,2659,3764,2386,3372,1457,1818,1420,1728,1353,1511,1330,1400,1316,1303,1318,1159,1348,1047,1398,949,1479,863,1566,796,1691,749,1795,728,1913,723,2126,731,2346,775,2935,965,3134,1057,7074,3187,9609,4323,10273,4552,13583,5133,14015,5436,14406,5784,11484,6519,11871,6867,11554,6502;

This means that it is not difficult to over fill the parsing buffer while it waits for a terminator char.

The same code could be written like this

PD3112,3513;
PD3825,3566;
PD4558,3718;
PD4657,3698;
PD4758,3667:
...
...

and it should work better for the buffer, but is not very efficient ( nor very common from most HPGL generators)

Maybe something like this pseudo code to achieve a similar result on the Pico side,

switch (c) {
      case COMMA:
        if (c == COMMA) { 
        // Handle the comma character
        // is second comma - we have a co-ordinate pair - send it
          if (commaCounter == 1){
          process_command();
          }else {
            commaCounter++ ;           
          }
        }
        break;
      case S_COLON:
        // Handle the semicolon character
        if (c == S_COLON) {
          // Add we have reached the end of the command - send it
          process_command();

          commaCounter = 0; 
        }
        break;

What I hope is that this may make using flow control easier as the buffer will not wait till it has overflowed, before processing the commands, but rather it will process valid command pairs as they arrive.

[terjeio]

I am not sure I follow you - commands are processed as they arrive and handshaking should not rely on the content of the data stream. I tinkered a bit with using XON/XOFF for handshaking but at least Windows does not seem to have XON/XOFF handling implemented at the driver level so I gave up on that. Hardware handshaking (or USB CDC flow control) should be implemented using exactly the same logic - but by either handling the hardware pin or the USB CDC flow control instead of sending XON/XOFF. The protocol i simple, just high- and low watermarks checked when inserting/removing data from the buffer to determine when to signal stop/start sending to the host.

[just-jason]

I am not sure if i have understood this correctly, but it looks to me like the HPGL code is only processed when a ; char is recieved.
https://github.com/grblHAL/Templates/blob/1907a0cc5ef9f8ef7e7bc70837a6dabbe5d89518/my_plugin/hpgl/hpgl.c#L122

ie , this would be seen as one single HPGL command by the parser
PD3112,3513,3825,3566,4558,3718,4657,3698,4758,3667,4846,3605,4970,3460,5046,3373,5146,3372,6937,4569,9296,6216,9208,6409,9089,6585,8949,6744,8781,6876,8592,6981,8394,7056,8183,7100,7971,7109,7760,7087,7544,7030,7412,6937,6162,6105,2778,3949,2720,3905,2659,3764,2386,3372,1457,1818,1420,1728,1353,1511,1330,1400,1316,1303,1318,1159,1348,1047,1398,949,1479,863,1566,796,1691,749,1795,728,1913,723,2126,731,2346,775,2935,965,3134,1057,7074,3187,9609,4323,10273,4552,13583,5133,14015,5436,14406,5784,11484,6519,11871,6867,11554,6502;

just as would this

PD3112,3513;

The first is "one" HPGL command , but each co-ordinate pair is really a separate move command.

The problem is , the buffer can easily become full before it receives a ; command that would trigger the parser to see this as a complete command and process that command - as well as clearing the buffer.

You could imagine a similar situation in sending G Code , if there was no /n or /r after each XY move , then the commands would be pushed into the buffer and never processed while the parser waits for a /n or /r and eventually this would cause buffer overflow. In the case of the current HPGL parser, it is waiting for ;
If we are able to have the buffer constantly cleared , then the USB CDC flow control should be able to work.

[terjeio]

it looks to me like the HPGL code is only processed when a ; char is recieved.

No, the active command is repeated when enough parameter values are received. IIRC the ; char is used to terminate the active command. It is easy enough to verify by entering your example above from a terminal by writing the values manually.

Gcode is different as there has to be a line terminator to delimit input into blocks (or lines) of code - and there is a well defined limit to what is allowed within a block.

[just-jason]

Thanks for clearing that up. I had miss read the way the parser was processing the commands.
Can I please ask, I am still somewhat confused , as this was the only explination I could think of as to why the the USB CDC flow control should not be working.
I tested the PIco with a very simple bit of code like this

 Serial.begin(115200);
}
void loop() {
 if (Serial.available()){
  Serial.write(Serial.read());
  delay(20); // Allow some characters in
  }
}

My logic was - if flow control was not working , then the serial buffer would overflow and data would be lost. This does not happen , and the Pico faithfully ( slowly) echos back a large file that is sent to the USB serial port. The data flow is paused by the Pico when the buffer is full and started again when the data is written out of the buffer making space.

If the same test is done on another board without USB CDC , eg the ESP8266, the data is lost.
To me , this would mean that the USB CDC on the Pico works correctly and is able to control the flow of data from the PC. If I try and send that same file to the Pico when running GRBLhal, the process hangs after a few seconds and will not send any more data, even after waiting many minutes.

It looked to me as though the data in the buffer in GRBLhal was not being cleared and so the USB CDC will not allow any more data in, but now that I know that the data is being processed, I am now not sure what is going on anymore.

[terjeio]

Hmm, it seems that no more characters than can fit in the input buffer is read at a time meaning that the overflow flag will never be set. Perhaps there is a call that has to be made to the USB CDC driver to enable handshaking?

Your example above uses the Arduino framework - grblHAL does not. Perhaps clues for how to implement handshaking can be found in the Arduino framework?

I have no explanation for why grblHAL hangs - perhaps something in the USB CDC driver that blocks grblHAL foreground processing?

[just-jason]

It is very stange indead.

In an attempt to degug, I added an echo call ( hal.stream.write(c) ) to this function
The result was this

It seems to hang after the first draw move.
The same file was succesfully sent using a python script with buffer querry.

I am quite confident that the board and basic firmware are fuctional.

One point that I have noticed is that when looking at the sent data counter from the screen captures here
it looks like a block of +/- 4k is first sent to the Pico, and then the CDC stops the flow and waits for more space to become avalible. I am not sure what is going on here.