uart_ow: Better error handling in corner cases

Signed-off-by: Brendan Le Foll <brendan.le.foll@intel.com>

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

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);
+        int res = mraa_uart_ow_bit(m_uart, (bit) ? 1 : 0);
-        return ((rv) ? true : false);
+        if (res == -1) {
+            throw std::invalid_argument("Unknown UART_OW error");
+        }
+        return ((res) ? true : false);
     }
 
     /**

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;

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
+static mraa_result_t
-read_byte(mraa_uart_ow_context dev)
+_ow_read_byte(mraa_uart_ow_context dev, uint8_t *ch)
 {
-    unsigned char ch = 0;
+    while (!mraa_uart_read(dev, (char*) ch, 1))
-
-    while (!mraa_uart_read(dev, &ch, 1))
         ;
 
-    return ch;
+    return MRAA_SUCCESS;
 }
 
 // low-level write byte
-static void
+static int
-write_byte(mraa_uart_ow_context dev, const unsigned char ch)
+_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
+static mraa_result_t
-set_speed(mraa_uart_context dev, mraa_boolean_t speed)
+_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)
+    int ret = 0;
-        write_byte(dev, 0xff); /* write a 1 bit */
+    uint8_t ch;
-    else
+    if (bit) {
-        write_byte(dev, 0x00); /* write a 0 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);
+    if (_ow_set_speed(dev, 0) != MRAA_SUCCESS) {
-    /* pull the data line low */
+        return MRAA_ERROR_INVALID_HANDLE;
-    write_byte(dev, 0xf0);
+    }
 
-    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)