From 3c57ebe5a4eca6cd83fc4cf4d7400604adc13e64 Mon Sep 17 00:00:00 2001 From: Jessy Diamond Exum Date: Wed, 9 Aug 2017 17:04:19 -0700 Subject: [PATCH] ELM327: Fixed crash. Support for arbitrary length replies Added limited throughput test. --- boardesp/elm327.c | 61 +++++++++++++++++++++++++------------ tests/automated/elm_wifi.py | 31 +++++++++++++++++++ tests/elm_car_simulator.py | 8 ++++- 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/boardesp/elm327.c b/boardesp/elm327.c index 8a82cbc6..813f1d27 100644 --- a/boardesp/elm327.c +++ b/boardesp/elm327.c @@ -45,7 +45,7 @@ static uint16 stripped_msg_len = 0; static char in_msg[0x100]; static uint16 in_msg_len = 0; -static char rsp_buff[0x200]; +static char rsp_buff[536]; //TCP min MTU static uint16 rsp_buff_len = 0; static bool elm_mode_echo = true; @@ -173,13 +173,6 @@ static int ICACHE_FLASH_ATTR panda_usbemu_can_write(bool ext, uint32_t addr, return returned_count; } -static int ICACHE_FLASH_ATTR panda_usbemu_can_read(panda_can_msg_t **can_msgs) { - int returned_count = spi_comm((uint8_t *)((const uint16 []){1,0}), 4, recvData, 0x40); - if(returned_count > 0x40) return 0; - *can_msgs = (panda_can_msg_t*)(recvData+1); - return returned_count/sizeof(panda_can_msg_t); -} - static int8_t ICACHE_FLASH_ATTR elm_decode_hex_char(char b){ if(b >= '0' && b <= '9') return b - '0'; if(b >= 'A' && b <= 'F') return (b - 'A') + 10; @@ -233,8 +226,11 @@ static int ICACHE_FLASH_ATTR elm_msg_is_at_cmd(char *data, uint16_t len){ } static void ICACHE_FLASH_ATTR elm_append_rsp(char *data, uint16_t len) { - if(rsp_buff_len + len > sizeof(rsp_buff)) + uint16_t overflow_len = 0; + if(rsp_buff_len + len > sizeof(rsp_buff)) { + overflow_len = rsp_buff_len + len - sizeof(rsp_buff); len = sizeof(rsp_buff) - rsp_buff_len; + } if(!elm_mode_linefeed) { memcpy(rsp_buff + rsp_buff_len, data, len); rsp_buff_len += len; @@ -245,6 +241,11 @@ static void ICACHE_FLASH_ATTR elm_append_rsp(char *data, uint16_t len) { rsp_buff[rsp_buff_len++] = '\n'; } } + if(overflow_len) { + os_printf("Packet full, sending\n"); + elm_tcp_tx_flush(); + elm_append_rsp(data + len, overflow_len); + } } #define elm_append_rsp_const(str) elm_append_rsp(str, sizeof(str)-1) @@ -265,7 +266,6 @@ static void ICACHE_FLASH_ATTR elm_append_in_msg(char *data, uint16_t len) { void ICACHE_FLASH_ATTR elm_append_rsp_can_msg_addr(panda_can_msg_t *recv) { //Show address uint32_t addr = panda_get_can_addr(recv); - os_printf("Printing can address: %08x\n", addr); if(recv->ext){ elm_append_rsp_hex_byte(addr>>24); elm_append_rsp_hex_byte(addr>>16); @@ -277,17 +277,31 @@ void ICACHE_FLASH_ATTR elm_append_rsp_can_msg_addr(panda_can_msg_t *recv) { } } +#define LOOPCOUNT_FULL 4 int loopcount = 0; static volatile os_timer_t elm_timeout; -bool did_multimessage = 0; +bool did_multimessage = false; +bool got_msg_this_run = false; void ICACHE_FLASH_ATTR elm_timer_cb(void *arg){ loopcount--; if(loopcount>0) { for(int pass = 0; pass < 16 && loopcount; pass++){ panda_can_msg_t *can_msgs; - int num_can_msgs = panda_usbemu_can_read(&can_msgs); + + + int returned_count = spi_comm((uint8_t *)((const uint16 []){1,0}), 4, recvData, 0x40); + //os_printf("returned count = %d\n", returned_count); + if(returned_count > 0x40 || returned_count < 0){ + os_printf("CAN read got invalid length\n"); + continue; + } + can_msgs = (panda_can_msg_t*)(recvData+1); + int num_can_msgs = returned_count/sizeof(panda_can_msg_t); + os_printf("Received %d can messages\n", num_can_msgs); + + //int num_can_msgs = panda_usbemu_can_read(&can_msgs); if(!num_can_msgs) break; for(int i = 0; i < num_can_msgs; i++){ @@ -301,8 +315,9 @@ void ICACHE_FLASH_ATTR elm_timer_cb(void *arg){ //TODO make elm only print messages that have the same PID if ((panda_get_can_addr(recv) & 0x7F8) == 0x7E8 && recv->len == 8) { if(recv->data[0] <= 7) { - os_printf("Found matching message, index: %d\n", i); - loopcount = 0; + got_msg_this_run = true; + loopcount = LOOPCOUNT_FULL; + os_printf(" CAN msg response, index: %d\n", i); if(elm_mode_additional_headers){ elm_append_rsp_can_msg_addr(recv); @@ -311,15 +326,18 @@ void ICACHE_FLASH_ATTR elm_timer_cb(void *arg){ for(int j = 1; j < recv->data[0]+1; j++) elm_append_rsp_hex_byte(recv->data[j]); } - elm_append_rsp_const("\r\r>"); + //elm_append_rsp_const("\r\r>"); + elm_append_rsp_const("\r"); elm_tcp_tx_flush(); - return; + //return; } else if((recv->data[0] & 0xF0) == 0x10) { + got_msg_this_run = true; + loopcount = LOOPCOUNT_FULL; panda_usbemu_can_write(0, 0x7E0 | (panda_get_can_addr(recv)&0x7), "\x30\x00\x00", 3); did_multimessage = 1; - os_printf("Found matching multi message, index: %d, len %d\n", i, + os_printf(" CAN multimsg start response, index: %d, len %d\n", i, ((recv->data[0]&0xF)<<8) | recv->data[1]); if(!elm_mode_additional_headers) { @@ -337,7 +355,9 @@ void ICACHE_FLASH_ATTR elm_timer_cb(void *arg){ elm_tcp_tx_flush(); } else if (did_multimessage && (recv->data[0] & 0xF0) == 0x20) { - os_printf("Found extra multi message data, index: %d\n", i); + got_msg_this_run = true; + loopcount = LOOPCOUNT_FULL; + os_printf(" CAN multimsg data response, index: %d\n", i); if(!elm_mode_additional_headers) { elm_append_rsp(&hex_lookup[recv->data[0] & 0xF], 1); @@ -358,11 +378,12 @@ void ICACHE_FLASH_ATTR elm_timer_cb(void *arg){ if(did_multimessage) { os_printf("End of multi message\n"); did_multimessage = 0; - } else { + } else if(!got_msg_this_run) { os_printf("No data collected\n"); //elm_append_rsp_const("UNABLE TO CONNECT\r\r>"); elm_append_rsp_const("NO DATA\r"); } + got_msg_this_run = false; elm_append_rsp_const("\r>"); elm_tcp_tx_flush(); } @@ -598,7 +619,7 @@ static void ICACHE_FLASH_ATTR elm_process_obd_cmd(char *cmd, uint16_t len) { //elm_append("SEARCHING...\r"); os_printf("Starting up timer\n"); - loopcount = 4; + loopcount = LOOPCOUNT_FULL; os_timer_disarm(&elm_timeout); os_timer_setfn(&elm_timeout, (os_timer_func_t *)elm_timer_cb, NULL); os_timer_arm(&elm_timeout, elm_mode_timeout, 0); diff --git a/tests/automated/elm_wifi.py b/tests/automated/elm_wifi.py index 35c72341..2c1180e4 100644 --- a/tests/automated/elm_wifi.py +++ b/tests/automated/elm_wifi.py @@ -139,6 +139,10 @@ def test_elm_basic_send_can(): send_compare(s, b'010D\r', b"7E8 03 41 0D 53 \r\r>") send_compare(s, b'1F00\r', b"NO DATA\r\r>") # Unhandled msg, no response. + + # Repeat last check to see if it still works after NO DATA was received + send_compare(s, b'0100\r', b"7E8 06 41 00 FF FF FF FE \r\r>") + send_compare(s, b'010D\r', b"7E8 03 41 0D 53 \r\r>") finally: sim.stop() sim.join() @@ -183,3 +187,30 @@ def test_elm_send_can_multimsg(): sim.stop() sim.join() s.close() + +# TODO: Expand test to full throughput. +# Max throughput currently causes dropped wifi packets +def test_elm_send_can_multimsg_throughput(): + s = elm_connect() + serial = os.getenv("CANSIMSERIAL") if os.getenv("CANSIMSERIAL") else None + sim = elm_car_simulator.ELMCanCarSimulator(serial) + sim.start() + + try: + sync_reset(s) + send_compare(s, b'ATSP6\r', b"ATSP6\rOK\r\r>") # Set Proto + send_compare(s, b'ATE0\r', b'ATE0\rOK\r\r>') # Echo OFF + send_compare(s, b'ATS0\r', b'OK\r\r>') # Spaces OFF + send_compare(s, b'ATH1\r', b'OK\r\r>') # Headers ON + + send_compare(s, b'09fd\r', # headers OFF, Spaces ON + ("7E8123649FD01AAAAAA\r" + + "".join( + ("7E82"+hex((num+1)%0x10)[2:].upper()+"AAAAAA" + + hex(num)[2:].upper().zfill(8) + "\r" for num in range(80)) + ) + "\r>").encode() + ) + finally: + sim.stop() + sim.join() + s.close() diff --git a/tests/elm_car_simulator.py b/tests/elm_car_simulator.py index dcae1786..ce81626d 100755 --- a/tests/elm_car_simulator.py +++ b/tests/elm_car_simulator.py @@ -84,7 +84,13 @@ class ELMCanCarSimulator(threading.Thread): elif mode == 0x09: # Mode: Request vehicle information if pid == 0x02: # Show VIN outmsg = b"1D4GP00R55B123456" - if pid == 0xFF: # test very long multi message + if pid == 0xFD: # test long multi message + parts = (b'\xAA\xAA\xAA' + struct.pack(">I", num) for num in range(80)) + outmsg = b'\xAA\xAA\xAA' + b''.join(parts) + if pid == 0xFE: # test very long multi message + parts = (b'\xAA\xAA\xAA' + struct.pack(">I", num) for num in range(584)) + outmsg = b'\xAA\xAA\xAA' + b''.join(parts) + b'\xAA' + if pid == 0xFF: outmsg = b"\xAA"*(0xFFF-3) if outmsg: