Skip to content

Commit

Permalink
Add better error message when FluidNC configuration is broken (#2574)
Browse files Browse the repository at this point in the history
  • Loading branch information
breiler authored Jul 26, 2024
1 parent cd06f49 commit d7b0749
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ protected void openCommAfterEvent() throws Exception {
/**
* Called prior to sending commands, throw an exception if not ready.
*/
abstract protected void isReadyToSendCommandsEvent() throws Exception;
abstract protected void isReadyToSendCommandsEvent() throws ControllerException;
/**
* Called prior to streaming commands, separate in case you need to be more
* restrictive about streaming a file vs. sending a command.
Expand Down Expand Up @@ -490,11 +490,11 @@ public GcodeState getCurrentGcodeState() {
* Note: this is the only place where a string is sent to the comm.
*/
@Override
public void sendCommandImmediately(GcodeCommand command) throws Exception {
public void sendCommandImmediately(GcodeCommand command) throws ControllerException {
isReadyToSendCommandsEvent();

if (!isCommOpen()) {
throw new Exception("Cannot send command(s), comm port is not open.");
throw new ControllerException("Cannot send command(s), comm port is not open.");
}

this.setCurrentState(CommunicatorState.COMM_SENDING);
Expand All @@ -503,13 +503,13 @@ public void sendCommandImmediately(GcodeCommand command) throws Exception {
}

@Override
public Boolean isReadyToReceiveCommands() throws Exception {
public Boolean isReadyToReceiveCommands() throws ControllerException {
if (!isCommOpen()) {
throw new Exception("Comm port is not open.");
throw new ControllerException("Comm port is not open.");
}

if (this.isStreaming()) {
throw new Exception("Already streaming.");
throw new ControllerException("Already streaming.");
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@ protected void isReadyToStreamCommandsEvent() throws Exception {
}

@Override
protected void isReadyToSendCommandsEvent() throws Exception {
protected void isReadyToSendCommandsEvent() throws ControllerException {
if (!isCommOpen()) {
throw new Exception(Localization.getString("controller.exception.booting"));
throw new ControllerException(Localization.getString("controller.exception.booting"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public interface IController {
/*
Stream information
*/
Boolean isReadyToReceiveCommands() throws Exception;
Boolean isReadyToReceiveCommands() throws ControllerException;
Boolean isReadyToStreamFile() throws Exception;
Boolean isStreaming();
long getSendDuration();
Expand Down Expand Up @@ -183,7 +183,7 @@ public interface IController {
Stream content
*/
GcodeCommand createCommand(String gcode) throws Exception;
void sendCommandImmediately(GcodeCommand cmd) throws Exception;
void sendCommandImmediately(GcodeCommand cmd) throws ControllerException;
void queueStream(IGcodeStreamReader r);

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022-2023 Will Winder
Copyright 2022-2024 Will Winder
This file is part of Universal Gcode Sender (UGS).
Expand All @@ -20,6 +20,7 @@ This file is part of Universal Gcode Sender (UGS).

import com.willwinder.universalgcodesender.Capabilities;
import com.willwinder.universalgcodesender.ConnectionWatchTimer;
import com.willwinder.universalgcodesender.ControllerException;
import com.willwinder.universalgcodesender.GrblCapabilitiesConstants;
import com.willwinder.universalgcodesender.GrblUtils;
import com.willwinder.universalgcodesender.IController;
Expand Down Expand Up @@ -64,6 +65,7 @@ This file is part of Universal Gcode Sender (UGS).
import static com.willwinder.universalgcodesender.model.UnitUtils.Units.MM;
import static com.willwinder.universalgcodesender.model.UnitUtils.scaleUnits;
import com.willwinder.universalgcodesender.services.MessageService;
import com.willwinder.universalgcodesender.types.CommandException;
import com.willwinder.universalgcodesender.types.GcodeCommand;
import com.willwinder.universalgcodesender.utils.ControllerUtils;
import static com.willwinder.universalgcodesender.utils.ControllerUtils.sendAndWaitForCompletion;
Expand Down Expand Up @@ -372,7 +374,7 @@ public Boolean isCommOpen() {
}

@Override
public Boolean isReadyToReceiveCommands() throws Exception {
public Boolean isReadyToReceiveCommands() {
return isCommOpen() && !this.isStreaming();
}

Expand Down Expand Up @@ -612,7 +614,17 @@ private void queryControllerInformation() throws Exception {

private void refreshFirmwareSettings() throws FirmwareSettingsException {
messageService.dispatchMessage(MessageType.INFO, "*** Fetching device settings\n");
firmwareSettings.refresh();
try {
firmwareSettings.refresh();
} catch (FirmwareSettingsException e) {
messageService.dispatchMessage(MessageType.ERROR, "*** There was an error while fetching the configuration from the controller:\n");
messageService.dispatchMessage(MessageType.ERROR, e.getMessage() + "\"");
throw e;
} catch (CommandException e) {
messageService.dispatchMessage(MessageType.ERROR, "*** There was an error while reading the configuration, see detailed error message below:\n");
messageService.dispatchMessage(MessageType.ERROR, e.getMessage() + "\"");
throw e;
}
}

@Override
Expand All @@ -627,7 +639,7 @@ public GcodeCommand createCommand(String command) throws Exception {
}

@Override
public void sendCommandImmediately(GcodeCommand cmd) throws Exception {
public void sendCommandImmediately(GcodeCommand cmd) throws ControllerException {
communicator.queueCommand(cmd);
communicator.streamCommands();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ This file is part of Universal Gcode Sender (UGS).
import com.willwinder.universalgcodesender.firmware.fluidnc.commands.GetFirmwareSettingsCommand;
import com.willwinder.universalgcodesender.model.Axis;
import com.willwinder.universalgcodesender.model.UnitUtils;
import com.willwinder.universalgcodesender.types.CommandException;
import com.willwinder.universalgcodesender.utils.ControllerUtils;
import org.apache.commons.lang3.StringUtils;

Expand Down Expand Up @@ -57,21 +58,24 @@ public FluidNCSettings(IController controller) {
this.controller = controller;
}

public void refresh() throws FirmwareSettingsException {
public void refresh() throws FirmwareSettingsException, CommandException {
GetFirmwareSettingsCommand firmwareSettingsCommand = new GetFirmwareSettingsCommand();

try {
GetFirmwareSettingsCommand firmwareSettingsCommand = new GetFirmwareSettingsCommand();
ControllerUtils.sendAndWaitForCompletion(controller, firmwareSettingsCommand);
} catch (InterruptedException e) {
throw new FirmwareSettingsException("Timed out waiting for the controller settings", e);
}

if (firmwareSettingsCommand.isOk()) {
firmwareSettingsCommand.getSettings().keySet().forEach(key -> {
String value = firmwareSettingsCommand.getSettings().get(key);
FirmwareSetting firmwareSetting = new FirmwareSetting(key, value, "", "", "");
settings.put(key.toLowerCase(), firmwareSetting);
listeners.forEach(l -> l.onUpdatedFirmwareSetting(firmwareSetting));
});
}
} catch (Exception e) {
throw new FirmwareSettingsException("Couldn't fetch settings", e);

if (firmwareSettingsCommand.isOk()) {
Map<String, String> responseSettings = firmwareSettingsCommand.getSettings();
responseSettings.keySet().forEach(key -> {
String value = responseSettings.get(key);
FirmwareSetting firmwareSetting = new FirmwareSetting(key, value, "", "", "");
settings.put(key.toLowerCase(), firmwareSetting);
listeners.forEach(l -> l.onUpdatedFirmwareSetting(firmwareSetting));
});
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ This file is part of Universal Gcode Sender (UGS).
*/
package com.willwinder.universalgcodesender.firmware.fluidnc.commands;

import com.willwinder.universalgcodesender.types.CommandException;
import org.apache.commons.lang3.StringUtils;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.error.YAMLException;

import java.util.AbstractMap;
import java.util.HashMap;
Expand All @@ -40,9 +42,13 @@ public Map<String, String> getSettings() {
}

String response = StringUtils.removeEnd(getResponse(), "ok");
Yaml yaml = new Yaml();
Map<String, Object> settingsTree = yaml.load(response);
return flatten(settingsTree);
try {
Yaml yaml = new Yaml();
Map<String, Object> settingsTree = yaml.load(response);
return flatten(settingsTree);
} catch (YAMLException e) {
throw new CommandException(e);
}
}

private Map<String, String> flatten(Map<String, Object> mapToFlatten) {
Expand All @@ -60,8 +66,7 @@ private Stream<Map.Entry<String, Object>> flatten(Map.Entry<String, Object> entr
}

Object value = entry.getValue();
if (value instanceof Map<?, ?>) {
Map<?, ?> properties = (Map<?, ?>) value;
if (value instanceof Map<?, ?> properties) {
return properties.entrySet().stream()
.flatMap(e -> flatten(new AbstractMap.SimpleEntry<>(entry.getKey().toLowerCase() + "/" + e.getKey().toString().toLowerCase(), e.getValue())));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ This file is part of Universal Gcode Sender (UGS).
import com.willwinder.universalgcodesender.AbstractController;
import com.willwinder.universalgcodesender.Capabilities;
import com.willwinder.universalgcodesender.CapabilitiesConstants;
import com.willwinder.universalgcodesender.ControllerException;
import com.willwinder.universalgcodesender.GrblUtils;
import com.willwinder.universalgcodesender.StatusPollTimer;
import com.willwinder.universalgcodesender.communicator.ICommunicator;
Expand Down Expand Up @@ -134,9 +135,9 @@ protected void isReadyToStreamCommandsEvent() throws Exception {
}

@Override
protected void isReadyToSendCommandsEvent() throws Exception {
protected void isReadyToSendCommandsEvent() throws ControllerException {
if (!this.isReady && !isSmoothieReady) {
throw new Exception(Localization.getString("controller.exception.booting"));
throw new ControllerException(Localization.getString("controller.exception.booting"));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
Copyright 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 <http://www.gnu.org/licenses/>.
*/
package com.willwinder.universalgcodesender.types;

/**
* Exception related to commands and the response parsing
*
* @author Joacim Breiler
*/
public class CommandException extends RuntimeException {
public CommandException(String message) {
super(message);
}

public CommandException(String message, Throwable throwable) {
super(message, throwable);
}

public CommandException(Throwable throwable) {
super(throwable);
}
}

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 Will Winder
Copyright 2022-2024 Will Winder
This file is part of Universal Gcode Sender (UGS).
Expand Down Expand Up @@ -46,9 +46,9 @@ private ControllerUtils() {
* @param controller the controller to send the command through
* @param command a command to send
* @param maxExecutionTime the max number of milliseconds to wait before throwing a timeout error
* @throws Exception if the command could not be sent or a timeout occurred
* @throws InterruptedException if the command could not be sent or a timeout occurred
*/
public static <T extends GcodeCommand> T sendAndWaitForCompletion(IController controller, T command, long maxExecutionTime) throws Exception {
public static <T extends GcodeCommand> T sendAndWaitForCompletion(IController controller, T command, long maxExecutionTime) throws InterruptedException {
final AtomicBoolean isDone = new AtomicBoolean(false);
CommandListener listener = c -> isDone.set(c.isDone());
command.addListener(listener);
Expand All @@ -57,7 +57,7 @@ public static <T extends GcodeCommand> T sendAndWaitForCompletion(IController co
long startTime = System.currentTimeMillis();
while (!isDone.get()) {
if (System.currentTimeMillis() > startTime + maxExecutionTime) {
throw new RuntimeException("The command \"" + command.getCommandString() + "\" has timed out as it wasn't finished within " + maxExecutionTime + "ms");
throw new InterruptedException("The command \"" + command.getCommandString() + "\" has timed out as it wasn't finished within " + maxExecutionTime + "ms");
}
Thread.sleep(10);
}
Expand All @@ -72,10 +72,10 @@ public static <T extends GcodeCommand> T sendAndWaitForCompletion(IController co
* @param command a command
* @param <T> a class extending from {@link GcodeCommand}
* @return the executed command with the response
* @throws Exception if the command could not be sent or a timeout occurred
* @throws InterruptedException if the command could not be sent or a timeout occurred
*/

public static <T extends GcodeCommand> T sendAndWaitForCompletion(IController controller, T command) throws Exception {
public static <T extends GcodeCommand> T sendAndWaitForCompletion(IController controller, T command) throws InterruptedException {
return sendAndWaitForCompletion(controller, command, MAX_EXECUTION_TIME);
}

Expand All @@ -89,7 +89,7 @@ public static void waitOnActiveCommands(IController controller) throws Interrupt
long startTime = System.currentTimeMillis();
while (controller.getActiveCommand().isPresent()) {
if (startTime + maxExecutionTime < System.currentTimeMillis()) {
throw new RuntimeException("The command \"" + controller.getActiveCommand().get().getCommandString() + "\" has timed out as it wasn't finished within " + maxExecutionTime + "ms");
throw new InterruptedException("The command \"" + controller.getActiveCommand().get().getCommandString() + "\" has timed out as it wasn't finished within " + maxExecutionTime + "ms");
}

Thread.sleep(10);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.willwinder.universalgcodesender.firmware.fluidnc.commands;

import com.willwinder.universalgcodesender.types.CommandException;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import org.junit.Test;

import java.util.Map;

import static org.junit.Assert.assertEquals;

public class GetFirmwareSettingsCommandTest {
@Test
public void appendResponseWithYamlConfigShouldParseSettingsAsYaml() {
Expand All @@ -26,4 +28,14 @@ public void appendResponseWithYamlConfigShouldParseSettingsAsYaml() {
assertEquals("800", settings.get("axis/x/steps_per_mm"));
assertEquals("800", settings.get("axis/y/steps_per_mm"));
}

@Test
public void getSettingsShouldThrowExceptionOnFaultyYaml() {
GetFirmwareSettingsCommand command = new GetFirmwareSettingsCommand();
command.appendResponse("meta: a command with nested colons : should not be allowed");
command.appendResponse("ok");

CommandException exception = assertThrows(CommandException.class, command::getSettings);
assertTrue(exception.getMessage().startsWith("mapping values are not allowed here"));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 Will Winder
Copyright 2022-2024 Will Winder
This file is part of Universal Gcode Sender (UGS).
Expand All @@ -23,24 +23,22 @@ This file is part of Universal Gcode Sender (UGS).
import com.willwinder.universalgcodesender.listeners.ControllerState;
import com.willwinder.universalgcodesender.model.CommunicatorState;
import com.willwinder.universalgcodesender.types.GcodeCommand;
import org.junit.Test;

import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import org.junit.Test;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
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 java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

public class ControllerUtilsTest {

@Test
Expand Down Expand Up @@ -75,7 +73,7 @@ public void sendAndWaitForCompletionShouldBlockAndReturnWhenCommandIsDone() thro
public void sendAndWaitForCompletionShouldTimeOut() throws Exception {
IController controller = mock(IController.class);
GcodeCommand command = new GcodeCommand("blah");
assertThrows("The command \"blah\" has timed out as it wasn't finished within 100ms", RuntimeException.class, () -> ControllerUtils.sendAndWaitForCompletion(controller, command, 100));
assertThrows("The command \"blah\" has timed out as it wasn't finished within 100ms", InterruptedException.class, () -> ControllerUtils.sendAndWaitForCompletion(controller, command, 100));
}

@Test
Expand Down

0 comments on commit d7b0749

Please sign in to comment.