From c41c3b41d54b4a09f4d6886c7aff9f5d7f1b88d2 Mon Sep 17 00:00:00 2001 From: Brendan Le Foll Date: Fri, 15 Apr 2016 12:03:32 +0100 Subject: [PATCH] uart_ow: Better error handling in corner cases Signed-off-by: Brendan Le Foll --- api/mraa/uart_ow.h | 12 +++---- api/mraa/uart_ow.hpp | 25 +++++++++++--- examples/uart_ow.c | 3 +- src/uart_ow/uart_ow.c | 76 +++++++++++++++++++++++++------------------ 4 files changed, 72 insertions(+), 44 deletions(-) diff --git a/api/mraa/uart_ow.h b/api/mraa/uart_ow.h index f54690a..ce53bd5 100644 --- a/api/mraa/uart_ow.h +++ b/api/mraa/uart_ow.h @@ -117,18 +117,18 @@ mraa_result_t mraa_uart_ow_stop(mraa_uart_ow_context dev); * Read a byte from the 1-wire bus * * @param dev uart_ow context - * @return the byte read + * @return the byte read or -1 for error */ -uint8_t mraa_uart_ow_read_byte(mraa_uart_ow_context dev); +int mraa_uart_ow_read_byte(mraa_uart_ow_context dev); /** * Write a byte to a 1-wire bus * * @param dev uart_ow context * @param byte the byte to write to the bus - * @return the byte read back during the time slot + * @return the byte read back during the time slot or -1 for error */ -uint8_t mraa_uart_ow_write_byte(mraa_uart_ow_context dev, uint8_t byte); +int mraa_uart_ow_write_byte(mraa_uart_ow_context dev, uint8_t byte); /** * Write a bit to a 1-wire bus and read a bit corresponding to the @@ -137,9 +137,9 @@ uint8_t mraa_uart_ow_write_byte(mraa_uart_ow_context dev, uint8_t byte); * * @param dev uart_ow context * @param bit the bit to write to the bus - * @return the bit read back during the time slot + * @return the bit read back during the time slot or -1 for error */ -uint8_t mraa_uart_ow_bit(mraa_uart_ow_context dev, uint8_t bit); +int mraa_uart_ow_bit(mraa_uart_ow_context dev, uint8_t bit); /** * Send a reset pulse to the 1-wire bus and test for device presence diff --git a/api/mraa/uart_ow.hpp b/api/mraa/uart_ow.hpp index 6c3c1d4..d20f87e 100644 --- a/api/mraa/uart_ow.hpp +++ b/api/mraa/uart_ow.hpp @@ -46,6 +46,7 @@ class UartOW * UartOW Constructor, takes a pin number which will map directly to the * linux uart number, this 'enables' the uart, nothing more * + * @throws std::invalid_argument in case of error * @param uart the index of the uart to use */ UartOW(int uart) @@ -61,6 +62,7 @@ class UartOW * UartOW Constructor, takes a string to the path of the serial * interface that is needed. * + * @throws std::invalid_argument in case of error * @param path the file path for the UART to use */ UartOW(std::string path) @@ -96,24 +98,35 @@ class UartOW /** * Read a byte from the 1-wire bus * + * @throws std::invalid_argument in case of error * @return the byte read */ uint8_t readByte() { - return mraa_uart_ow_read_byte(m_uart); + int res = mraa_uart_ow_read_byte(m_uart); + if (res == -1) { + throw std::invalid_argument("Unknown UART_OW error"); + } + return (uint8_t) res; } /** * Write a byte to a 1-wire bus * * @param byte the byte to write to the bus + * + * @throws std::invalid_argument in case of error * @return the byte read back during the time slot */ uint8_t writeByte(uint8_t byte) { - return mraa_uart_ow_write_byte(m_uart, byte); + int res = mraa_uart_ow_write_byte(m_uart, byte); + if (res == -1) { + throw std::invalid_argument("Unknown UART_OW error"); + } + return (uint8_t) res; } /** @@ -122,13 +135,17 @@ class UartOW * and RX together with a diode, forming a loopback. * * @param bit the bit to write to the bus + * @throws std::invalid_argument in case of error * @return the bit read back during the time slot */ bool writeBit(bool bit) { - uint8_t rv = mraa_uart_ow_bit(m_uart, (bit) ? 1 : 0); - return ((rv) ? true : false); + int res = mraa_uart_ow_bit(m_uart, (bit) ? 1 : 0); + if (res == -1) { + throw std::invalid_argument("Unknown UART_OW error"); + } + return ((res) ? true : false); } /** diff --git a/examples/uart_ow.c b/examples/uart_ow.c index 4e15ceb..63ed410 100644 --- a/examples/uart_ow.c +++ b/examples/uart_ow.c @@ -58,8 +58,7 @@ main(int argc, char** argv) uint8_t count = 0; // start the search from scratch - uint8_t result = mraa_uart_ow_rom_search(uart, 1, id); - + mraa_result_t result = mraa_uart_ow_rom_search(uart, 1, id); if (result == MRAA_ERROR_UART_OW_NO_DEVICES) { printf("No devices detected.\n"); return 1; diff --git a/src/uart_ow/uart_ow.c b/src/uart_ow/uart_ow.c index 74675c8..bbd91cb 100644 --- a/src/uart_ow/uart_ow.c +++ b/src/uart_ow/uart_ow.c @@ -47,22 +47,20 @@ static int LastFamilyDiscrepancy; static mraa_boolean_t LastDeviceFlag; // low-level read byte -static unsigned char -read_byte(mraa_uart_ow_context dev) +static mraa_result_t +_ow_read_byte(mraa_uart_ow_context dev, uint8_t *ch) { - unsigned char ch = 0; - - while (!mraa_uart_read(dev, &ch, 1)) + while (!mraa_uart_read(dev, (char*) ch, 1)) ; - return ch; + return MRAA_SUCCESS; } // low-level write byte -static void -write_byte(mraa_uart_ow_context dev, const unsigned char ch) +static int +_ow_write_byte(mraa_uart_ow_context dev, const char ch) { - mraa_uart_write(dev, &ch, 1); + return mraa_uart_write(dev, &ch, 1); } // Here we setup a very simple termios with the minimum required @@ -70,20 +68,22 @@ write_byte(mraa_uart_ow_context dev, const unsigned char ch) // use the low speed (9600 bd) for emitting the reset pulse, and // high speed (115200 bd) for actual data communications. // -static void -set_speed(mraa_uart_context dev, mraa_boolean_t speed) +static mraa_result_t +_ow_set_speed(mraa_uart_context dev, mraa_boolean_t speed) { if (!dev) { syslog(LOG_ERR, "uart_ow: set_speed: context is NULL"); - return; + return MRAA_ERROR_INVALID_HANDLE; } static speed_t baud; - if (speed) + if (speed) { baud = B115200; - else + } + else { baud = B9600; + } struct termios termio = { .c_cflag = baud | CS8 | CLOCAL | CREAD, .c_iflag = 0, .c_oflag = 0, .c_lflag = NOFLSH, .c_cc = { 0 }, @@ -94,10 +94,10 @@ set_speed(mraa_uart_context dev, mraa_boolean_t speed) // TCSANOW is required if (tcsetattr(dev->fd, TCSANOW, &termio) < 0) { syslog(LOG_ERR, "uart_ow: tcsetattr() failed"); - return; + return MRAA_ERROR_INVALID_RESOURCE; } - return; + return MRAA_SUCCESS; } // Perform the 1-Wire Search Algorithm on the 1-Wire bus using the existing @@ -291,31 +291,38 @@ mraa_uart_ow_get_dev_path(mraa_uart_ow_context dev) return mraa_uart_get_dev_path(dev); } -uint8_t +int mraa_uart_ow_bit(mraa_uart_ow_context dev, uint8_t bit) { if (!dev) { syslog(LOG_ERR, "uart_ow: ow_bit: context is NULL"); - return 0; + return -1; } - if (bit) - write_byte(dev, 0xff); /* write a 1 bit */ - else - write_byte(dev, 0x00); /* write a 0 bit */ + int ret = 0; + uint8_t ch; + if (bit) { + ret = _ow_write_byte(dev, 0xff); /* write a 1 bit */ + } + else { + ret = _ow_write_byte(dev, 0x00); /* write a 0 bit */ + } /* return the bit present on the bus (0xff is a '1', anything else * (typically 0xfc or 0x00) is a 0 */ - return (read_byte(dev) == 0xff); + if (_ow_read_byte(dev, &ch) == -1 || ret == -1) { + return -1; + } + return (ch == 0xff); } -uint8_t +int mraa_uart_ow_write_byte(mraa_uart_ow_context dev, uint8_t byte) { if (!dev) { syslog(LOG_ERR, "uart_ow: write_byte: context is NULL"); - return 0; + return -1; } /* writing bytes - each bit on the byte to send corresponds to a @@ -341,12 +348,12 @@ mraa_uart_ow_write_byte(mraa_uart_ow_context dev, uint8_t byte) return byte; } -uint8_t +int mraa_uart_ow_read_byte(mraa_uart_ow_context dev) { if (!dev) { syslog(LOG_ERR, "uart_ow: read_byte: context is NULL"); - return 0; + return -1; } /* we read by sending 0xff, so the bus is released on the initial @@ -379,14 +386,19 @@ mraa_uart_ow_reset(mraa_uart_ow_context dev) * window. If no device is present, the receive value will equal the * transmit value. Otherwise the receive value can vary. */ - set_speed(dev, 0); - /* pull the data line low */ - write_byte(dev, 0xf0); + if (_ow_set_speed(dev, 0) != MRAA_SUCCESS) { + return MRAA_ERROR_INVALID_HANDLE; + } - rv = read_byte(dev); + /* pull the data line low */ + _ow_write_byte(dev, 0xf0); + + _ow_read_byte(dev, &rv); /* back up to high speed for normal data transmissions */ - set_speed(dev, 1); + if (_ow_set_speed(dev, 1) != MRAA_SUCCESS) { + return MRAA_ERROR_INVALID_HANDLE; + } /* shorted data line */ if (rv == 0x00)