htool_console: Reconnect USB on failure

examples/htool_console.c

@@ -105,26 +105,39 @@ int htool_console_run(struct libhoth_device* dev,
   bool quit = false;
 
   while (!quit) {
+    // Any previous failure should reset all of the USB state before retrying
+    while (status != LIBHOTH_OK) {
+      // TODO: Read STDIN during this time and buffer it so we can capture
+      // quit events even when the console is disconnected. We will also want
+      // to tune the reconnect time to match.
+      status = libhoth_device_reconnect(dev);
+      // If USB is down we might fail reconnect, just retry
+      if (status != LIBHOTH_OK) {
+        // Make sure we don't end up in a tight retry loop
+        usleep(100 * 1000);
+      }
+    }
+    // Give an opportunity for other clients to use the interface.
+    libhoth_release_device(dev);
+    usleep(1000 * opts->yield_ms);
+    status = libhoth_claim_device(dev, 1000 * 1000 * opts->claim_timeout_secs);
+    if (status != LIBHOTH_OK) {
+      // If USB is down we might fail claim, just go back and retry
+      continue;
+    }
+
     status = libhoth_read_console(dev, STDOUT_FILENO, false, opts->channel_id,
                                   &offset);
     if (status != LIBHOTH_OK) {
-      break;
+      // Device resets cause failures, just loop and allow reconnection
+      continue;
     }
 
     status = libhoth_write_console(dev, opts->channel_id, opts->force_drive_tx,
                                    &quit);
     if (status != LIBHOTH_OK) {
-      break;
-    }
-
-    libhoth_release_device(dev);
-
-    // Give an opportunity for other clients to use the interface.
-    usleep(1000 * opts->yield_ms);
-
-    status = libhoth_claim_device(dev, 1000 * 1000 * opts->claim_timeout_secs);
-    if (status != LIBHOTH_OK) {
-      break;
+      // Device resets cause failures, just loop and allow reconnection
+      continue;
     }
   }
 

protocol/console.c

@@ -106,6 +106,10 @@ int libhoth_read_console(struct libhoth_device* dev, int fd,
   if (status != 0) {
     return status;
   }
+  if (req.offset != resp.resp.offset) {
+    fprintf(stderr, "Console jumped forward %u bytes\n",
+            resp.resp.offset - req.offset);
+  }
 
   int len = response_size - sizeof(resp.resp);
   if (len > 0) {
@@ -130,9 +134,9 @@ int libhoth_read_console(struct libhoth_device* dev, int fd,
         return -1;
       }
     }
-    *offset = resp.resp.offset + len;
   }
 
+  *offset = resp.resp.offset + len;
   return 0;
 }
 

transports/libhoth_usb.c

@@ -92,41 +92,53 @@ static int libhoth_usb_release(struct libhoth_device* dev) {
   return libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id);
 }
 
+static int libhoth_usb_close_internal(struct libhoth_usb_device* usb_dev) {
+  int status = LIBUSB_SUCCESS;
+  if (usb_dev->handle != NULL) {
+    switch (usb_dev->info.type) {
+      case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX:
+        status = libhoth_usb_mailbox_close(usb_dev);
+        break;
+      case LIBHOTH_USB_INTERFACE_TYPE_FIFO:
+        status = libhoth_usb_fifo_close(usb_dev);
+        break;
+      default:
+        status = LIBHOTH_ERR_INTERFACE_NOT_FOUND;
+        break;
+    }
+    libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id);
+    libusb_close(usb_dev->handle);
+    usb_dev->handle = NULL;
+  }
+  return status;
+}
+
 static int libhoth_usb_reconnect(struct libhoth_device* dev) {
   struct libhoth_usb_device* usb_dev = dev->user_ctx;
+  if (usb_dev == NULL) {
+    return LIBUSB_ERROR_INVALID_PARAM;
+  }
   libusb_context* usb_ctx = usb_dev->ctx;
   uint64_t timeout_us = usb_dev->claim_timeout_us;
-
-  struct libusb_device* libusb_dev = libusb_get_device(usb_dev->handle);
-
-  struct libhoth_usb_loc usb_loc;
-  usb_loc.bus = libusb_get_bus_number(libusb_dev);
-  usb_loc.num_ports = libusb_get_port_numbers(
-      libusb_dev, (uint8_t*)&usb_loc.ports, LIBHOTH_NUM_PORTS);
-  if (usb_loc.num_ports == LIBUSB_ERROR_OVERFLOW) {
-    fprintf(stderr, "Failed to list port numbers when reconnecting. (%s)\n",
-            libusb_strerror(LIBUSB_ERROR_OVERFLOW));
-    return LIBUSB_ERROR_OVERFLOW;
-  }
-
-  libhoth_usb_close(dev);
+  struct libhoth_usb_loc usb_loc = usb_dev->loc;
 
   uint64_t start_time_ms = libhoth_get_monotonic_ms();
-
   while (1) {
-    libusb_exit(usb_ctx);
-    int ret = libusb_init(&usb_ctx);
-    if (ret != 0) {
-      fprintf(
-          stderr,
-          "libusb_init_context failed while reconnecting (error: %d (%s))\n",
-          ret, libusb_strerror(ret));
-    }
-
-    ret = libhoth_usb_get_device(usb_ctx, &usb_loc, &libusb_dev);
+    libusb_device* new_libusb_dev = NULL;
+    int ret = libhoth_usb_get_device(usb_ctx, &usb_loc, &new_libusb_dev);
     if (ret == 0) {
-      // Found the device
-      break;
+      struct libhoth_usb_device_init_options opts;
+      opts.usb_ctx = usb_ctx;
+      opts.usb_device = new_libusb_dev;
+      opts.timeout_us = timeout_us;
+      opts.prng_seed = libhoth_prng_seed();
+
+      // Close the old handle and driver data, but keep the usb_dev structure.
+      libhoth_usb_close_internal(usb_dev);
+
+      ret = libhoth_usb_device_open(&opts, dev);
+      libusb_unref_device(new_libusb_dev);
+      return ret;
     }
 
     uint64_t current_time_ms = libhoth_get_monotonic_ms();
@@ -136,27 +148,18 @@ static int libhoth_usb_reconnect(struct libhoth_device* dev) {
           stderr,
           "libhoth_usb_open timed out while reconnecting (error: %d (%s))\n",
           ret, libusb_strerror(ret));
-      libusb_exit(usb_ctx);
       return ret;  // Timeout
     }
 
     // 100ms delay
     usleep(100 * 1000);
   }
-
-  struct libhoth_usb_device_init_options opts;
-  opts.usb_ctx = usb_ctx;
-  opts.usb_device = libusb_dev;
-  opts.timeout_us = timeout_us;
-  opts.prng_seed = libhoth_prng_seed();
-
-  return libhoth_usb_device_open(&opts, dev);
 }
 
 static int libhoth_usb_device_open(
     const struct libhoth_usb_device_init_options* options,
     struct libhoth_device* dev) {
-  struct libhoth_usb_device* usb_dev = NULL;
+  struct libhoth_usb_device* usb_dev = dev->user_ctx;
   struct libusb_device_descriptor device_descriptor;
   int status =
       libusb_get_device_descriptor(options->usb_device, &device_descriptor);
@@ -185,14 +188,23 @@ static int libhoth_usb_device_open(
     goto err_out;
   }
 
-  usb_dev = calloc(1, sizeof(struct libhoth_usb_device));
   if (usb_dev == NULL) {
-    status = LIBHOTH_ERR_MALLOC_FAILED;
-    goto err_out;
+    usb_dev = calloc(1, sizeof(struct libhoth_usb_device));
+    if (usb_dev == NULL) {
+      status = LIBHOTH_ERR_MALLOC_FAILED;
+      goto err_out;
+    }
+    dev->user_ctx = usb_dev;
   }
   usb_dev->info = info;
   usb_dev->ctx = options->usb_ctx;
   usb_dev->claim_timeout_us = options->timeout_us;
+
+  if (libhoth_get_usb_loc(options->usb_device, &usb_dev->loc) != 0) {
+    status = LIBUSB_ERROR_OTHER;
+    goto err_out;
+  }
+
   status = libusb_open(options->usb_device, &usb_dev->handle);
   if (status != LIBUSB_SUCCESS) {
     goto err_out;
@@ -204,7 +216,6 @@ static int libhoth_usb_device_open(
   dev->claim = libhoth_usb_claim;
   dev->release = libhoth_usb_release;
   dev->reconnect = libhoth_usb_reconnect;
-  dev->user_ctx = usb_dev;
 
   status = libhoth_claim_device(dev, options->timeout_us);
   if (status != LIBHOTH_OK) {
@@ -231,14 +242,10 @@ static int libhoth_usb_device_open(
   return LIBHOTH_OK;
 
 err_out:
-  if (dev != NULL) {
-    if (usb_dev != NULL) {
-      if (usb_dev->handle != NULL) {
-        libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id);
-        libusb_close(usb_dev->handle);
-      }
-      free(usb_dev);
-    }
+  if (usb_dev != NULL && usb_dev->handle != NULL) {
+    libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id);
+    libusb_close(usb_dev->handle);
+    usb_dev->handle = NULL;
   }
   libusb_free_config_descriptor(config_descriptor);
   return status;
@@ -257,6 +264,9 @@ int libhoth_usb_open(const struct libhoth_usb_device_init_options* options,
 
   int ret = libhoth_usb_device_open(options, dev);
   if (ret != LIBUSB_SUCCESS) {
+    if (dev->user_ctx != NULL) {
+      free(dev->user_ctx);
+    }
     free(dev);
     return ret;
   }
@@ -273,6 +283,9 @@ int libhoth_usb_send_request(struct libhoth_device* dev, const void* request,
 
   struct libhoth_usb_device* usb_dev =
       (struct libhoth_usb_device*)dev->user_ctx;
+  if (usb_dev->handle == NULL) {
+    return LIBUSB_ERROR_NO_DEVICE;
+  }
   switch (usb_dev->info.type) {
     case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX:
       return libhoth_usb_mailbox_send_request(usb_dev, request, request_size);
@@ -293,6 +306,9 @@ int libhoth_usb_receive_response(struct libhoth_device* dev, void* response,
 
   struct libhoth_usb_device* usb_dev =
       (struct libhoth_usb_device*)dev->user_ctx;
+  if (usb_dev->handle == NULL) {
+    return LIBUSB_ERROR_NO_DEVICE;
+  }
   switch (usb_dev->info.type) {
     case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX:
       return libhoth_usb_mailbox_receive_response(
@@ -307,33 +323,17 @@ int libhoth_usb_receive_response(struct libhoth_device* dev, void* response,
 }
 
 int libhoth_usb_close(struct libhoth_device* dev) {
-  int status;
   if (dev->user_ctx == NULL) {
-    return LIBUSB_ERROR_INVALID_PARAM;
+    return LIBUSB_SUCCESS;
   }
 
   struct libhoth_usb_device* usb_dev =
       (struct libhoth_usb_device*)dev->user_ctx;
   dev->user_ctx = NULL;
-  switch (usb_dev->info.type) {
-    case LIBHOTH_USB_INTERFACE_TYPE_MAILBOX:
-      status = libhoth_usb_mailbox_close(usb_dev);
-      break;
-    case LIBHOTH_USB_INTERFACE_TYPE_FIFO:
-      status = libhoth_usb_fifo_close(usb_dev);
-      break;
-    default:
-      return LIBHOTH_ERR_INTERFACE_NOT_FOUND;
-  }
-  if (status != LIBHOTH_OK) {
-    return status;
-  }
-  if (usb_dev->handle != NULL) {
-    libusb_release_interface(usb_dev->handle, usb_dev->info.interface_id);
-    libusb_close(usb_dev->handle);
-  }
+
+  int status = libhoth_usb_close_internal(usb_dev);
   free(usb_dev);
-  return LIBHOTH_OK;
+  return status;
 }
 
 enum libusb_error transfer_status_to_error(

transports/libhoth_usb_device.h

@@ -19,6 +19,8 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+#include "transports/libhoth_usb.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -72,6 +74,7 @@ struct libhoth_usb_device {
   libusb_context* ctx;
   libusb_device_handle* handle;
   uint32_t claim_timeout_us;
+  struct libhoth_usb_loc loc;
   struct libhoth_usb_interface_info info;
   union driver_data {
     struct libhoth_usb_mailbox mailbox;