pbio/drv/legodev: Replace unplug reset by watchdog.

This keeps the device connection manager in control even if the UART
device and PT threads get in a confused state.

Fixes pybricks/support#1137

Author: laurensvalk

CHANGELOG.md

@@ -4,10 +4,13 @@
 
 ## [Unreleased]
 
+### Fixed
 - Fixed Technic (Extra) Large motors not working ([support#1131]) on all hubs.
 - Fixed Powered Up Light not working ([support#1131]) on all hubs.
+- Fixed UART sensors not working on Technic Hub ([support#1137]).
 
-[support#1064]: https://github.com/pybricks/support/issues/1131
+[support#1131]: https://github.com/pybricks/support/issues/1131
+[support#1137]: https://github.com/pybricks/support/issues/1137
 
 ## [3.3.0b7] - 2023-06-30
 

lib/pbio/drv/legodev/legodev_pup.c

@@ -34,6 +34,10 @@
 /** The number of consecutive repeated detections needed for an affirmative ID. */
 #define AFFIRMATIVE_MATCH_COUNT 20
 
+/** If the UART device did not call back to this driver after this time, assume
+    it is disconnected and restart device detection. */
+#define UART_WATCHDOG_TIMEOUT 1100
+
 typedef enum {
     DEV_ID_GROUP_GND,
     DEV_ID_GROUP_VCC,
@@ -49,6 +53,7 @@ typedef struct {
     uint8_t gpio_value;
     uint8_t prev_gpio_value;
     uint8_t dev_id_match_count;
+    struct timer watchdog;
 } dcm_data_t;
 
 typedef struct {
@@ -325,6 +330,13 @@ PROCESS_THREAD(pbio_legodev_pup_process, ev, data) {
             for (int i = 0; i < PBDRV_CONFIG_LEGODEV_PUP_NUM_EXT_DEV; i++) {
                 dev = &ext_devs[i];
 
+                // If UART device was connected but not reporting data, restart detection.
+                if (dev->connected_type_id == PBDRV_LEGODEV_TYPE_ID_LPF2_UNKNOWN_UART &&
+                    dev->connected_type_id == dev->prev_type_id && timer_expired(&dev->dcm.watchdog)) {
+                    dev->connected_type_id = PBDRV_LEGODEV_TYPE_ID_NONE;
+                    dev->dcm.dev_id_match_count = 0;
+                }
+
                 // Keep running device detection unless UART sensor is attached.
                 if (dev->connected_type_id != PBDRV_LEGODEV_TYPE_ID_LPF2_UNKNOWN_UART) {
                     poll_dcm(dev);
@@ -383,6 +395,7 @@ void pbdrv_legodev_init(void) {
         const pbdrv_ioport_pup_port_platform_data_t *port_data = &pbdrv_ioport_pup_platform_data.ports[legodev_data->ioport_index];
         legodev->ext_dev->pdata = legodev_data;
         legodev->ext_dev->pins = &port_data->pins;
+        timer_set(&legodev->ext_dev->dcm.watchdog, UART_WATCHDOG_TIMEOUT);
 
         // Initialize uart device manager.
         pbio_dcmotor_t *dcmotor;
@@ -394,25 +407,28 @@ void pbdrv_legodev_init(void) {
     pbdrv_legodev_pup_uart_process_start();
 }
 
+static pbdrv_legodev_dev_t *pbdrv_legodev_get_parent_of(pbdrv_legodev_pup_uart_dev_t *uartdev) {
+    for (uint8_t i = 0; i < PBIO_ARRAY_SIZE(devs); i++) {
+        pbdrv_legodev_dev_t *dev = &devs[i];
+        if (!dev->is_internal && dev->ext_dev->uart_dev == uartdev) {
+            return dev;
+        }
+    }
+    return NULL;
+}
+
 /**
- * Resets device detection. Called by legodev_uart if a device gets out of sync, usually by unplugging.
+ * Resets device connection watchdog timer. This allows the detection manager
+ * to resume when a UART device stopped working, usually by being unplugged.
  *
  * @param [in]  dev           legodev device.
  */
-void pbdrv_legodev_pup_reset_device_detection(pbdrv_legodev_pup_uart_dev_t *uartdev) {
-    for (uint8_t i = 0; i < PBIO_ARRAY_SIZE(devs); i++) {
-        pbdrv_legodev_dev_t *dev = &devs[i];
-        if (dev->is_internal) {
-            // Internal devices don't have a uart device.
-            continue;
-        }
-        if (dev->ext_dev->uart_dev == uartdev) {
-            // On match reset device connection manager state.
-            dev->ext_dev->connected_type_id = PBDRV_LEGODEV_TYPE_ID_NONE;
-            dev->ext_dev->dcm.dev_id_match_count = 0;
-            return;
-        }
+void pbdrv_legodev_pup_reset_watchdog(pbdrv_legodev_pup_uart_dev_t *uartdev) {
+    pbdrv_legodev_dev_t *dev = pbdrv_legodev_get_parent_of(uartdev);
+    if (dev == NULL) {
+        return;
     }
+    timer_restart(&dev->ext_dev->dcm.watchdog);
 }
 
 pbdrv_legodev_pup_uart_dev_t *pbdrv_legodev_get_uart_dev(pbdrv_legodev_dev_t *legodev) {

lib/pbio/drv/legodev/legodev_pup.h

@@ -33,11 +33,11 @@ extern const pbdrv_legodev_pup_int_platform_data_t
 extern const pbdrv_legodev_pup_ext_platform_data_t
     pbdrv_legodev_pup_ext_platform_data[PBDRV_CONFIG_LEGODEV_PUP_NUM_EXT_DEV];
 
-void pbdrv_legodev_pup_reset_device_detection(pbdrv_legodev_pup_uart_dev_t *dev);
+void pbdrv_legodev_pup_reset_watchdog(pbdrv_legodev_pup_uart_dev_t *uartdev);
 
 #else
 
-static inline void pbdrv_legodev_pup_reset_device_detection(pbdrv_legodev_pup_uart_dev_t *dev) {
+static inline void pbdrv_legodev_pup_reset_watchdog(pbdrv_legodev_pup_uart_dev_t *uartdev) {
 }
 
 #endif // PBDRV_CONFIG_LEGODEV_PUP

lib/pbio/drv/legodev/legodev_pup_uart.c

@@ -264,6 +264,7 @@ static void pbdrv_legodev_request_data_set(pbdrv_legodev_pup_uart_dev_t *port_da
 void pbdrv_legodev_pup_uart_start_sync(pbdrv_legodev_pup_uart_dev_t *dev) {
     debug_pr_str("Wait for STATUS_SYNCING: set\n");
     dev->status = PBDRV_LEGODEV_PUP_UART_STATUS_SYNCING;
+    pbdrv_legodev_pup_reset_watchdog(dev);
     process_poll(&pbdrv_legodev_pup_uart_process);
 }
 
@@ -294,6 +295,9 @@ static void pbdrv_legodev_pup_uart_parse_msg(pbdrv_legodev_pup_uart_dev_t *data)
     uint32_t speed;
     uint8_t msg_type, cmd, msg_size, mode, cmd2;
 
+    // Getting messages from the device means it is alive.
+    pbdrv_legodev_pup_reset_watchdog(data);
+
     msg_type = data->rx_msg[0] & LUMP_MSG_TYPE_MASK;
     cmd = data->rx_msg[0] & LUMP_MSG_CMD_MASK;
     msg_size = ev3_uart_get_msg_size(data->rx_msg[0]);
@@ -633,7 +637,6 @@ static void pbdrv_legodev_pup_uart_parse_msg(pbdrv_legodev_pup_uart_dev_t *data)
 err:
     data->status = PBDRV_LEGODEV_PUP_UART_STATUS_ERR;
     debug_pr("Data error: %s\n", data->last_err);
-    pbdrv_legodev_pup_reset_device_detection(data);
 }
 
 static uint8_t ev3_uart_set_msg_hdr(lump_msg_type_t type, lump_msg_size_t size, lump_cmd_t cmd) {
@@ -726,6 +729,9 @@ static PT_THREAD(pbdrv_legodev_pup_uart_update(pbdrv_legodev_pup_uart_dev_t * da
     uint8_t speed_payload[4];
     pbio_set_uint32_le(speed_payload, EV3_UART_SPEED_LPF2);
     ev3_uart_prepare_tx_msg(data, LUMP_MSG_TYPE_CMD, LUMP_CMD_SPEED, speed_payload, sizeof(speed_payload));
+
+    pbdrv_uart_flush(data->uart);
+
     PT_SPAWN(&data->pt, &data->write_pt, pbdrv_legodev_pup_uart_send_prepared_msg(data, &err));
     if (err != PBIO_SUCCESS) {
         DBG_ERR(data->last_err = "Speed tx failed");
@@ -734,6 +740,8 @@ static PT_THREAD(pbdrv_legodev_pup_uart_update(pbdrv_legodev_pup_uart_dev_t * da
 
     pbdrv_uart_flush(data->uart);
 
+    pbdrv_legodev_pup_reset_watchdog(data);
+
     // read one byte to check for ACK
     PBIO_PT_WAIT_READY(&data->pt, err = pbdrv_uart_read_begin(data->uart, data->rx_msg, 1, 10));
     if (err != PBIO_SUCCESS) {
@@ -755,6 +763,9 @@ static PT_THREAD(pbdrv_legodev_pup_uart_update(pbdrv_legodev_pup_uart_dev_t * da
 
     // To get in sync with the data stream from the sensor, we look for a valid TYPE command.
     for (;;) {
+
+        pbdrv_legodev_pup_reset_watchdog(data);
+
         PBIO_PT_WAIT_READY(&data->pt, err = pbdrv_uart_read_begin(data->uart, data->rx_msg, 1, EV3_UART_IO_TIMEOUT));
         if (err != PBIO_SUCCESS) {
             DBG_ERR(data->last_err = "UART Rx error during sync");
@@ -974,9 +985,6 @@ static PT_THREAD(pbdrv_legodev_pup_uart_update(pbdrv_legodev_pup_uart_dev_t * da
     // Turn off battery supply to this port
     pbdrv_motor_driver_coast(data->dcmotor->motor_driver);
 
-    // reset device detection manager
-    pbdrv_legodev_pup_reset_device_detection(data);
-
     PT_END(&data->pt);
 }
 

lib/pbio/test/src/test_uartdev.c

@@ -19,7 +19,6 @@
 #include <pbio/util.h>
 #include <test-pbio.h>
 
-#include "../drv/legodev/legodev_pup.h"
 #include "../drv/legodev/legodev_pup_uart.h"
 
 #include "../src/processes.h"