From 5fc5bb40bd07cdd174e88d5a9883bbfb1a2203cd Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Fri, 11 Feb 2022 04:03:15 -0800 Subject: [PATCH] executeCommand knows about opcodes (#3915) * executeCommand knows about opcodes * kick * extra line * gross duplication * ugh more --- .../rusefi/binaryprotocol/BinaryProtocol.java | 79 +++++++++++-------- .../com/rusefi/binaryprotocol/IoHelper.java | 19 ++--- .../rusefi/io/commands/GetOutputsCommand.java | 7 +- .../proxy/client/LocalApplicationProxy.java | 6 +- .../rusefi/ui/livedocs/LiveDocsRegistry.java | 9 +-- .../com/rusefi/PerformanceTraceHelper.java | 4 +- .../rusefi/ui/engine/EngineSnifferPanel.java | 2 +- .../client/LocalApplicationProxyTest.java | 6 +- .../server/ControllerConnectionState.java | 6 +- 9 files changed, 76 insertions(+), 62 deletions(-) diff --git a/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocol.java b/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocol.java index e71f893557..e2f9732742 100644 --- a/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocol.java +++ b/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocol.java @@ -282,10 +282,7 @@ public class BinaryProtocol { if (needCompositeLogger) { getComposite(); } else if (isCompositeLoggerEnabled) { - byte[] packet = new byte[2]; - packet[0] = Fields.TS_SET_LOGGER_SWITCH; - packet[1] = Fields.TS_COMPOSITE_DISABLE; - executeCommand(packet, "disable composite"); + executeCommand(Fields.TS_SET_LOGGER_SWITCH, new byte[] { Fields.TS_COMPOSITE_DISABLE }, "disable composite"); isCompositeLoggerEnabled = false; closeComposites(); } @@ -379,12 +376,11 @@ public class BinaryProtocol { int remainingSize = image.getSize() - offset; int requestSize = Math.min(remainingSize, Fields.BLOCKING_FACTOR); - byte[] packet = new byte[5]; - packet[0] = Fields.TS_READ_COMMAND; - putShort(packet, 1, swap16(offset)); - putShort(packet, 3, swap16(requestSize)); + byte[] packet = new byte[4]; + putShort(packet, 0, swap16(offset)); + putShort(packet, 2, swap16(requestSize)); - byte[] response = executeCommand(packet, "load image offset=" + offset); + byte[] response = executeCommand(Fields.TS_READ_COMMAND, packet, "load image offset=" + offset); if (!checkResponseCode(response, (byte) Fields.TS_RESPONSE_OK) || response.length != requestSize + 1) { String code = (response == null || response.length == 0) ? "empty" : "ERROR_CODE=" + getCode(response); @@ -455,7 +451,7 @@ public class BinaryProtocol { public int getCrcFromController(int configSize) { byte[] packet = createCrcCommand(configSize); - byte[] response = executeCommand(packet, "get CRC32"); + byte[] response = executeCommand(Fields.TS_CRC_CHECK_COMMAND, packet, "get CRC32"); if (checkResponseCode(response, (byte) Fields.TS_RESPONSE_OK) && response.length == 5) { ByteBuffer bb = ByteBuffer.wrap(response, 1, 4); @@ -472,15 +468,22 @@ public class BinaryProtocol { } public static byte[] createCrcCommand(int size) { - byte[] packet = new byte[5]; - packet[0] = Fields.TS_CRC_CHECK_COMMAND; - putShort(packet, 1, swap16(/*offset = */ 0)); - putShort(packet, 3, swap16(size)); + byte[] packet = new byte[4]; + putShort(packet, 0, swap16(/*offset = */ 0)); + putShort(packet, 2, swap16(size)); return packet; } - public byte[] executeCommand(byte[] packet, String msg) { - return executeCommand(packet, msg, false); + public byte[] executeCommand(char opcode, String msg) { + return executeCommand(opcode, null, msg); + } + + public byte[] executeCommand(char opcode, String msg, boolean allowLongResponse) { + return executeCommand(opcode, null, msg, allowLongResponse); + } + + public byte[] executeCommand(char opcode, byte[] data, String msg) { + return executeCommand(opcode, data, msg, false); } /** @@ -488,14 +491,26 @@ public class BinaryProtocol { * * @return null in case of IO issues */ - public byte[] executeCommand(byte[] packet, String msg, boolean allowLongResponse) { + public byte[] executeCommand(char opcode, byte[] packet, String msg, boolean allowLongResponse) { if (isClosed) return null; + + byte[] fullRequest; + + if (packet != null) { + fullRequest = new byte[packet.length + 1]; + System.arraycopy(packet, 0, fullRequest, 1, packet.length); + } else { + fullRequest = new byte[1]; + } + + fullRequest[0] = (byte)opcode; + try { linkManager.assertCommunicationThread(); dropPending(); - sendPacket(packet); + sendPacket(fullRequest); return receivePacket(msg, allowLongResponse); } catch (IOException e) { log.error(msg + ": executeCommand failed: " + e); @@ -517,16 +532,15 @@ public class BinaryProtocol { public void writeData(byte[] content, int contentOffset, int ecuOffset, int size) { isBurnPending = true; - byte[] packet = new byte[5 + size]; - packet[0] = Fields.TS_CHUNK_WRITE_COMMAND; - putShort(packet, 1, swap16(ecuOffset)); - putShort(packet, 3, swap16(size)); + byte[] packet = new byte[4 + size]; + putShort(packet, 0, swap16(ecuOffset)); + putShort(packet, 2, swap16(size)); - System.arraycopy(content, contentOffset, packet, 5, size); + System.arraycopy(content, contentOffset, packet, 4, size); long start = System.currentTimeMillis(); while (!isClosed && (System.currentTimeMillis() - start < Timeouts.BINARY_IO_TIMEOUT)) { - byte[] response = executeCommand(packet, "writeImage"); + byte[] response = executeCommand(Fields.TS_CHUNK_WRITE_COMMAND, packet, "writeImage"); if (!checkResponseCode(response, (byte) Fields.TS_RESPONSE_OK) || response.length != 1) { log.error("writeData: Something is wrong, retrying..."); continue; @@ -543,7 +557,7 @@ public class BinaryProtocol { while (true) { if (isClosed) return; - byte[] response = executeCommand(new byte[]{Fields.TS_BURN_COMMAND}, "burn"); + byte[] response = executeCommand(Fields.TS_BURN_COMMAND, "burn"); if (!checkResponseCode(response, (byte) Fields.TS_RESPONSE_BURN_OK) || response.length != 1) { continue; } @@ -579,7 +593,7 @@ public class BinaryProtocol { long start = System.currentTimeMillis(); while (!isClosed && (System.currentTimeMillis() - start < Timeouts.BINARY_IO_TIMEOUT)) { - byte[] response = executeCommand(command, "execute", false); + byte[] response = executeCommand(Fields.TS_EXECUTE, command, "execute", false); if (!checkResponseCode(response, (byte) Fields.TS_RESPONSE_COMMAND_OK) || response.length != 1) { continue; } @@ -590,17 +604,14 @@ public class BinaryProtocol { public static byte[] getTextCommandBytes(String text) { byte[] asBytes = text.getBytes(); - byte[] command = new byte[asBytes.length + 1]; - command[0] = Fields.TS_EXECUTE; - System.arraycopy(asBytes, 0, command, 1, asBytes.length); - return command; + return asBytes; } private String requestPendingMessages() { if (isClosed) return null; try { - byte[] response = executeCommand(new byte[]{Fields.TS_GET_TEXT}, "text", true); + byte[] response = executeCommand(Fields.TS_GET_TEXT, "text", true); if (response != null && response.length == 1) Thread.sleep(100); return new String(response, 1, response.length - 1); @@ -614,13 +625,11 @@ public class BinaryProtocol { if (isClosed) return; - byte[] packet = new byte[1]; - packet[0] = Fields.TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY; // get command would enable composite logging in controller but we need to turn it off from our end // todo: actually if console gets disconnected composite logging might end up enabled in controller? isCompositeLoggerEnabled = true; - byte[] response = executeCommand(packet, "composite log", true); + byte[] response = executeCommand(Fields.TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY, "composite log", true); if (checkResponseCode(response, (byte) Fields.TS_RESPONSE_OK)) { List events = CompositeParser.parse(response); createCompositesIfNeeded(); @@ -635,7 +644,7 @@ public class BinaryProtocol { byte[] packet = GetOutputsCommand.createRequest(); - byte[] response = executeCommand(packet, "output channels", false); + byte[] response = executeCommand(Fields.TS_OUTPUT_COMMAND, packet, "output channels", false); if (response == null || response.length != (Fields.TS_OUTPUT_SIZE + 1) || response[0] != Fields.TS_RESPONSE_OK) return false; diff --git a/java_console/io/src/main/java/com/rusefi/binaryprotocol/IoHelper.java b/java_console/io/src/main/java/com/rusefi/binaryprotocol/IoHelper.java index d9a04edf81..79dfb48588 100644 --- a/java_console/io/src/main/java/com/rusefi/binaryprotocol/IoHelper.java +++ b/java_console/io/src/main/java/com/rusefi/binaryprotocol/IoHelper.java @@ -36,8 +36,7 @@ public class IoHelper { log.info("makeCrc32Packet: raw packet " + IoStream.printByteArray(command)); byte[] packet = new byte[command.length + 6]; - packet[0] = (byte) (command.length / 256); - packet[1] = (byte) command.length; + putShort(packet, 0, command.length); System.arraycopy(command, 0, packet, 2, command.length); int crc = getCrc32(command); @@ -57,19 +56,15 @@ public class IoHelper { } public static void putInt(byte[] packet, int offset, int value) { - int index = offset + 3; - for (int i = 0; i < 4; i++) { - packet[index--] = (byte) value; - value >>= 8; - } + packet[offset + 3] = (byte) value; + packet[offset + 2] = (byte) (value >> 8); + packet[offset + 1] = (byte) (value >> 16); + packet[offset] = (byte) (value >> 24); } public static void putShort(byte[] packet, int offset, int value) { - int index = offset + 1; - for (int i = 0; i < 2; i++) { - packet[index--] = (byte) value; - value >>= 8; - } + packet[offset + 1] = (byte) value; + packet[offset] = (byte) (value >> 8); } /** diff --git a/java_console/io/src/main/java/com/rusefi/io/commands/GetOutputsCommand.java b/java_console/io/src/main/java/com/rusefi/io/commands/GetOutputsCommand.java index ba47d12e52..5b8cca0645 100644 --- a/java_console/io/src/main/java/com/rusefi/io/commands/GetOutputsCommand.java +++ b/java_console/io/src/main/java/com/rusefi/io/commands/GetOutputsCommand.java @@ -11,10 +11,9 @@ import static com.rusefi.binaryprotocol.IoHelper.swap16; public class GetOutputsCommand { public static byte[] createRequest() { - byte[] packet = new byte[5]; - packet[0] = Fields.TS_OUTPUT_COMMAND; - putShort(packet, 1, 0); // offset - putShort(packet, 3, swap16(Fields.TS_OUTPUT_SIZE)); + byte[] packet = new byte[4]; + putShort(packet, 0, 0); // offset + putShort(packet, 2, swap16(Fields.TS_OUTPUT_SIZE)); return packet; } diff --git a/java_console/io/src/main/java/com/rusefi/proxy/client/LocalApplicationProxy.java b/java_console/io/src/main/java/com/rusefi/proxy/client/LocalApplicationProxy.java index 4275020081..62ccf861bb 100644 --- a/java_console/io/src/main/java/com/rusefi/proxy/client/LocalApplicationProxy.java +++ b/java_console/io/src/main/java/com/rusefi/proxy/client/LocalApplicationProxy.java @@ -113,7 +113,11 @@ public class LocalApplicationProxy implements Closeable { while (true) { sleep(context.gaugePokingPeriod()); if (isTimeForApplicationToConnect(lastActivity.get(), BINARY_IO_TIMEOUT / 2)) { - byte[] commandPacket = GetOutputsCommand.createRequest(); + // TODO: why is this logic duplicated from BinaryProtocol? + byte[] commandPacket = new byte[5]; + commandPacket[0] = Fields.TS_OUTPUT_COMMAND; + System.arraycopy(GetOutputsCommand.createRequest(), 0, commandPacket, 1, 4); + // we do not really need the data, we just need to take response from the socket authenticatorToProxyStream.sendAndGetPacket(commandPacket, "Gauge Poker", false); } diff --git a/java_console/io/src/main/java/com/rusefi/ui/livedocs/LiveDocsRegistry.java b/java_console/io/src/main/java/com/rusefi/ui/livedocs/LiveDocsRegistry.java index 25bba55d76..8ea73268d8 100644 --- a/java_console/io/src/main/java/com/rusefi/ui/livedocs/LiveDocsRegistry.java +++ b/java_console/io/src/main/java/com/rusefi/ui/livedocs/LiveDocsRegistry.java @@ -50,12 +50,11 @@ public enum LiveDocsRegistry { return context -> { Field[] values = StateDictionary.INSTANCE.getFields(context); int size = Field.getStructureSize(values); - byte[] packet = new byte[5]; - packet[0] = Fields.TS_GET_STRUCT; - putShort(packet, 1, swap16(context.ordinal())); // offset - putShort(packet, 3, swap16(size)); + byte[] packet = new byte[4]; + putShort(packet, 0, swap16(context.ordinal())); // offset + putShort(packet, 2, swap16(size)); - byte[] responseWithCode = binaryProtocol.executeCommand(packet, "get LiveDoc"); + byte[] responseWithCode = binaryProtocol.executeCommand(Fields.TS_GET_STRUCT, packet, "get LiveDoc"); if (responseWithCode == null || responseWithCode.length != (size + 1) || responseWithCode[0] != Fields.TS_RESPONSE_OK) return null; diff --git a/java_console/ui/src/main/java/com/rusefi/PerformanceTraceHelper.java b/java_console/ui/src/main/java/com/rusefi/PerformanceTraceHelper.java index 9ebd729655..c4358676e1 100644 --- a/java_console/ui/src/main/java/com/rusefi/PerformanceTraceHelper.java +++ b/java_console/ui/src/main/java/com/rusefi/PerformanceTraceHelper.java @@ -21,12 +21,12 @@ public class PerformanceTraceHelper { JOptionPane.showMessageDialog(parent, "Failed to located serial ports"); return; } - bp.executeCommand(new byte[]{Fields.TS_PERF_TRACE_BEGIN}, "begin trace"); + bp.executeCommand(Fields.TS_PERF_TRACE_BEGIN, "begin trace"); try { Thread.sleep(500); - byte[] packet = bp.executeCommand(new byte[]{Fields.TS_PERF_TRACE_GET_BUFFER}, "get trace", true); + byte[] packet = bp.executeCommand(Fields.TS_PERF_TRACE_GET_BUFFER, "get trace", true); if (!checkResponseCode(packet, (byte) Fields.TS_RESPONSE_OK) || ((packet.length - 1) % 8) != 0) throw new IllegalStateException("Unexpected packet, length=" + (packet != null ? 0 : packet.length)); diff --git a/java_console/ui/src/main/java/com/rusefi/ui/engine/EngineSnifferPanel.java b/java_console/ui/src/main/java/com/rusefi/ui/engine/EngineSnifferPanel.java index f040d5549c..1dce13bc2b 100644 --- a/java_console/ui/src/main/java/com/rusefi/ui/engine/EngineSnifferPanel.java +++ b/java_console/ui/src/main/java/com/rusefi/ui/engine/EngineSnifferPanel.java @@ -240,7 +240,7 @@ public class EngineSnifferPanel { int index = getInsertIndex(name, images.keySet()); - FileLog.MAIN.logLine("Registering " + name + "@" + index); + FileLog.MAIN.logLine("Engine sniffer register channel " + name + " at idx " + index); UpDownImage image = createImage(name); images.put(name, image); diff --git a/java_console/ui/src/test/java/com/rusefi/proxy/client/LocalApplicationProxyTest.java b/java_console/ui/src/test/java/com/rusefi/proxy/client/LocalApplicationProxyTest.java index 6bea9bc3fe..a955135066 100644 --- a/java_console/ui/src/test/java/com/rusefi/proxy/client/LocalApplicationProxyTest.java +++ b/java_console/ui/src/test/java/com/rusefi/proxy/client/LocalApplicationProxyTest.java @@ -91,7 +91,11 @@ public class LocalApplicationProxyTest { applicationConnection.getDataBuffer().read(protocolResponse); assertArrayEquals(protocolResponse, TS_PROTOCOL.getBytes()); - byte[] commandPacket = GetOutputsCommand.createRequest(); + // TODO: why is this logic duplicated from BinaryProtocol? + byte[] commandPacket = new byte[5]; + commandPacket[0] = Fields.TS_OUTPUT_COMMAND; + System.arraycopy(GetOutputsCommand.createRequest(), 0, commandPacket, 1, 4); + applicationConnection.sendPacket(commandPacket); BinaryProtocolServer.Packet response = applicationConnection.readPacket(); assertEquals(Fields.TS_OUTPUT_SIZE + 1, response.getPacket().length); diff --git a/java_tools/proxy_server/src/main/java/com/rusefi/server/ControllerConnectionState.java b/java_tools/proxy_server/src/main/java/com/rusefi/server/ControllerConnectionState.java index 924f219173..8d5a54e282 100644 --- a/java_tools/proxy_server/src/main/java/com/rusefi/server/ControllerConnectionState.java +++ b/java_tools/proxy_server/src/main/java/com/rusefi/server/ControllerConnectionState.java @@ -128,7 +128,11 @@ public class ControllerConnectionState { } public void getOutputs() throws IOException { - byte[] commandPacket = GetOutputsCommand.createRequest(); + // TODO: why is this logic duplicated from BinaryProtocol? + byte[] commandPacket = new byte[5]; + commandPacket[0] = Fields.TS_OUTPUT_COMMAND; + System.arraycopy(GetOutputsCommand.createRequest(), 0, commandPacket, 1, 4); + long start = System.currentTimeMillis(); stream.sendPacket(commandPacket);