From 3f1b34c30d39e9668a7bc24101e6c41aac7f8521 Mon Sep 17 00:00:00 2001 From: Joacim Breiler Date: Fri, 6 Dec 2024 05:20:33 +0100 Subject: [PATCH] Simplified the GRBL settings handling (#2653) --- .../universalgcodesender/GrblController.java | 3 +- .../firmware/grbl/GrblFirmwareSettings.java | 80 +++-- ...lFirmwareSettingsCommunicatorListener.java | 275 ------------------ .../grbl/GrblFirmwareSettingsInterceptor.java | 124 ++++++++ ...GFirmwareSettingsCommunicatorListener.java | 3 +- .../GrblFirmwareSettingsInterceptorTest.java | 83 ++++++ .../grbl/GrblFirmwareSettingsTest.java | 231 +++++++++------ .../ugs/nbp/core/actions/HomingAction.java | 11 +- 8 files changed, 398 insertions(+), 412 deletions(-) delete mode 100644 ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsCommunicatorListener.java create mode 100644 ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptor.java create mode 100644 ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptorTest.java diff --git a/ugs-core/src/com/willwinder/universalgcodesender/GrblController.java b/ugs-core/src/com/willwinder/universalgcodesender/GrblController.java index 6b240aa30a..46523e3a63 100644 --- a/ugs-core/src/com/willwinder/universalgcodesender/GrblController.java +++ b/ugs-core/src/com/willwinder/universalgcodesender/GrblController.java @@ -24,6 +24,7 @@ This file is part of Universal Gcode Sender (UGS). import com.willwinder.universalgcodesender.firmware.IFirmwareSettings; import com.willwinder.universalgcodesender.firmware.grbl.GrblCommandCreator; import com.willwinder.universalgcodesender.firmware.grbl.GrblFirmwareSettings; +import com.willwinder.universalgcodesender.firmware.grbl.GrblFirmwareSettingsInterceptor; import com.willwinder.universalgcodesender.gcode.util.GcodeUtils; import com.willwinder.universalgcodesender.i18n.Localization; import com.willwinder.universalgcodesender.listeners.ControllerState; @@ -93,9 +94,9 @@ public GrblController(ICommunicator communicator) { super(communicator, new GrblCommandCreator()); this.positionPollTimer = new StatusPollTimer(this); this.firmwareSettings = new GrblFirmwareSettings(this); - this.comm.addListener(firmwareSettings); this.initializer = new GrblControllerInitializer(this); this.overrideManager = new GrblOverrideManager(this, communicator, messageService); + new GrblFirmwareSettingsInterceptor(this, firmwareSettings); } public GrblController() { diff --git a/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettings.java b/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettings.java index 31e510ce78..04feec9b13 100644 --- a/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettings.java +++ b/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettings.java @@ -1,5 +1,5 @@ /* - Copyright 2018 Will Winder + Copyright 2018-2024 Will Winder This file is part of Universal Gcode Sender (UGS). @@ -19,16 +19,15 @@ This file is part of Universal Gcode Sender (UGS). package com.willwinder.universalgcodesender.firmware.grbl; import com.willwinder.universalgcodesender.IController; -import com.willwinder.universalgcodesender.communicator.AbstractCommunicator; import com.willwinder.universalgcodesender.firmware.FirmwareSetting; import com.willwinder.universalgcodesender.firmware.FirmwareSettingsException; import com.willwinder.universalgcodesender.firmware.IFirmwareSettings; import com.willwinder.universalgcodesender.firmware.IFirmwareSettingsListener; import com.willwinder.universalgcodesender.i18n.Localization; -import com.willwinder.universalgcodesender.communicator.ICommunicatorListener; import com.willwinder.universalgcodesender.model.Axis; import com.willwinder.universalgcodesender.model.UnitUtils; import com.willwinder.universalgcodesender.types.GcodeCommand; +import com.willwinder.universalgcodesender.utils.ControllerUtils; import org.apache.commons.lang3.math.NumberUtils; import java.text.DecimalFormat; @@ -36,19 +35,18 @@ This file is part of Universal Gcode Sender (UGS). import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.logging.Logger; /** - * Handles the firmware settings on a GRBL controller. It needs to be registered as a listener - * to {@link AbstractCommunicator#addListener(ICommunicatorListener)} - * for it to be able to process all commands to/from the controller. + * Handles the firmware settings on a GRBL controller. * * @author Joacim Breiler * @author MerrellM */ -public class GrblFirmwareSettings implements ICommunicatorListener, IFirmwareSettingsListener, IFirmwareSettings { +public class GrblFirmwareSettings implements IFirmwareSettings { private static final Logger LOGGER = Logger.getLogger(GrblFirmwareSettings.class.getName()); /** @@ -78,13 +76,17 @@ public class GrblFirmwareSettings implements ICommunicatorListener, IFirmwareSet private final Map settings = new ConcurrentHashMap<>(); /** - * A delegate for all serial communication handling + * All listeners for listening to changed settings */ - private final GrblFirmwareSettingsCommunicatorListener serialCommunicatorDelegate; + private final Set listeners = ConcurrentHashMap.newKeySet(); + + /** + * The controller to be used for communication + */ + private final IController controller; public GrblFirmwareSettings(IController controller) { - this.serialCommunicatorDelegate = new GrblFirmwareSettingsCommunicatorListener(controller); - this.serialCommunicatorDelegate.addListener(this); + this.controller = controller; } /** @@ -109,9 +111,18 @@ synchronized public FirmwareSetting setValue(final String key, final String valu // Make a copy of existing property and send it to our controller final FirmwareSetting newSetting = new FirmwareSetting(oldSetting.getKey(), value, oldSetting.getUnits(), oldSetting.getDescription(), oldSetting.getShortDescription()); - return serialCommunicatorDelegate - .updateSettingOnController(newSetting) - .orElse(oldSetting); + try { + GcodeCommand command = controller.createCommand(newSetting.getKey() + "=" + newSetting.getValue()); + ControllerUtils.sendAndWaitForCompletion(controller, command); + if (command.isOk()) { + updateFirmwareSetting(newSetting); + return newSetting; + } + } catch (Exception e) { + throw new FirmwareSettingsException("Couldn't send update setting command to the controller: " + newSetting.getKey() + "=" + newSetting.getValue() + ".", e); + } + + return oldSetting; } /** @@ -139,12 +150,12 @@ public FirmwareSetting setValue(final String key, final double value) throws Fir @Override public void addListener(IFirmwareSettingsListener listener) { - serialCommunicatorDelegate.addListener(listener); + listeners.add(listener); } @Override public void removeListener(IFirmwareSettingsListener listener) { - serialCommunicatorDelegate.removeListener(listener); + listeners.remove(listener); } @Override @@ -421,40 +432,15 @@ public UnitUtils.Units getReportingUnits() { .orElse(UnitUtils.Units.UNKNOWN); } - /* - * SerialCommunicatorListener - */ - @Override - public void rawResponseListener(String response) { - serialCommunicatorDelegate.rawResponseListener(response); - } - - @Override - public void commandSent(GcodeCommand command) { - serialCommunicatorDelegate.commandSent(command); - } - - @Override - public void commandSkipped(GcodeCommand command) { - serialCommunicatorDelegate.commandSkipped(command); - } - - @Override - public void communicatorPausedOnError() { - serialCommunicatorDelegate.communicatorPausedOnError(); - } - - @Override - public void onConnectionClosed() { - } - - /* - * IFirmwareSettingsListener + /** + * Updates a firmware setting + * + * @param setting the setting */ - @Override - public void onUpdatedFirmwareSetting(FirmwareSetting setting) { + public void updateFirmwareSetting(FirmwareSetting setting) { LOGGER.log(Level.FINE, "Updating setting " + setting.getKey() + " = " + setting.getValue()); settings.put(setting.getKey(), setting); + listeners.forEach(listener -> listener.onUpdatedFirmwareSetting(setting)); } /* diff --git a/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsCommunicatorListener.java b/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsCommunicatorListener.java deleted file mode 100644 index d081d54b02..0000000000 --- a/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsCommunicatorListener.java +++ /dev/null @@ -1,275 +0,0 @@ -/* - Copyright 2018 Will Winder - - This file is part of Universal Gcode Sender (UGS). - - UGS is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - UGS is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with UGS. If not, see . - */ -package com.willwinder.universalgcodesender.firmware.grbl; - -import com.willwinder.universalgcodesender.GrblUtils; -import com.willwinder.universalgcodesender.IController; -import com.willwinder.universalgcodesender.firmware.FirmwareSetting; -import com.willwinder.universalgcodesender.firmware.FirmwareSettingsException; -import com.willwinder.universalgcodesender.firmware.IFirmwareSettingsListener; -import com.willwinder.universalgcodesender.communicator.ICommunicatorListener; -import com.willwinder.universalgcodesender.types.GcodeCommand; -import com.willwinder.universalgcodesender.utils.GrblLookups; -import com.willwinder.universalgcodesender.utils.ThreadHelper; - -import java.util.*; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * A serial communicator for handling settings on a GRBL controller. - * - * When updating a setting through {@link GrblFirmwareSettingsCommunicatorListener#updateSettingOnController(FirmwareSetting)} - * the method will block until the update process is finished or if the operation took too long. - * - * @author Joacim Breiler - */ -public class GrblFirmwareSettingsCommunicatorListener implements ICommunicatorListener { - private static final Logger logger = Logger.getLogger(GrblFirmwareSettingsCommunicatorListener.class.getName()); - - /** - * Number of seconds to wait until the controller has persisted the setting - */ - private static final int UPDATE_TIMEOUT_SECONDS = 2; - - /** - * Parser for settings message from GRBL containing the key and value. Ex: $13=0 - * Starting in GRBL 1.1 the description is disabled by default. - */ - private static final Pattern SETTING_MESSAGE_REGEX = Pattern.compile("\\$(\\d+)=([^ ]*)"); - - /** - * A lookup helper for fetching setting descriptions - */ - private final GrblLookups grblLookups; - - /** - * A connected controller - */ - private final IController controller; - - /** - * All listeners for listening to changed settings - */ - private final Set listeners; - - /** - * When updating a setting, the new setting is temporarily saved here - */ - private FirmwareSetting newSetting; - - /** - * When updating a setting, the old setting is temporarily saved here - */ - private FirmwareSetting updatedSetting; - - /** - * Constructor for creating a serial communicator - * - * @param controller the controller that is used for storing settings - */ - public GrblFirmwareSettingsCommunicatorListener(IController controller) { - this.grblLookups = new GrblLookups("setting_codes"); - this.controller = controller; - this.listeners = Collections.synchronizedSet(new HashSet<>()); - } - - /** - * Returns if the serial communicator is still sending the setting command - * and is awaiting reply from the controller. - * - * @return true if a setting is updating - */ - private boolean isUpdatingSettings() { - return newSetting != null; - } - - /** - * Adds a listener for new settings. - * - * @param listener a listener to add - */ - public void addListener(IFirmwareSettingsListener listener) { - listeners.add(listener); - } - - /** - * Removes a listener. - * - * @param listener a listener to remove - */ - public void removeListener(IFirmwareSettingsListener listener) { - listeners.remove(listener); - } - - /** - * Converts a response message to a firmware setting. If the response message isn't in the format - * {@code $[key]=[value]} an empty optional will be returned. - * - * @param response the response message from the controller - * @return the converted firmware setting or an empty optional if the response was unknown. - */ - private Optional convertMessageToSetting(String response) { - Matcher settingMatcher = SETTING_MESSAGE_REGEX.matcher(response); - if (!settingMatcher.find()) { - return Optional.empty(); - } - - String key = "$" + settingMatcher.group(1); - String value = settingMatcher.group(2); - String units = ""; - String description = ""; - String shortDescription = ""; - - String[] lookup = grblLookups.lookup(settingMatcher.group(1)); - if (lookup != null) { - shortDescription = lookup[1]; - units = lookup[2]; - description = lookup[3]; - } - - return Optional.of(new FirmwareSetting(key, value, units, description, shortDescription)); - } - - /** - * Returns if the controller is ready to receive setting commands. - * - * @return true if controller is ready - */ - private boolean canSendToController() { - return controller != null && controller.isCommOpen() && !controller.isStreaming(); - } - - /** - * Sends a command to update a setting on the controller. The method will block until we get a response - * from the controller or if a timeout is triggered if the setting took too long to update. - * - * @param setting the setting to update - * @return the updated setting or an empty optional if it couldn't be updated. - * @throws FirmwareSettingsException will be thrown if the controller isn't ready to receive setting updates or if - * the update took to long and caused a timeout - */ - public Optional updateSettingOnController(FirmwareSetting setting) throws FirmwareSettingsException { - - if (isUpdatingSettings()) { - throw new FirmwareSettingsException("The settings are being updated in another thread."); - } - - if (!canSendToController()) { - throw new FirmwareSettingsException("The controller is not ready to receive commands."); - } - - boolean previousSingleStepMode = controller.getSingleStepMode(); - boolean previousStatusUpdatesEnabled = controller.getStatusUpdatesEnabled(); - controller.setStatusUpdatesEnabled(false); - controller.setSingleStepMode(true); - - try { - try { - updatedSetting = null; - newSetting = setting; - GcodeCommand command = controller.createCommand(setting.getKey() + "=" + setting.getValue()); - controller.sendCommandImmediately(command); - } catch (Exception e) { - throw new FirmwareSettingsException("Couldn't send update setting command to the controller: " + setting.getKey() + "=" + setting.getValue() + ".", e); - } - - waitUntilUpdateFinished(); - } finally { - controller.setSingleStepMode(previousSingleStepMode); - controller.setStatusUpdatesEnabled(previousStatusUpdatesEnabled); - } - - // Reset internal states - Optional result = Optional.ofNullable(updatedSetting); - updatedSetting = null; - newSetting = null; - return result; - } - - /** - * Block and wait until the setting has been updated or until a timeout has occured. - * The timeout will wait {@link GrblFirmwareSettingsCommunicatorListener#UPDATE_TIMEOUT_SECONDS}. - * - * @throws FirmwareSettingsException if a timeout has occured. - */ - private void waitUntilUpdateFinished() throws FirmwareSettingsException { - try { - ThreadHelper.waitUntil(() -> !isUpdatingSettings(), UPDATE_TIMEOUT_SECONDS, TimeUnit.SECONDS); - } catch (TimeoutException ignored) { - newSetting = null; - throw new FirmwareSettingsException("Timeout while updating the setting on the controller."); - } - } - - @Override - public void rawResponseListener(String response) { - - // Make sure we either got a setting response or has sent an update command to the controller. - if (!SETTING_MESSAGE_REGEX.matcher(response).find() && !isUpdatingSettings()) { - return; - } - - Optional setting = convertMessageToSetting(response); - if (setting.isPresent()) { - updatedSetting = setting.get(); - newSetting = null; - listeners.forEach(listener -> listener.onUpdatedFirmwareSetting(updatedSetting)); - } else if (isUpdatingSettings() && GrblUtils.isErrorResponse(response)) { - logger.log(Level.WARNING, "Couldn't update setting " + newSetting.getKey() + " with value " + newSetting.getValue()); - updatedSetting = null; - newSetting = null; - controller.cancelCommands(); - } else if (isUpdatingSettings() && GrblUtils.isOkResponse(response)) { - logger.log(Level.INFO, "Updated setting " + newSetting.getKey() + " to " + newSetting.getValue()); - updatedSetting = newSetting; - newSetting = null; - listeners.forEach(listener -> listener.onUpdatedFirmwareSetting(updatedSetting)); - } else { - logger.log(Level.WARNING, "Got unexpected message while waiting for setting update status: " + response); - } - } - - @Override - public void commandSent(GcodeCommand command) { - // We are about to receive all settings from the controller - if (command.getCommandString().startsWith(GrblUtils.GRBL_VIEW_SETTINGS_COMMAND)) { - newSetting = null; - updatedSetting = null; - } - } - - @Override - public void commandSkipped(GcodeCommand command) { - - } - - @Override - public void communicatorPausedOnError() { - - } - - @Override - public void onConnectionClosed() { - } -} diff --git a/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptor.java b/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptor.java new file mode 100644 index 0000000000..69a3c77af5 --- /dev/null +++ b/ugs-core/src/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptor.java @@ -0,0 +1,124 @@ +/* + Copyright 2018-2024 Will Winder + + This file is part of Universal Gcode Sender (UGS). + + UGS is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + UGS is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with UGS. If not, see . + */ +package com.willwinder.universalgcodesender.firmware.grbl; + +import com.willwinder.universalgcodesender.GrblUtils; +import com.willwinder.universalgcodesender.IController; +import com.willwinder.universalgcodesender.firmware.FirmwareSetting; +import com.willwinder.universalgcodesender.listeners.DefaultControllerListener; +import com.willwinder.universalgcodesender.types.GcodeCommand; +import com.willwinder.universalgcodesender.utils.GrblLookups; + +import java.util.Arrays; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * A firmware settings interceptor that will update the settings that is sent or retrieved. + * + * @author Joacim Breiler + */ +public class GrblFirmwareSettingsInterceptor extends DefaultControllerListener { + + /** + * Parser for settings message from GRBL containing the key and value. Ex: $13=0 + * Starting in GRBL 1.1 the description is disabled by default. + */ + private static final Pattern SETTING_MESSAGE_REGEX = Pattern.compile("\\$(\\d+)=([^ ]*)"); + + /** + * A lookup helper for fetching setting descriptions + */ + private final GrblLookups grblLookups; + + /** + * A connected controller + */ + private final GrblFirmwareSettings firmwareSettings; + private final IController controller; + + /** + * Constructor for creating a serial communicator + * + * @param controller the controller that is used for storing settings + */ + public GrblFirmwareSettingsInterceptor(IController controller, GrblFirmwareSettings firmwareSettings) { + this.grblLookups = new GrblLookups("setting_codes"); + this.firmwareSettings = firmwareSettings; + this.controller = controller; + controller.addListener(this); + } + + /** + * Converts a response message to a firmware setting. If the response message isn't in the format + * {@code $[key]=[value]} an empty optional will be returned. + * + * @param response the response message from the controller + * @return the converted firmware setting or an empty optional if the response was unknown. + */ + private Optional convertMessageToSetting(String response) { + Matcher settingMatcher = SETTING_MESSAGE_REGEX.matcher(response); + if (!settingMatcher.find()) { + return Optional.empty(); + } + + String key = "$" + settingMatcher.group(1); + String value = settingMatcher.group(2); + String units = ""; + String description = ""; + String shortDescription = ""; + + String[] lookup = grblLookups.lookup(settingMatcher.group(1)); + if (lookup != null) { + shortDescription = lookup[1]; + units = lookup[2]; + description = lookup[3]; + } + + return Optional.of(new FirmwareSetting(key, value, units, description, shortDescription)); + } + + /** + * Unregisters this as a listener to the controller + */ + public void destroy() { + controller.removeListener(this); + } + + @Override + public void commandComplete(GcodeCommand command) { + if (!command.isOk()) { + return; + } + + // If the command was a setting command + if (SETTING_MESSAGE_REGEX.matcher(command.getCommandString()).find()) { + convertMessageToSetting(command.getCommandString()) + .ifPresent(firmwareSettings::updateFirmwareSetting); + } else if (command.getCommandString().equals(GrblUtils.GRBL_VIEW_SETTINGS_COMMAND)) { + Arrays.stream(command.getResponse().split("\n")) + .filter(line -> SETTING_MESSAGE_REGEX.matcher(line).find()) + .map(this::convertMessageToSetting) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(firmwareSettings::updateFirmwareSetting); + } + } +} diff --git a/ugs-core/src/com/willwinder/universalgcodesender/firmware/tinyg/TinyGFirmwareSettingsCommunicatorListener.java b/ugs-core/src/com/willwinder/universalgcodesender/firmware/tinyg/TinyGFirmwareSettingsCommunicatorListener.java index 1d76289179..e8fb488933 100644 --- a/ugs-core/src/com/willwinder/universalgcodesender/firmware/tinyg/TinyGFirmwareSettingsCommunicatorListener.java +++ b/ugs-core/src/com/willwinder/universalgcodesender/firmware/tinyg/TinyGFirmwareSettingsCommunicatorListener.java @@ -24,7 +24,6 @@ This file is part of Universal Gcode Sender (UGS). import com.willwinder.universalgcodesender.firmware.FirmwareSetting; import com.willwinder.universalgcodesender.firmware.FirmwareSettingsException; import com.willwinder.universalgcodesender.firmware.IFirmwareSettingsListener; -import com.willwinder.universalgcodesender.firmware.grbl.GrblFirmwareSettingsCommunicatorListener; import com.willwinder.universalgcodesender.communicator.ICommunicatorListener; import com.willwinder.universalgcodesender.types.GcodeCommand; import com.willwinder.universalgcodesender.firmware.tinyg.commands.TinyGGcodeCommand; @@ -218,7 +217,7 @@ public Optional updateSettingOnController(FirmwareSetting setti /** * Block and wait until the setting has been updated or until a timeout has occured. - * The timeout will wait {@link GrblFirmwareSettingsCommunicatorListener#UPDATE_TIMEOUT_SECONDS}. + * The timeout will wait {@link com.willwinder.universalgcodesender.firmware.tinyg.TinyGFirmwareSettingsCommunicatorListener#UPDATE_TIMEOUT_SECONDS}. * * @throws FirmwareSettingsException if a timeout has occured. */ diff --git a/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptorTest.java b/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptorTest.java new file mode 100644 index 0000000000..5836957c86 --- /dev/null +++ b/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsInterceptorTest.java @@ -0,0 +1,83 @@ +package com.willwinder.universalgcodesender.firmware.grbl; + +import com.willwinder.universalgcodesender.IController; +import com.willwinder.universalgcodesender.firmware.FirmwareSetting; +import com.willwinder.universalgcodesender.types.GcodeCommand; +import static org.junit.Assert.assertEquals; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.List; + +public class GrblFirmwareSettingsInterceptorTest { + private GrblFirmwareSettingsInterceptor target; + + private GrblFirmwareSettings firmwareSettings; + + @Before + public void setUp() { + IController controller = mock(IController.class); + firmwareSettings = mock(GrblFirmwareSettings.class); + target = new GrblFirmwareSettingsInterceptor(controller, firmwareSettings); + } + + @Test + public void shouldUpdateSettingsOnFirmwareSettingsCommandWithOkResponse() { + ArgumentCaptor firmwareSettingArgumentCaptor = ArgumentCaptor.forClass(FirmwareSetting.class); + doNothing().when(firmwareSettings).updateFirmwareSetting(firmwareSettingArgumentCaptor.capture()); + + GcodeCommand command = new GcodeCommand("$$"); + command.appendResponse("$21=0"); + command.appendResponse("$22=1"); + command.appendResponse("ok"); + target.commandComplete(command); + + verify(firmwareSettings, times(2)).updateFirmwareSetting(any()); + List settingUpdates = firmwareSettingArgumentCaptor.getAllValues(); + assertEquals(2, settingUpdates.size()); + assertEquals("$21", settingUpdates.get(0).getKey()); + assertEquals("0", settingUpdates.get(0).getValue()); + assertEquals("$22", settingUpdates.get(1).getKey()); + assertEquals("1", settingUpdates.get(1).getValue()); + } + + @Test + public void shouldNotUpdateSettingsOnFirmwareSettingsCommandWithErrorResponse() { + GcodeCommand command = new GcodeCommand("$$"); + command.appendResponse("$21=0"); + command.appendResponse("$22=0"); + command.appendResponse("error"); + target.commandComplete(command); + verify(firmwareSettings, times(0)).updateFirmwareSetting(any()); + } + + @Test + public void shouldUpdateSettingOnFirmwareSettingCommandWithOkResponse() { + ArgumentCaptor firmwareSettingArgumentCaptor = ArgumentCaptor.forClass(FirmwareSetting.class); + doNothing().when(firmwareSettings).updateFirmwareSetting(firmwareSettingArgumentCaptor.capture()); + + GcodeCommand command = new GcodeCommand("$21=1"); + command.appendResponse("ok"); + target.commandComplete(command); + + verify(firmwareSettings, times(1)).updateFirmwareSetting(any()); + List settingUpdates = firmwareSettingArgumentCaptor.getAllValues(); + assertEquals(1, settingUpdates.size()); + assertEquals("$21", settingUpdates.get(0).getKey()); + assertEquals("1", settingUpdates.get(0).getValue()); + } + + @Test + public void shouldUpdateSettingOnFirmwareSettingCommandWithErrorResponse() { + GcodeCommand command = new GcodeCommand("$21=1"); + command.appendResponse("error"); + target.commandComplete(command); + verify(firmwareSettings, times(0)).updateFirmwareSetting(any()); + } +} \ No newline at end of file diff --git a/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsTest.java b/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsTest.java index 64e8d36c03..109d70a8ad 100644 --- a/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsTest.java +++ b/ugs-core/test/com/willwinder/universalgcodesender/firmware/grbl/GrblFirmwareSettingsTest.java @@ -6,18 +6,31 @@ import com.willwinder.universalgcodesender.firmware.IFirmwareSettingsListener; import com.willwinder.universalgcodesender.model.Axis; import com.willwinder.universalgcodesender.model.UnitUtils; +import com.willwinder.universalgcodesender.types.GcodeCommand; import com.willwinder.universalgcodesender.utils.ThreadHelper; +import static org.assertj.core.api.Fail.fail; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import org.mockito.invocation.InvocationOnMock; import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - /** * @author Joacim Breiler */ @@ -37,9 +50,10 @@ public void settingMessagesShouldBeProcessedAndAdded() { assertEquals("The firmware settings should start with zero settings", 0, target.getAllSettings().size()); // Emulate a settings-message from the controller - target.rawResponseListener("$0=10"); + setFirmwareSetting("$0", "10"); assertEquals(1, target.getAllSettings().size()); + assertTrue(target.getSetting("$0").isPresent()); assertEquals("10", target.getSetting("$0").get().getValue()); } @@ -48,24 +62,25 @@ public void settingMessagesShouldNotAddDuplicates() { assertEquals("The firmware settings should start with zero settings", 0, target.getAllSettings().size()); // Emulate a settings-message from the controller - target.rawResponseListener("$0=10"); - target.rawResponseListener("$0=11"); + setFirmwareSetting("$0", "10"); + setFirmwareSetting("$0", "11"); assertEquals(1, target.getAllSettings().size()); + assertTrue(target.getSetting("$0").isPresent()); assertEquals("11", target.getSetting("$0").get().getValue()); } @Test - public void isHomingEnabledShouldBeTrue() throws InterruptedException, FirmwareSettingsException { + public void isHomingEnabledShouldBeTrue() throws FirmwareSettingsException { // Emulate a settings-message from the controller - target.rawResponseListener("$22=1"); + setFirmwareSetting("$22", "1"); assertTrue(target.isHomingEnabled()); } @Test public void isHomingEnabledShouldBeFalse() throws FirmwareSettingsException { // Emulate a settings-message from the controller - target.rawResponseListener("$22=0"); + setFirmwareSetting("$22", "0"); assertFalse(target.isHomingEnabled()); } @@ -81,19 +96,19 @@ public void getReportingUnitsShouldReturnUnknownIfNotSet() { @Test public void getReportingUnitsShouldReturnMm() { - target.rawResponseListener("$13=0"); + setFirmwareSetting("$13", "0"); assertEquals(UnitUtils.Units.MM, target.getReportingUnits()); } @Test public void getReportingUnitsShouldReturnInch() { - target.rawResponseListener("$13=1"); + setFirmwareSetting("$13", "1"); assertEquals(UnitUtils.Units.INCH, target.getReportingUnits()); } @Test public void getReportingUnitsShouldReturnUnkownOnUnknownValues() { - target.rawResponseListener("$13=2"); + setFirmwareSetting("$13", "2"); assertEquals(UnitUtils.Units.UNKNOWN, target.getReportingUnits()); } @@ -106,8 +121,8 @@ public void settingMessagesShouldBeSentAsEvents() { doNothing().when(firmwareSettingsListener).onUpdatedFirmwareSetting(firmwareSettingArgumentCaptor.capture()); // Emulate settings messages from the controller - target.rawResponseListener("$0=10"); - target.rawResponseListener("$0=11"); + setFirmwareSetting("$0", "10"); + setFirmwareSetting("$0", "11"); List settingUpdates = firmwareSettingArgumentCaptor.getAllValues(); assertEquals(2, settingUpdates.size()); @@ -123,7 +138,7 @@ public void setValueForSettingThatDoesNotExistShouldThrowException() throws Firm @Test public void setValueWithSameValueShouldNotUpdate() throws FirmwareSettingsException { // Given - target.rawResponseListener("$0=10"); + setFirmwareSetting("$0", "10"); // When IFirmwareSettingsListener firmwareSettingsListener = mock(IFirmwareSettingsListener.class); @@ -138,9 +153,9 @@ public void setValueWithSameValueShouldNotUpdate() throws FirmwareSettingsExcept @Test public void setValueShouldUpdateOnController() throws Exception { // Given - when(controller.isStreaming()).thenReturn(false); - when(controller.isCommOpen()).thenReturn(true); - target.rawResponseListener("$0=10"); + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$0", "10"); // Add a listener IFirmwareSettingsListener firmwareSettingsListener = mock(IFirmwareSettingsListener.class); @@ -161,7 +176,8 @@ public void setValueShouldUpdateOnController() throws Exception { // Simulate the response from the controller Thread.sleep(200); - target.rawResponseListener("ok"); + command.setOk(true); + command.setDone(true); // Wait until the value gets updated FirmwareSetting firmwareSetting = (FirmwareSetting) setValueFuture.get(); @@ -177,9 +193,9 @@ public void setValueShouldUpdateOnController() throws Exception { @Test public void setValueShouldNotUpdateOnError() throws Exception { // Given - when(controller.isStreaming()).thenReturn(false); - when(controller.isCommOpen()).thenReturn(true); - target.rawResponseListener("$0=10"); + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$0", "10"); // Add a listener IFirmwareSettingsListener firmwareSettingsListener = mock(IFirmwareSettingsListener.class); @@ -198,7 +214,8 @@ public void setValueShouldNotUpdateOnError() throws Exception { // Simulate the response from the controller Thread.sleep(200); - target.rawResponseListener("error"); + command.setError(true); + command.setDone(true); // Wait until the value gets updated FirmwareSetting setting = (FirmwareSetting) setValueFuture.get(); @@ -213,59 +230,52 @@ public void setValueShouldNotUpdateOnError() throws Exception { @Test public void setValueShouldTimeoutIfNoResponseFromController() throws Exception { // Given - when(controller.isStreaming()).thenReturn(false); - when(controller.isCommOpen()).thenReturn(true); - target.rawResponseListener("$0=10"); + when(controller.createCommand(anyString())).thenAnswer((InvocationOnMock invocation) -> new GcodeCommand(invocation.getArgument(0))); + setFirmwareSetting("$0", "10"); // Add a listener IFirmwareSettingsListener firmwareSettingsListener = mock(IFirmwareSettingsListener.class); target.addListener(firmwareSettingsListener); - // When - try { - target.setValue("$0", "11"); - } catch (FirmwareSettingsException e) { - assertTrue("Make sure the exception contains the word 'Timeout'", e.getMessage().contains("Timeout")); - } - - // Then + // When / Then + assertThrows(FirmwareSettingsException.class, () -> target.setValue("$0", "11")); verify(controller, times(1)).sendCommandImmediately(any()); verify(firmwareSettingsListener, times(0)).onUpdatedFirmwareSetting(any()); } @Test public void getInvertDirectionShouldReturnEachBitAsAxis() throws FirmwareSettingsException { - target.rawResponseListener("$3=0"); - assertEquals(false, target.isInvertDirection(Axis.X)); - assertEquals(false, target.isInvertDirection(Axis.Y)); - assertEquals(false, target.isInvertDirection(Axis.Z)); - - target.rawResponseListener("$3=1"); - assertEquals(true, target.isInvertDirection(Axis.X)); - assertEquals(false, target.isInvertDirection(Axis.Y)); - assertEquals(false, target.isInvertDirection(Axis.Z)); - - target.rawResponseListener("$3=2"); - assertEquals(false, target.isInvertDirection(Axis.X)); - assertEquals(true, target.isInvertDirection(Axis.Y)); - assertEquals(false, target.isInvertDirection(Axis.Z)); - - target.rawResponseListener("$3=4"); - assertEquals(false, target.isInvertDirection(Axis.X)); - assertEquals(false, target.isInvertDirection(Axis.Y)); - assertEquals(true, target.isInvertDirection(Axis.Z)); - - target.rawResponseListener("$3=7"); - assertEquals(true, target.isInvertDirection(Axis.X)); - assertEquals(true, target.isInvertDirection(Axis.Y)); - assertEquals(true, target.isInvertDirection(Axis.Z)); + setFirmwareSetting("$3", "0"); + assertFalse(target.isInvertDirection(Axis.X)); + assertFalse(target.isInvertDirection(Axis.Y)); + assertFalse(target.isInvertDirection(Axis.Z)); + + setFirmwareSetting("$3", "1"); + assertTrue(target.isInvertDirection(Axis.X)); + assertFalse(target.isInvertDirection(Axis.Y)); + assertFalse(target.isInvertDirection(Axis.Z)); + + setFirmwareSetting("$3", "2"); + assertFalse(target.isInvertDirection(Axis.X)); + assertTrue(target.isInvertDirection(Axis.Y)); + assertFalse(target.isInvertDirection(Axis.Z)); + + setFirmwareSetting("$3", "4"); + assertFalse(target.isInvertDirection(Axis.X)); + assertFalse(target.isInvertDirection(Axis.Y)); + assertTrue(target.isInvertDirection(Axis.Z)); + + setFirmwareSetting("$3", "7"); + assertTrue(target.isInvertDirection(Axis.X)); + assertTrue(target.isInvertDirection(Axis.Y)); + assertTrue(target.isInvertDirection(Axis.Z)); } @Test - public void setInvertDirectionXShouldSetBit() throws FirmwareSettingsException, InterruptedException { - when(controller.isStreaming()).thenReturn(false); - when(controller.isCommOpen()).thenReturn(true); - target.rawResponseListener("$3=7"); + public void setInvertDirectionXToFalseShouldUnsetBit() throws Exception { + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$3", "7"); // Try setting X to false ThreadHelper.invokeLater(() -> { @@ -277,11 +287,22 @@ public void setInvertDirectionXShouldSetBit() throws FirmwareSettingsException, }); Thread.sleep(100); - target.rawResponseListener("ok"); + // Simulate answer from server + command.setOk(true); + command.setDone(true); + Thread.sleep(100); + + assertTrue(target.getSetting("$3").isPresent()); assertEquals("6", target.getSetting("$3").get().getValue()); + } + @Test + public void setInvertDirectionXShouldSetBit() throws Exception { + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$3", "6"); - // Try setting X to true + // Try setting X to false ThreadHelper.invokeLater(() -> { try { target.setInvertDirection(Axis.X, true); @@ -290,16 +311,21 @@ public void setInvertDirectionXShouldSetBit() throws FirmwareSettingsException, } }); Thread.sleep(100); - target.rawResponseListener("ok"); + // Simulate answer from server + command.setOk(true); + command.setDone(true); + Thread.sleep(100); + + assertTrue(target.getSetting("$3").isPresent()); assertEquals("7", target.getSetting("$3").get().getValue()); } @Test - public void setInvertDirectionYShouldSetBit() throws FirmwareSettingsException, InterruptedException { - when(controller.isStreaming()).thenReturn(false); - when(controller.isCommOpen()).thenReturn(true); - target.rawResponseListener("$3=7"); + public void setInvertDirectionYShouldSetBit() throws Exception { + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$3", "7"); // Try setting Y to false ThreadHelper.invokeLater(() -> { @@ -310,11 +336,23 @@ public void setInvertDirectionYShouldSetBit() throws FirmwareSettingsException, } }); Thread.sleep(100); - target.rawResponseListener("ok"); + + // Simulate answer from server + command.setOk(true); + command.setDone(true); + Thread.sleep(100); + + assertTrue(target.getSetting("$3").isPresent()); assertEquals("5", target.getSetting("$3").get().getValue()); + } + @Test + public void setInvertDirectionYShouldUnsetBit() throws Exception { + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$3", "5"); - // Try setting Y to true + // Try setting Y to false ThreadHelper.invokeLater(() -> { try { target.setInvertDirection(Axis.Y, true); @@ -323,16 +361,21 @@ public void setInvertDirectionYShouldSetBit() throws FirmwareSettingsException, } }); Thread.sleep(100); - target.rawResponseListener("ok"); + // Simulate answer from server + command.setOk(true); + command.setDone(true); + Thread.sleep(100); + + assertTrue(target.getSetting("$3").isPresent()); assertEquals("7", target.getSetting("$3").get().getValue()); } @Test - public void setInvertDirectionZShouldSetBit() throws FirmwareSettingsException, InterruptedException { - when(controller.isStreaming()).thenReturn(false); - when(controller.isCommOpen()).thenReturn(true); - target.rawResponseListener("$3=7"); + public void setInvertDirectionZShouldSetBit() throws Exception { + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$3", "7"); // Try setting Z to false ThreadHelper.invokeLater(() -> { @@ -343,11 +386,24 @@ public void setInvertDirectionZShouldSetBit() throws FirmwareSettingsException, } }); Thread.sleep(100); - target.rawResponseListener("ok"); + + // Simulate answer from server + command.setOk(true); + command.setDone(true); + Thread.sleep(100); + + assertTrue(target.getSetting("$3").isPresent()); assertEquals("3", target.getSetting("$3").get().getValue()); + } - // Try setting Z to true + @Test + public void setInvertDirectionZShouldUnsetBit() throws Exception { + GcodeCommand command = new GcodeCommand(""); + when(controller.createCommand(anyString())).thenReturn(command); + setFirmwareSetting("$3", "3"); + + // Try setting Z to false ThreadHelper.invokeLater(() -> { try { target.setInvertDirection(Axis.Z, true); @@ -356,23 +412,32 @@ public void setInvertDirectionZShouldSetBit() throws FirmwareSettingsException, } }); Thread.sleep(100); - target.rawResponseListener("ok"); + // Simulate answer from server + command.setOk(true); + command.setDone(true); + Thread.sleep(100); + + assertTrue(target.getSetting("$3").isPresent()); assertEquals("7", target.getSetting("$3").get().getValue()); } @Test public void getMaxSpindleSpeedShouldReturnIntegerValue() throws FirmwareSettingsException { - target.rawResponseListener("$30=1000.0"); + setFirmwareSetting("$30", "1000.0"); assertEquals(1000, target.getMaxSpindleSpeed()); - target.rawResponseListener("$30=1000"); + setFirmwareSetting("$30", "1000"); assertEquals(1000, target.getMaxSpindleSpeed()); - target.rawResponseListener("$30=1000,00"); + setFirmwareSetting("$30", "1000,00"); assertEquals(1000, target.getMaxSpindleSpeed()); - target.rawResponseListener("$30=1000.9"); + setFirmwareSetting("$30", "1000.9"); assertEquals(1000, target.getMaxSpindleSpeed()); } + + private void setFirmwareSetting(String key, String value) { + target.updateFirmwareSetting(new FirmwareSetting(key, value)); + } } diff --git a/ugs-platform/ugs-platform-ugscore/src/main/java/com/willwinder/ugs/nbp/core/actions/HomingAction.java b/ugs-platform/ugs-platform-ugscore/src/main/java/com/willwinder/ugs/nbp/core/actions/HomingAction.java index 6bcfab1960..66323e9036 100644 --- a/ugs-platform/ugs-platform-ugscore/src/main/java/com/willwinder/ugs/nbp/core/actions/HomingAction.java +++ b/ugs-platform/ugs-platform-ugscore/src/main/java/com/willwinder/ugs/nbp/core/actions/HomingAction.java @@ -28,6 +28,7 @@ This file is part of Universal Gcode Sender (UGS). import com.willwinder.universalgcodesender.model.BackendAPI; import com.willwinder.universalgcodesender.model.UGSEvent; import com.willwinder.universalgcodesender.model.events.ControllerStateEvent; +import com.willwinder.universalgcodesender.model.events.FirmwareSettingEvent; import com.willwinder.universalgcodesender.utils.GUIHelpers; import org.openide.awt.ActionID; import org.openide.awt.ActionReference; @@ -35,7 +36,9 @@ This file is part of Universal Gcode Sender (UGS). import org.openide.awt.ActionRegistration; import org.openide.util.ImageUtilities; -import javax.swing.*; +import javax.swing.AbstractAction; +import javax.swing.Action; +import java.awt.EventQueue; import java.awt.event.ActionEvent; @ActionID( @@ -57,7 +60,7 @@ public final class HomingAction extends AbstractAction implements UGSEventListen public static final String ICON_BASE = "resources/icons/home.svg"; - private BackendAPI backend; + private final BackendAPI backend; public HomingAction() { this.backend = CentralLookup.getDefault().lookup(BackendAPI.class); @@ -73,8 +76,8 @@ public HomingAction() { @Override public void UGSEvent(UGSEvent cse) { - if (cse instanceof ControllerStateEvent) { - java.awt.EventQueue.invokeLater(() -> { + if (cse instanceof ControllerStateEvent || cse instanceof FirmwareSettingEvent) { + EventQueue.invokeLater(() -> { updateToolTip(); setEnabled(isEnabled()); });