From: norly Date: Mon, 18 Feb 2019 23:24:21 +0000 (+0100) Subject: Sanity check TTY input and bail on problems X-Git-Url: https://git.enpas.org/?a=commitdiff_plain;h=8376bf6ff844f12088f568d2284a4ef4a69ca9c4;p=elmcan.git Sanity check TTY input and bail on problems Also, introduce module parameter 'accept_flaky_uart'. If your adapter or its connection is unreliable, set this option to true to try and make the best of a bad situation, but undefined behavior is prone to occur. --- diff --git a/module/elmcan.c b/module/elmcan.c index 1be5da8..82ffc86 100644 --- a/module/elmcan.c +++ b/module/elmcan.c @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -45,6 +46,16 @@ MODULE_DESCRIPTION("ELM327 based CAN interface"); MODULE_LICENSE("GPL"); MODULE_AUTHOR("Max Staudt "); +/* If this is enabled, we'll try to make the best of the situation + * even if we receive unexpected characters on the line. + * No guarantees. + * Handle with care, it's likely your hardware is unreliable! + */ +static bool accept_flaky_uart = false; +module_param_named(accept_flaky_uart, accept_flaky_uart, bool, 0444); +MODULE_PARM_DESC(accept_flaky_uart, "Don't bail at the first invalid character. Behavior undefined."); + + /* Line discipline ID number */ #ifndef N_ELMCAN #define N_ELMCAN 29 @@ -193,7 +204,6 @@ static void elm327_kick_into_cmd_mode(struct elmcan *elm) elm327_send(elm, ELM327_MAGIC_STRING, 1); elm->state = ELM_GETMAGICCHAR; - elm->rxfill = 0; } } @@ -361,6 +371,12 @@ static inline void elm327_hw_failure(struct elmcan *elm) * (assumes elm->lock taken) * ************************************************************************/ +static bool elm327_is_ready_char(char c) +{ + return (c & 0x3f) == ELM327_READY_CHAR; +} + + static void elm327_parse_error(struct elmcan *elm, int len) { struct can_frame frame; @@ -442,6 +458,23 @@ static int elm327_parse_frame(struct elmcan *elm, int len) } } + /* If we accept stray characters coming in: + * Check for stray characters on a payload line. + * No idea what causes this. + */ + if (accept_flaky_uart + && hexlen < len + && !isdigit(elm->rxbuf[hexlen]) + && !isupper(elm->rxbuf[hexlen]) + && '<' != elm->rxbuf[hexlen] + && ' ' != elm->rxbuf[hexlen]) { + /* The line is likely garbled anyway, so bail. + * The main code will restart listening. + */ + elm327_kick_into_cmd_mode(elm); + return 3; + } + /* Use spaces in CAN ID to distinguish 29 or 11 bit address length. * No out-of-bounds access: * We use the fact that we can always read from elm->rxbuf. @@ -554,8 +587,8 @@ static void elm327_parse_line(struct elmcan *elm, int len) /* Parse an error line. */ elm327_parse_error(elm, len); - /* After the error line, we expect a prompt. */ - elm->state = ELM_GETPROMPT; + /* Start afresh. */ + elm327_kick_into_cmd_mode(elm); } break; default: @@ -673,7 +706,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm) elm->state = ELM_GETPROMPT; i++; break; - } else if ((elm->rxbuf[i] & 0x3f) == ELM327_READY_CHAR) { + } else if (elm327_is_ready_char(elm->rxbuf[i])) { /* Mask 0xc0 to work around hardware bugs */ elm327_send(elm, ELM327_MAGIC_STRING, 1); i++; @@ -691,7 +724,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm) * Bits 0xc0 are sometimes set (randomly), hence the mask. * Probably bad hardware. */ - if ((elm->rxbuf[elm->rxfill - 1] & 0x3f) == ELM327_READY_CHAR) { + if (elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) { elm327_handle_prompt(elm); } @@ -715,7 +748,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm) return; } else if (len == elm->rxfill) { if (elm->state == ELM_RECEIVING - && elm->rxbuf[elm->rxfill - 1] == ELM327_READY_CHAR) { + && elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) { /* The ELM327's AT ST response timeout ran out, * so we got a prompt. * Clear RX buffer and restart listening. @@ -965,9 +998,46 @@ static void elmcan_ldisc_rx(struct tty_struct *tty, put_elm(elm); return; } + + /* Ignore NUL characters, which the PIC microcontroller may + * inadvertently insert due to a known hardware bug. + * See ELM327 documentation, which refers to a Microchip PIC + * bug description. + */ if (*cp != 0) { + /* Check for stray characters on the UART line. + * No idea what causes this. + */ + if (!accept_flaky_uart + && !isdigit(*cp) + && !isupper(*cp) + && ELM327_MAGIC_CHAR != *cp + && ELM327_READY_CHAR != *cp + && '<' != *cp + && 'a' != *cp + && 'b' != *cp + && 'v' != *cp + && '.' != *cp + && '?' != *cp + && '\r' != *cp + && ' ' != *cp) { + /* We've received an invalid character, so bail. + * There's something wrong with the ELM327, or + * with the UART line. + */ + netdev_err(elm->dev, + "Received illegal character %02x.\n", + *cp); + elm327_hw_failure(elm); + spin_unlock_bh(&elm->lock); + + put_elm(elm); + return; + } + elm->rxbuf[elm->rxfill++] = *cp; } + cp++; } diff --git a/readme.rst b/readme.rst index d2aced2..23920d8 100644 --- a/readme.rst +++ b/readme.rst @@ -92,6 +92,21 @@ sheet. This needs to be done before attaching the line discipline. +Module parameters +------------------ + +- ``accept_flaky_uart`` - Try to live with a bad controller or UART line + + Some adapters and/or their connection are unreliable. Enable this + option to try and work around the situation. This is a best-effort + workaround, so undefined behavior will likely occur sooner or later. + + LOAD TIME:: + + module/kernel parameter: accept_flaky_uart=[0|1] + + + Known limitations of the controller ------------------------------------