move allowLongResponse option to ECU side (#3917)

* executeCommand knows about opcodes

* kick

* remove allowLongResponse

* add api in firmware instead

* test size

* bad merge

* firmware missed a spot

* fix logic

* haha we have to disable it
This commit is contained in:
Matthew Kennedy 2022-02-11 13:03:20 -08:00 committed by GitHub
parent d475b4ce89
commit 35c4c0bfba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 56 additions and 58 deletions

View File

@ -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) {

View File

@ -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<const uint8_t*>(output), outputSize);
tsChannel->writeCrcPacket(TS_RESPONSE_COMMAND_OK, reinterpret_cast<const uint8_t*>(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<const uint8_t*>(configError), strlen(configError));
tsChannel->sendResponse(TS_CRC, reinterpret_cast<const uint8_t*>(configError), strlen(configError), true);
break;
}
default:

View File

@ -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);

View File

@ -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

View File

@ -2004,3 +2004,4 @@ end_struct
#define CAN_ECU_SERIAL_RX_ID 0x100
#define CAN_ECU_SERIAL_TX_ID 0x102

View File

@ -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;

View File

@ -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<CompositeEvent> events = CompositeParser.parse(response);
createCompositesIfNeeded();

View File

@ -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");

View File

@ -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);
}
}
}

View File

@ -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);

View File

@ -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())) {

View File

@ -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));

View File

@ -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;

View File

@ -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");