Moved the responsibility for parsing the command response from the co…

…mmunicator to the command (#2190) The commands will now parse the response and figure out if the command has been completed or not. This is especially important for multiline response commands. This also makes the communicators generic as it no longer contain protocol specific implementation.
winder · Apr 1, 2023 · 223ab37 · 223ab37
1 parent 2219913
commit 223ab37
Show file tree

Hide file tree

Showing 35 changed files with 653 additions and 362 deletions.
diff --git a/ugs-core/src/com/willwinder/universalgcodesender/AbstractController.java b/ugs-core/src/com/willwinder/universalgcodesender/AbstractController.java
@@ -64,7 +64,7 @@ public abstract class AbstractController implements ICommunicatorListener, ICont
 
     // These abstract objects are initialized in concrete class.
     protected final ICommunicator comm;
-    protected MessageService messageService;
+    protected MessageService messageService = new MessageService();
 
     // Added value
     private final AtomicBoolean isStreaming = new AtomicBoolean(false);
@@ -752,7 +752,6 @@ public void commandComplete(String response) throws UnexpectedCommand {
         }
 
         GcodeCommand command = activeCommands.pollFirst();
-        updateCommandFromResponse(command, response);
         updateParserModalState(command);
 
         numCommandsCompleted.incrementAndGet();
@@ -765,8 +764,6 @@ public void commandComplete(String response) throws UnexpectedCommand {
         checkStreamFinished();
     }
 
-    protected abstract void updateCommandFromResponse(GcodeCommand command, String response);
-
     @Override
     public void rawResponseListener(String response) {
         rawResponseHandler(response);

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/GrblController.java b/ugs-core/src/com/willwinder/universalgcodesender/GrblController.java
@@ -21,8 +21,8 @@ This file is part of Universal Gcode Sender (UGS).
 import com.willwinder.universalgcodesender.communicator.GrblCommunicator;
 import com.willwinder.universalgcodesender.communicator.ICommunicator;
 import com.willwinder.universalgcodesender.firmware.IFirmwareSettings;
+import com.willwinder.universalgcodesender.firmware.grbl.GrblCommandCreator;
 import com.willwinder.universalgcodesender.firmware.grbl.GrblFirmwareSettings;
-import com.willwinder.universalgcodesender.gcode.DefaultCommandCreator;
 import com.willwinder.universalgcodesender.gcode.util.GcodeUtils;
 import com.willwinder.universalgcodesender.i18n.Localization;
 import com.willwinder.universalgcodesender.listeners.ControllerState;
@@ -79,7 +79,7 @@ public class GrblController extends AbstractController {
     private boolean temporaryCheckSingleStepMode = false;
 
     public GrblController(ICommunicator comm) {
-        super(comm, new DefaultCommandCreator());
+        super(comm, new GrblCommandCreator());
         this.positionPollTimer = new StatusPollTimer(this);
 
         // Add our controller settings manager
@@ -219,7 +219,7 @@ else if (GrblUtils.isGrblVersionString(response)) {
                 Logger.getLogger(GrblController.class.getName()).log(Level.CONFIG,
                         "{0} = {1}", new Object[]{Localization.getString("controller.log.realtime"), this.capabilities.hasCapability(GrblCapabilitiesConstants.REAL_TIME)});
             }
-            
+
             else if (GrblUtils.isGrblProbeMessage(response)) {
                 Position p = GrblUtils.parseProbePosition(response, getFirmwareSettings().getReportingUnits());
                 if (p != null) {
@@ -691,9 +691,4 @@ public int getStatusUpdateRate() {
     public void setStatusUpdateRate(int rate) {
         positionPollTimer.setUpdateInterval(rate);
     }
-
-    @Override
-    protected void updateCommandFromResponse(GcodeCommand command, String response) {
-        GrblUtils.updateGcodeCommandFromResponse(command, response);
-    }
 }
diff --git a/ugs-core/src/com/willwinder/universalgcodesender/SmoothieController.java b/ugs-core/src/com/willwinder/universalgcodesender/SmoothieController.java
@@ -45,14 +45,12 @@ public class SmoothieController extends AbstractController {
 
     private final Capabilities capabilities;
     private final IFirmwareSettings firmwareSettings;
+    private final StatusPollTimer statusPollTimer;
     private String firmwareVersion = "Unknown";
     private ControllerStatus controllerStatus;
-
     private boolean isSmoothieReady = false;
     private boolean isReady;
 
-    private StatusPollTimer statusPollTimer;
-
     public SmoothieController() {
         this(new SmoothieCommunicator());
     }
@@ -286,9 +284,4 @@ protected void setControllerState(ControllerState controllerState) {
                 .build();
         dispatchStatusString(controllerStatus);
     }
-
-    @Override
-    protected void updateCommandFromResponse(GcodeCommand command, String response) {
-        GrblUtils.updateGcodeCommandFromResponse(command, response);
-    }
 }
diff --git a/ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java b/ugs-core/src/com/willwinder/universalgcodesender/TinyGController.java
@@ -24,7 +24,7 @@ This file is part of Universal Gcode Sender (UGS).
 import com.willwinder.universalgcodesender.firmware.IFirmwareSettings;
 import com.willwinder.universalgcodesender.firmware.tinyg.TinyGFirmwareSettings;
 import com.willwinder.universalgcodesender.gcode.ICommandCreator;
-import com.willwinder.universalgcodesender.gcode.TinyGGcodeCommandCreator;
+import com.willwinder.universalgcodesender.firmware.tinyg.TinyGGcodeCommandCreator;
 import com.willwinder.universalgcodesender.gcode.util.GcodeUtils;
 import com.willwinder.universalgcodesender.i18n.Localization;
 import com.willwinder.universalgcodesender.listeners.ControllerState;
@@ -33,7 +33,7 @@ This file is part of Universal Gcode Sender (UGS).
 import com.willwinder.universalgcodesender.listeners.MessageType;
 import com.willwinder.universalgcodesender.model.*;
 import com.willwinder.universalgcodesender.types.GcodeCommand;
-import com.willwinder.universalgcodesender.types.TinyGGcodeCommand;
+import com.willwinder.universalgcodesender.firmware.tinyg.commands.TinyGGcodeCommand;
 import com.willwinder.universalgcodesender.utils.ControllerUtils;
 
 import java.util.List;
@@ -431,19 +431,4 @@ public CommunicatorState getCommunicatorState() {
     protected CommunicatorState getControlState(ControllerState controllerState) {
         return ControllerUtils.getCommunicatorState(controllerState, this, comm);
     }
-
-    @Override
-    protected void updateCommandFromResponse(GcodeCommand command, String response) {
-        JsonObject jo = TinyGUtils.jsonToObject(response);
-        command.setResponse(response);
-        if (TinyGUtils.isErrorResponse(jo)) {
-            command.setOk(false);
-            command.setError(true);
-        } else {
-            command.setOk(true);
-            command.setError(false);
-        }
-
-        command.setDone(true);
-    }
 }
diff --git a/ugs-core/src/com/willwinder/universalgcodesender/TinyGUtils.java b/ugs-core/src/com/willwinder/universalgcodesender/TinyGUtils.java
@@ -95,7 +95,7 @@ public class TinyGUtils {
      */
     private static final Pattern NUMBER_REGEX = Pattern.compile("^[-]?[\\d]+(\\.\\d+)?");
 
-    private static JsonParser parser = new JsonParser();
+    private static final JsonParser parser = new JsonParser();
 
     public static JsonObject jsonToObject(String response) {
         return parser.parse(response).getAsJsonObject();

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/communicator/BufferedCommunicator.java b/ugs-core/src/com/willwinder/universalgcodesender/communicator/BufferedCommunicator.java
@@ -242,38 +242,27 @@ public void cancelSend() {
      * @param command The command being sent.
      */
     abstract protected void sendingCommand(String command);
-
-    /**
-     * Returns whether or not a command has been completed based on a response
-     * from the controller.
-     * @param response
-     * @return true if a command has completed.
-     */
-    abstract protected boolean processedCommand(String response);
 
-    /**
-     * Returns whether or not a completed command had an error based on a
-     * response from the controller.
-     * @param response
-     * @return true if a command has completed.
-     */
-    abstract protected boolean processedCommandIsError(String response);
-
     /** 
-     * Processes message from GRBL. This should only be called from the
+     * Processes message from the controller. This should only be called from the
      * connection object.
-     * @param response
+     * @param response the raw response line text
      */
     @Override
     public void handleResponseMessage(String response) {
-        // Send this information back up to the Controller.
+        if (!activeCommandList.isEmpty()) {
+            handleResponseForActiveCommand(response);
+        }
         getEventDispatcher().rawResponseListener(response);
+    }
 
+    private void handleResponseForActiveCommand(String response) {
+        GcodeCommand activeCommand = activeCommandList.getFirst();
+        activeCommand.appendResponse(response);
 
         // Pause if there was an error and if there are more commands queued
-        if (processedCommandIsError(response) &&
-                (nextCommand != null                    // No cached command
-                    || (activeCommandList.size() > 1)   // No more commands (except for the one being popped further down)
+        if (activeCommand.isError() &&
+                (activeCommandList.size() > 1   // No more commands (except for the one being popped further down)
                     || (commandStream != null && commandStream.getNumRowsRemaining() > 0) // No more rows in stream
                     || (commandBuffer != null && commandBuffer.size() > 0))) { // No commands in buffer
 
@@ -282,14 +271,14 @@ public void handleResponseMessage(String response) {
         }
 
         // Keep the data flow going in case of an "ok" or an "error".
-        if (processedCommand(response)) {
+        if (activeCommand.isDone()) {
             // Pop the front of the active list.
-            if (this.activeCommandList != null && this.activeCommandList.size() > 0) {
-                GcodeCommand command = this.activeCommandList.pop();
-                this.sentBufferSize -= (command.getCommandString().length() + 1);
+            if (areActiveCommands()) {
+                GcodeCommand command = activeCommandList.pop();
+                sentBufferSize -= (command.getCommandString().length() + 1);
 
                 if (!isPaused()) {
-                    this.streamCommands();
+                    streamCommands();
                 }
             }
         }

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/communicator/GrblCommunicator.java b/ugs-core/src/com/willwinder/universalgcodesender/communicator/GrblCommunicator.java
@@ -19,7 +19,6 @@ This file is part of Universal Gcode Sender (UGS).
 package com.willwinder.universalgcodesender.communicator;
 
 import com.willwinder.universalgcodesender.GrblUtils;
-import com.willwinder.universalgcodesender.communicator.BufferedCommunicator;
 import com.willwinder.universalgcodesender.communicator.event.ICommunicatorEventDispatcher;
 import com.willwinder.universalgcodesender.connection.Connection;
 import com.willwinder.universalgcodesender.types.GcodeCommand;
@@ -52,16 +51,6 @@ public int getBufferSize() {
         return GrblUtils.GRBL_RX_BUFFER_SIZE;
     }
 
-    @Override
-    protected boolean processedCommand(String response) {
-        return GrblUtils.isOkErrorAlarmResponse(response);
-    }
-
-    @Override
-    protected boolean processedCommandIsError(String response) {
-        return response.startsWith("error");
-    }
-
     /**
      * When a command is sent, check if it is one of the special commands which writes to the EEPROM.
      * If it is temporarily setSingleStepMode(true) to avoid corruption.

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/communicator/ICommunicator.java b/ugs-core/src/com/willwinder/universalgcodesender/communicator/ICommunicator.java
@@ -29,6 +29,10 @@ This file is part of Universal Gcode Sender (UGS).
 /**
  * An interface for describing a communicator, responsible for handling gcode command
  * queues and its streaming to a hardware connection.
+ * <p>
+ * To make ensure the performance of the stream, the events dispatched from this service should
+ * be sent using a separate queue or else any slow UI operations may starve the command
+ * stream to the service.
  *
  * @author Joacim Breiler
  */
@@ -70,7 +74,7 @@ public interface ICommunicator extends IConnectionListener {
 
     /**
      * Returns if there is any active commands that has been sent or is being processed
-     * by the hardware. These includes streams or single queued commands.
+     * by the hardware. These include streams or single queued commands.
      *
      * @return true if there is active commands being processed
      */

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/communicator/SmoothieCommunicator.java b/ugs-core/src/com/willwinder/universalgcodesender/communicator/SmoothieCommunicator.java
@@ -41,17 +41,6 @@ protected void sendingCommand(String command) {
 
     }
 
-    @Override
-    protected boolean processedCommand(String response) {
-        return SmoothieUtils.isOkErrorAlarmResponse(response) || SmoothieUtils.isVersionResponse(response);
-    }
-
-    @Override
-    protected boolean processedCommandIsError(String response) {
-        return GrblUtils.isErrorResponse(response);
-    }
-
-
     @Override
     public void setSingleStepMode(boolean enable) {
         // Never mind this

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/communicator/TinyGCommunicator.java b/ugs-core/src/com/willwinder/universalgcodesender/communicator/TinyGCommunicator.java
@@ -18,31 +18,20 @@ This file is part of Universal Gcode Sender (UGS).
  */
 package com.willwinder.universalgcodesender.communicator;
 
-import com.willwinder.universalgcodesender.types.TinyGGcodeCommand;
-
 /**
  * TinyG serial port interface class.
  *
  * @author wwinder
  */
 public class TinyGCommunicator extends BufferedCommunicator {
 
-    @Override
-    public int getBufferSize() {
-        return 254;
+    public TinyGCommunicator() {
+        setSingleStepMode(true);
     }
 
     @Override
-    protected boolean processedCommand(String response) {
-        return TinyGGcodeCommand.isOkErrorResponse(response);
-    }
-
-    /**
-     * Allows detecting errors and pausing the stream.
-     */
-    @Override
-    protected boolean processedCommandIsError(String response) {
-        return false;
+    public int getBufferSize() {
+        return 254;
     }
 
     @Override

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/communicator/XLCDCommunicator.java b/ugs-core/src/com/willwinder/universalgcodesender/communicator/XLCDCommunicator.java
@@ -18,8 +18,6 @@ This file is part of Universal Gcode Sender (UGS).
  */
 package com.willwinder.universalgcodesender.communicator;
 
-import com.willwinder.universalgcodesender.GrblUtils;
-
 /**
  * @author wwinder
  */
@@ -34,11 +32,11 @@ protected void sendingCommand(String command) {
     }
 
     @Override
-    protected boolean processedCommand(String response) {
-        if (UGSCommandCount > 0 && GrblUtils.isOkErrorAlarmResponse(response)) {
+    public void handleResponseMessage(String response) {
+        // Only append responses if there are an active command
+        if (UGSCommandCount > 0) {
             UGSCommandCount--;
-            return true;
+            super.handleResponseMessage(response);
         }
-        return false;
-    }    
+    }
 }
diff --git a/ugs-core/src/com/willwinder/universalgcodesender/connection/ResponseMessageHandler.java b/ugs-core/src/com/willwinder/universalgcodesender/connection/ResponseMessageHandler.java
@@ -22,6 +22,8 @@ This file is part of Universal Gcode Sender (UGS).
 
 import java.util.HashSet;
 import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Handles response messages from the serial connection buffering the data
@@ -33,6 +35,8 @@ This file is part of Universal Gcode Sender (UGS).
  */
 public class ResponseMessageHandler implements IResponseMessageHandler {
 
+    private final static Logger LOGGER = Logger.getLogger(ResponseMessageHandler.class.getSimpleName());
+
     private final StringBuilder inputBuffer;
     private final Set<IConnectionListener> listeners = new HashSet<>();
 
@@ -68,7 +72,14 @@ public void handleResponse(byte[] buffer, int offset, int length) {
     }
 
     public void notifyListeners(String message) {
-        listeners.forEach(listener -> listener.handleResponseMessage(message));
+        listeners.forEach(listener -> {
+            try {
+                listener.handleResponseMessage(message);
+            } catch (Exception e) {
+                LOGGER.log(Level.SEVERE, "The response message could not be handled: \"" + message + "\", unsafe to proceed, shutting down connection.", e);
+                throw e;
+            }
+        });
     }
 
     public void addListener(IConnectionListener connectionListener) {

diff --git a/ugs-core/src/com/willwinder/universalgcodesender/firmware/fluidnc/FluidNCController.java b/ugs-core/src/com/willwinder/universalgcodesender/firmware/fluidnc/FluidNCController.java
@@ -708,7 +708,6 @@ public ControllerStatus getControllerStatus() {
     public void rawResponseListener(String response) {
         if (GrblUtils.isGrblStatusString(response)) {
             getActiveCommand().filter(command -> command instanceof GetStatusCommand || command.getCommandString().contains("?")).ifPresent(command -> {
-                command.appendResponse(response);
                 activeCommands.removeFirst();
                 listeners.forEach(l -> l.commandComplete(command));
 
@@ -731,8 +730,6 @@ public void rawResponseListener(String response) {
             messageService.dispatchMessage(MessageType.VERBOSE, response + "\n");
         } else if (getActiveCommand().isPresent()) {
             GcodeCommand command = getActiveCommand().get();
-            command.appendResponse(response);
-
             if (command instanceof FluidNCCommand) {
                 if (command.isDone()) {
                     activeCommands.removeFirst();