Character Counting Support

[tatarize]

Modifies the GenericGcodeDriver to permit character counting.

• sendLine() is broken into formatLine(), sendData() and waitForOK().
• The formatLine() code merely formats the line in the way it was formatted in the sendLine() previously.
• The sendData() function adds functionality to add the sent line length to bufferedSend.
• The sendData() function adds functionality to add the sent line length to the commandLengthQueue.
• The waitForOK() function consists of the original code found in sendLine() function, but also removes the head of the commandLengthQueue and decrements that from the bufferedSend.

This should be testable for the original unaltered functionality of GenericGcodeDriver.java.

---

Phase 2.

The only function that overloads sendLine() is found in Grbl.java and consists of the same functionality as the original code. Except that the text string has the spaces removed: text.replace(" ", "")

In phase 2, this would be replaced with a different sendLine() which would can be permitted to implement character counting.

The formatLine() function will be overloaded to include the modification seen in the GRBL driver.

  protected String formatLine(String text, Object... parameters) {
    return String.format(FORMAT_LOCALE, text.replace(" ", "")+LINEEND(), parameters);
  }

And the sendLine() function will be modified to character count:

  protected void sendLine(String text, Object... parameters) throws IOException
  {
    String line = formatLine(text, parameters);
    int length = line.length();
    while (bufferedSend + length < 128)
    {
      waitForOK();
    }
    sendData(line);
  }

[tatarize]

Phase 2 would also need to instance the commandLengthQueue:

commandLengthQueue = new ArrayDeque<Integer>();

And maybe make the whole thing optional, so GRBL isn't forced into using that, or whatever you'd want there. I'd kinda prefer Phase 2 go to an interested 3rd Party, preferably one with a hardware GRBL device.

[mgmax]

In my opinion, commandLengthQueue should not be inside GenericLaserCutter. It is far too specialized to the GRBL case, and will make little sense for other machines that use different lower layers like USB or different queueing mechanisms. For GRBL, it can easily be added by overriding sendData, roughly:

GRBL:

@Override
sendData(line) {
   while (waiting for OK) {
      sleep();
   }
   commandLengthQueue.add(line.length())
   super.sendData(line);
   commandLengthQueue.pop();
}

[tatarize]

You might be right. Though with it uninitialized it would never really kick in. I was mostly focused on making sure it broke nothing, but making phase 2 of that easier. The breaking of that code into smaller bits would help with the polymorphism. Though I guess it could be all rewritten in the GRBL driver directly. I was trying for the subtlest changes to achieve the end. Since the problem with the last offer of that methodology was that was a bit of a hammer.

And it's not really waiting for OK, but rather waiting for enough okays that the current command could be fed into the planner. It's mostly keeping a record of each command sent and the count of the OKs and how much each OK frees from the planner. It's not a very hard thing to implement. Though even when I tell Visicut that my GRBL device is my own program I can't use the GRBL driver since it broke the internet support and still has no port property.

[t-oster]

@tatarize I am sorry this is moving so slowly. I just don't have the hardware and resources to review this properly. But if you update the code in a way, that it is obvious that the current behavior is unchanged if this is not explicitely enabled, I would be ok with merging it even if I do not understand every aspect.

[tatarize]

You're right it needs to be checked. The current behavior should be unchanged if isWaitForOKafterEachLine is set.

But, keep in mind this isn't fully implemented it's just tweaked and broken to allow another phase of tweaking things. There's a certain amount of mental load required to sort through the various ins-and-outs of this. And I think the raised issue requesting this was a while back #74 some five years ago. So working through the entire code thing for something that actually does nothing might be a tall order, and it touches on code that's critical for many drivers.

mg's suggestion isn't a terrible one. Implement this entirely within the grbl driver and don't touch the other base driver at all. At least then it can be clear that it would only break one driver. When people touch the more base functionalities it means more thinking and more worry and more risk within a mature codebase. So you need to consider all the facets and retest not just this but potentially anything derived from GenericGcodeDriver, implementing this directly and completely in GRBL means you can just rubber-stamp that because at worst it would affect one driver. It might even be reasonable to want this done as a completely different driver, just make a second copy of GRBL with character-counting. That way it can break 0 drivers, and give a platform for directly testing the ultimate benefits. It might be less elegant but it would make your job much easier because a pr that can break 0 things, is much easier than one that requires you think really hard about the entire codebase.