diff --git a/firmware/console/binary/ts_can_channel.cpp b/firmware/console/binary/ts_can_channel.cpp index 9502bf2a5c..278637d1d5 100644 --- a/firmware/console/binary/ts_can_channel.cpp +++ b/firmware/console/binary/ts_can_channel.cpp @@ -34,7 +34,7 @@ public: void stop() override; // Special override for writeCrcPacket for small packets - void writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size) override; + void writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size, bool allowLongPackets = false) override; }; @@ -52,7 +52,7 @@ void CanTsChannel::start() { void CanTsChannel::stop() { } -void CanTsChannel::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size) { +void CanTsChannel::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size, bool allowLongPackets) { #ifdef TS_CAN_DEVICE_SHORT_PACKETS_IN_ONE_FRAME // a special case for short packets: we can send them in 1 frame, without CRC & size, // because the CAN protocol is already protected by its own checksum. @@ -67,7 +67,7 @@ void CanTsChannel::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size #endif /* TS_CAN_DEVICE_SHORT_PACKETS_IN_ONE_FRAME */ // Packet too large, use default implementation - TsChannelBase::writeCrcPacket(responseCode, buf, size); + TsChannelBase::writeCrcPacket(responseCode, buf, size, allowLongPackets); } void CanTsChannel::write(const uint8_t* buffer, size_t size, bool) { diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index 88f06fd377..5dcb4f5107 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -608,7 +608,7 @@ static void handleGetText(TsChannelBase* tsChannel) { logMsg("get test sending [%d]\r\n", outputSize); #endif - tsChannel->writeCrcPacket(TS_RESPONSE_COMMAND_OK, reinterpret_cast(output), outputSize); + tsChannel->writeCrcPacket(TS_RESPONSE_COMMAND_OK, reinterpret_cast(output), outputSize, true); #if EFI_SIMULATOR logMsg("sent [%d]\r\n", outputSize); #endif @@ -775,14 +775,14 @@ int TunerStudioBase::handleCrcCommand(TsChannelBase* tsChannel, char *data, int if (currentEnd > transmitted) { // more normal case - tail after head - tsChannel->sendResponse(TS_CRC, start, COMPOSITE_PACKET_SIZE * (currentEnd - transmitted)); + tsChannel->sendResponse(TS_CRC, start, COMPOSITE_PACKET_SIZE * (currentEnd - transmitted), true); transmitted = currentEnd; } else if (currentEnd == transmitted) { tsChannel->sendResponse(TS_CRC, start, 0); } else { // we are here if tail of buffer has reached the end of buffer and re-started from the start of buffer // sending end of the buffer, next transmission would take care of the rest - tsChannel->sendResponse(TS_CRC, start, COMPOSITE_PACKET_SIZE * (COMPOSITE_PACKET_COUNT - transmitted)); + tsChannel->sendResponse(TS_CRC, start, COMPOSITE_PACKET_SIZE * (COMPOSITE_PACKET_COUNT - transmitted), true); transmitted = 0; } } @@ -790,7 +790,7 @@ int TunerStudioBase::handleCrcCommand(TsChannelBase* tsChannel, char *data, int case TS_GET_LOGGER_GET_BUFFER: { auto toothBuffer = GetToothLoggerBuffer(); - tsChannel->sendResponse(TS_CRC, toothBuffer.Buffer, toothBuffer.Length); + tsChannel->sendResponse(TS_CRC, toothBuffer.Buffer, toothBuffer.Length, true); } break; @@ -803,7 +803,7 @@ int TunerStudioBase::handleCrcCommand(TsChannelBase* tsChannel, char *data, int case TS_PERF_TRACE_GET_BUFFER: { auto trace = perfTraceGetBuffer(); - tsChannel->sendResponse(TS_CRC, trace.Buffer, trace.Size); + tsChannel->sendResponse(TS_CRC, trace.Buffer, trace.Size, true); } break; @@ -816,7 +816,7 @@ int TunerStudioBase::handleCrcCommand(TsChannelBase* tsChannel, char *data, int strcpy((char*)configError, "FACTORY_MODE_PLEASE_CONTACT_SUPPORT"); } #endif // HW_CHECK_MODE - tsChannel->sendResponse(TS_CRC, reinterpret_cast(configError), strlen(configError)); + tsChannel->sendResponse(TS_CRC, reinterpret_cast(configError), strlen(configError), true); break; } default: diff --git a/firmware/console/binary/tunerstudio_io.cpp b/firmware/console/binary/tunerstudio_io.cpp index dfc2d382a1..abcc0b211e 100644 --- a/firmware/console/binary/tunerstudio_io.cpp +++ b/firmware/console/binary/tunerstudio_io.cpp @@ -82,24 +82,33 @@ TsChannelBase::TsChannelBase(const char *name) { /** * Adds size to the beginning of a packet and a crc32 at the end. Then send the packet. */ -void TsChannelBase::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size) { +void TsChannelBase::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size, bool allowLongPackets) { // don't transmit a null buffer... if (!buf) { size = 0; } - if (size <= BLOCKING_FACTOR + 7) { - // for small packets we use a buffer for CRC calculation - writeCrcPacketSmall(responseCode, buf, size); - } else { + bool isBigPacket = size > BLOCKING_FACTOR + 7; + + if (isBigPacket && !allowLongPackets) { + // TODO: turn this in to an error once console output channel reads are chunked! + // Hardware CI may fail before that time. + //firmwareError(OBD_PCM_Processor_Fault, "tried to send disallowed long packet of size %d", size); + warning(OBD_PCM_Processor_Fault, "tried to send disallowed long packet of size %d", size); + } + + if (isBigPacket) { // for larger packets we do not use a buffer for CRC calculation meaning data is now allowed to modify while pending writeCrcPacketLarge(responseCode, buf, size); + } else { + // for small packets we use a buffer for CRC calculation + writeCrcPacketSmall(responseCode, buf, size); } } -void TsChannelBase::sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size) { +void TsChannelBase::sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size, bool allowLongPackets /* = false */) { if (mode == TS_CRC) { - writeCrcPacket(TS_RESPONSE_OK, buffer, size); + writeCrcPacket(TS_RESPONSE_OK, buffer, size, allowLongPackets); } else { if (size > 0) { write(buffer, size, true); diff --git a/firmware/console/binary/tunerstudio_io.h b/firmware/console/binary/tunerstudio_io.h index 879416c825..ecd4d1868c 100644 --- a/firmware/console/binary/tunerstudio_io.h +++ b/firmware/console/binary/tunerstudio_io.h @@ -53,8 +53,8 @@ public: #ifdef EFI_CAN_SERIAL virtual // CAN device needs this function to be virtual for small-packet optimization #endif - void writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size); - void sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size); + void writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size, bool allowLongPackets = false); + void sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size, bool allowLongPackets = false); /** * See 'blockingFactor' in rusefi.ini diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index 74d390514d..9ccf339d94 100644 --- a/firmware/integration/rusefi_config.txt +++ b/firmware/integration/rusefi_config.txt @@ -2004,3 +2004,4 @@ end_struct #define CAN_ECU_SERIAL_RX_ID 0x100 #define CAN_ECU_SERIAL_TX_ID 0x102 + 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 f980a48f5c..b902b5e266 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 @@ -258,10 +258,10 @@ public class BinaryProtocol { setController(newVersion); } - private byte[] receivePacket(String msg, boolean allowLongResponse) throws IOException { + private byte[] receivePacket(String msg) throws IOException { long start = System.currentTimeMillis(); synchronized (ioLock) { - return incomingData.getPacket(msg, allowLongResponse, start); + return incomingData.getPacket(msg, start); } } @@ -408,20 +408,12 @@ public class BinaryProtocol { 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); - } - /** * Blocking sending binary packet and waiting for a response * * @return null in case of IO issues */ - public byte[] executeCommand(char opcode, byte[] packet, String msg, boolean allowLongResponse) { + public byte[] executeCommand(char opcode, byte[] packet, String msg) { if (isClosed) return null; @@ -441,7 +433,7 @@ public class BinaryProtocol { dropPending(); sendPacket(fullRequest); - return receivePacket(msg, allowLongResponse); + return receivePacket(msg); } catch (IOException e) { log.error(msg + ": executeCommand failed: " + e); close(); @@ -510,7 +502,6 @@ public class BinaryProtocol { stream.sendPacket(command); } - /** * This method blocks until a confirmation is received or {@link Timeouts#BINARY_IO_TIMEOUT} is reached * @@ -521,7 +512,7 @@ public class BinaryProtocol { long start = System.currentTimeMillis(); while (!isClosed && (System.currentTimeMillis() - start < Timeouts.BINARY_IO_TIMEOUT)) { - byte[] response = executeCommand(Fields.TS_EXECUTE, command, "execute", false); + byte[] response = executeCommand(Fields.TS_EXECUTE, command, "execute"); if (!checkResponseCode(response, (byte) Fields.TS_RESPONSE_COMMAND_OK) || response.length != 1) { continue; } @@ -539,7 +530,7 @@ public class BinaryProtocol { if (isClosed) return null; try { - byte[] response = executeCommand(Fields.TS_GET_TEXT, "text", true); + byte[] response = executeCommand(Fields.TS_GET_TEXT, "text"); if (response != null && response.length == 1) Thread.sleep(100); return new String(response, 1, response.length - 1); @@ -555,7 +546,7 @@ public class BinaryProtocol { byte[] packet = GetOutputsCommand.createRequest(tsOutputSize); - byte[] response = executeCommand(Fields.TS_OUTPUT_COMMAND, packet, "output channels", false); + byte[] response = executeCommand(Fields.TS_OUTPUT_COMMAND, packet, "output channels"); if (response == null || response.length != (tsOutputSize + 1) || response[0] != Fields.TS_RESPONSE_OK) return false; diff --git a/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocolLogger.java b/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocolLogger.java index e3581b6862..ca5442f582 100644 --- a/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocolLogger.java +++ b/java_console/io/src/main/java/com/rusefi/binaryprotocol/BinaryProtocolLogger.java @@ -94,7 +94,7 @@ public class BinaryProtocolLogger { // todo: actually if console gets disconnected composite logging might end up enabled in controller? isCompositeLoggerEnabled = true; - byte[] response = binaryProtocol.executeCommand(Fields.TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY, "composite log", true); + byte[] response = binaryProtocol.executeCommand(Fields.TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY, "composite log"); if (checkResponseCode(response, (byte) Fields.TS_RESPONSE_OK)) { List events = CompositeParser.parse(response); createCompositesIfNeeded(); diff --git a/java_console/io/src/main/java/com/rusefi/binaryprotocol/IncomingDataBuffer.java b/java_console/io/src/main/java/com/rusefi/binaryprotocol/IncomingDataBuffer.java index e50d7ca9a5..d9f27e84ba 100644 --- a/java_console/io/src/main/java/com/rusefi/binaryprotocol/IncomingDataBuffer.java +++ b/java_console/io/src/main/java/com/rusefi/binaryprotocol/IncomingDataBuffer.java @@ -31,31 +31,27 @@ public class IncomingDataBuffer { private static final int BUFFER_SIZE = 32768; private final String loggingPrefix; + /** - * buffer for response bytes from controller + * buffer for queued response bytes from controller */ - private final CircularByteBuffer cbb; + private final CircularByteBuffer cbb = new CircularByteBuffer(BUFFER_SIZE); private final AbstractIoStream.StreamStats streamStats; public IncomingDataBuffer(String loggingPrefix, AbstractIoStream.StreamStats streamStats) { this.loggingPrefix = loggingPrefix; this.streamStats = Objects.requireNonNull(streamStats, "streamStats"); - this.cbb = new CircularByteBuffer(BUFFER_SIZE); } public byte[] getPacket(String msg) throws EOFException { - return getPacket(msg, false, System.currentTimeMillis()); - } - - public byte[] getPacket(String msg, boolean allowLongResponse) throws EOFException { - return getPacket(msg, allowLongResponse, System.currentTimeMillis()); + return getPacket(msg, System.currentTimeMillis()); } /** * why does this method return NULL in case of timeout?! * todo: there is a very similar BinaryProtocolServer#readPromisedBytes which throws exception in case of timeout */ - public byte[] getPacket(String msg, boolean allowLongResponse, long start) throws EOFException { + public byte[] getPacket(String msg, long start) throws EOFException { boolean isTimeout = waitForBytes(msg + " header", start, 2); if (isTimeout) return null; @@ -65,8 +61,6 @@ public class IncomingDataBuffer { // log.debug(loggingPrefix + "Got packet size " + packetSize); if (packetSize < 0) return null; - if (!allowLongResponse && packetSize > Math.max(Fields.BLOCKING_FACTOR, Fields.TS_OUTPUT_SIZE) + 10) - throw new IllegalArgumentException(packetSize + " packet while not allowLongResponse"); // this is about CRC calculation and mutable buffers on firmware side isTimeout = waitForBytes(loggingPrefix + msg + " body", start, packetSize + 4); if (isTimeout) @@ -74,14 +68,17 @@ public class IncomingDataBuffer { byte[] packet = new byte[packetSize]; getData(packet); + + // Compare the sent and computed CRCs, make sure they match! int packetCrc = swap32(getInt()); int actualCrc = getCrc32(packet); - - boolean isCrcOk = actualCrc == packetCrc; - if (!isCrcOk) { - log.error(String.format("CRC mismatch %x: vs %x", actualCrc, packetCrc)); + if (actualCrc != packetCrc) { + String errorMessage = String.format("CRC mismatch on recv packet for %s: got %x but expected %x", msg, actualCrc, packetCrc); + System.out.println(errorMessage); + log.warn(errorMessage); return null; } + onPacketArrived(); // if (log.debugEnabled()) // log.trace("packet arrived: " + Arrays.toString(packet) + ": crc OK"); diff --git a/java_console/io/src/main/java/com/rusefi/io/IoStream.java b/java_console/io/src/main/java/com/rusefi/io/IoStream.java index 61795078f1..1b6793d44b 100644 --- a/java_console/io/src/main/java/com/rusefi/io/IoStream.java +++ b/java_console/io/src/main/java/com/rusefi/io/IoStream.java @@ -101,11 +101,11 @@ public interface IoStream extends WriteStream, Closeable, StreamStatistics { return getDataBuffer().readShort(); } - default byte[] sendAndGetPacket(byte[] packet, String message, boolean allowLongResponse) throws IOException { + default byte[] sendAndGetPacket(byte[] packet, String message) throws IOException { // synchronization is needed for example to help SD card download to live with gauge poker synchronized (this) { sendPacket(packet); - return getDataBuffer().getPacket(message, allowLongResponse); + return getDataBuffer().getPacket(message); } } } diff --git a/java_console/io/src/main/java/com/rusefi/io/commands/HelloCommand.java b/java_console/io/src/main/java/com/rusefi/io/commands/HelloCommand.java index d73287f88b..12ce19516b 100644 --- a/java_console/io/src/main/java/com/rusefi/io/commands/HelloCommand.java +++ b/java_console/io/src/main/java/com/rusefi/io/commands/HelloCommand.java @@ -24,7 +24,7 @@ public class HelloCommand implements Command { @Nullable public static String getHelloResponse(IncomingDataBuffer incomingData) throws EOFException { - byte[] response = incomingData.getPacket("[hello]", true); + byte[] response = incomingData.getPacket("[hello]"); if (!checkResponseCode(response, (byte) Fields.TS_RESPONSE_OK)) return null; return new String(response, 1, response.length - 1); 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 62ccf861bb..576836ab26 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 @@ -119,7 +119,7 @@ public class LocalApplicationProxy implements Closeable { 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); + authenticatorToProxyStream.sendAndGetPacket(commandPacket, "Gauge Poker"); } if (isTimeForApplicationToConnect(lastActivity.get(), context.startUpIdle())) { 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 c4358676e1..fa27781fb3 100644 --- a/java_console/ui/src/main/java/com/rusefi/PerformanceTraceHelper.java +++ b/java_console/ui/src/main/java/com/rusefi/PerformanceTraceHelper.java @@ -26,7 +26,7 @@ public class PerformanceTraceHelper { try { Thread.sleep(500); - byte[] packet = bp.executeCommand(Fields.TS_PERF_TRACE_GET_BUFFER, "get trace", true); + byte[] packet = bp.executeCommand(Fields.TS_PERF_TRACE_GET_BUFFER, "get trace"); 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/tools/ConsoleTools.java b/java_console/ui/src/main/java/com/rusefi/tools/ConsoleTools.java index 2fc8bf04c7..63fd41496b 100644 --- a/java_console/ui/src/main/java/com/rusefi/tools/ConsoleTools.java +++ b/java_console/ui/src/main/java/com/rusefi/tools/ConsoleTools.java @@ -358,13 +358,13 @@ public class ConsoleTools { byte[] commandBytes = BinaryProtocol.getTextCommandBytes("hello"); stream.sendPacket(commandBytes); // skipping response - incomingData.getPacket("", true); + incomingData.getPacket(""); sleep(300); stream.sendPacket(new byte[]{Fields.TS_GET_TEXT}); sleep(300); - byte[] response = incomingData.getPacket("", true); + byte[] response = incomingData.getPacket(""); if (response == null) { System.out.println("No response"); return; 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 8d5a54e282..dc6a57f6b7 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 @@ -136,7 +136,7 @@ public class ControllerConnectionState { long start = System.currentTimeMillis(); stream.sendPacket(commandPacket); - byte[] packet = incomingData.getPacket("msg", true); + byte[] packet = incomingData.getPacket("msg"); outputRoundAroundDuration = (int) (System.currentTimeMillis() - start); if (packet == null) throw new IOException("getOutputs: No response");