uni-t-ut32x: migrate from USB transfers to serial-over-HID communication

Switch the UT32x driver from running specific USB transfers to generic
serial communication. Preset the bitrate and frame format, but allow for
user specified overrides as well. Default to the WCH CH9325 HID chip,
but allow for overrides or more specific selection so that users can
resolve ambiguities with multiple cables.

The switch from libusb to hidapi removes a limitation that specifically
was reported for the Mac platform. The serial-over-HID variant should
now work as well. See bug #555.

Drop the background transfers. Stick with a local acquisition stop
routine, because a STOP request needs to get sent to the device. Reduce
the receive buffer size such that either blocking or non-blocking calls
will work. The additional flexibility of the buffer handling and packet
processing does not harm.

configure.ac

@@ -293,7 +293,7 @@ SR_DRIVER([Teleinfo], [teleinfo], [serial_comm])
 SR_DRIVER([Testo], [testo], [libusb])
 SR_DRIVER([Tondaj SL-814], [tondaj-sl-814], [serial_comm])
 SR_DRIVER([UNI-T DMM], [uni-t-dmm], [libusb])
-SR_DRIVER([UNI-T UT32x], [uni-t-ut32x], [libusb])
+SR_DRIVER([UNI-T UT32x], [uni-t-ut32x], [serial_comm])
 SR_DRIVER([Victor DMM], [victor-dmm], [libusb])
 SR_DRIVER([Yokogawa DL/DLM], [yokogawa-dlm])
 SR_DRIVER([ZEROPLUS Logic Cube], [zeroplus-logic-cube], [libusb])

src/hardware/uni-t-ut32x/api.c

@@ -23,6 +23,7 @@
 
 static const uint32_t scanopts[] = {
 	SR_CONF_CONN,
+	SR_CONF_SERIALCOMM,
 };
 
 static const uint32_t drvopts[] = {
@@ -55,110 +56,63 @@ static const char *data_sources[] = {
 
 static GSList *scan(struct sr_dev_driver *di, GSList *options)
 {
-	struct drv_context *drvc;
+	const char *conn, *serialcomm;
-	struct dev_context *devc;
-	struct sr_dev_inst *sdi;
 	struct sr_config *src;
-	GSList *usb_devices, *devices, *l;
+	GSList *l, *devices;
-	unsigned int i;
+	struct sr_serial_dev_inst *serial;
-	const char *conn;
+	int rc;
+	struct sr_dev_inst *sdi;
+	struct dev_context *devc;
+	size_t i;
 
-	drvc = di->context;
+	conn = "hid/ch9325";
-
+	serialcomm = "2400/8n1";
-	conn = "1a86.e008";
 	for (l = options; l; l = l->next) {
 		src = l->data;
 		switch (src->key) {
 		case SR_CONF_CONN:
 			conn = g_variant_get_string(src->data, NULL);
 			break;
+		case SR_CONF_SERIALCOMM:
+			serialcomm = g_variant_get_string(src->data, NULL);
+			break;
 		}
 	}
-	if (!conn)
-		return NULL;
 
 	devices = NULL;
-	if ((usb_devices = sr_usb_find(drvc->sr_ctx->libusb_ctx, conn))) {
+	serial = sr_serial_dev_inst_new(conn, serialcomm);
-		/* We have a list of sr_usb_dev_inst matching the connection
+	rc = serial_open(serial, SERIAL_RDWR);
-		 * string. Wrap them in sr_dev_inst and we're done. */
+	serial_flush(serial);
-		for (l = usb_devices; l; l = l->next) {
+	/* Cannot query/identify the device. Successful open shall suffice. */
-			sdi = g_malloc0(sizeof(struct sr_dev_inst));
+	serial_close(serial);
+	if (rc != SR_OK) {
+		sr_serial_dev_inst_free(serial);
+		return devices;
+	}
+
+	sdi = g_malloc0(sizeof(*sdi));
 	sdi->status = SR_ST_INACTIVE;
 	sdi->vendor = g_strdup("UNI-T");
 	sdi->model = g_strdup("UT32x");
-			sdi->inst_type = SR_INST_USB;
+	sdi->inst_type = SR_INST_SERIAL;
-			sdi->conn = l->data;
+	sdi->conn = serial;
-			for (i = 0; i < ARRAY_SIZE(channel_names); i++)
+	devc = g_malloc0(sizeof(*devc));
-				sr_channel_new(sdi, i, SR_CHANNEL_ANALOG, TRUE,
-						channel_names[i]);
-			devc = g_malloc0(sizeof(struct dev_context));
 	sdi->priv = devc;
 	sr_sw_limits_init(&devc->limits);
 	devc->data_source = DEFAULT_DATA_SOURCE;
-			devices = g_slist_append(devices, sdi);
+	for (i = 0; i < ARRAY_SIZE(channel_names); i++) {
+		sr_channel_new(sdi, i, SR_CHANNEL_ANALOG, TRUE,
+				channel_names[i]);
 	}
-		g_slist_free(usb_devices);
+	devices = g_slist_append(devices, sdi);
-	} else
+
-		g_slist_free_full(usb_devices, g_free);
+	serial_close(serial);
+	if (!devices)
+		sr_serial_dev_inst_free(serial);
 
 	return std_scan_complete(di, devices);
 }
 
-static int dev_open(struct sr_dev_inst *sdi)
-{
-	struct sr_dev_driver *di = sdi->driver;
-	struct drv_context *drvc = di->context;
-	struct sr_usb_dev_inst *usb;
-	int ret;
-
-	usb = sdi->conn;
-
-	if (sr_usb_open(drvc->sr_ctx->libusb_ctx, usb) != SR_OK)
-		return SR_ERR;
-
-/*
- * The libusb 1.0.9 Darwin backend is broken: it can report a kernel
- * driver being active, but detaching it always returns an error.
- */
-#if !defined(__APPLE__)
-	if (libusb_kernel_driver_active(usb->devhdl, USB_INTERFACE) == 1) {
-		if ((ret = libusb_detach_kernel_driver(usb->devhdl, USB_INTERFACE)) < 0) {
-			sr_err("failed to detach kernel driver: %s",
-					libusb_error_name(ret));
-			return SR_ERR;
-		}
-	}
-#endif
-
-	if ((ret = libusb_set_configuration(usb->devhdl, USB_CONFIGURATION))) {
-		sr_err("Failed to set configuration: %s.", libusb_error_name(ret));
-		return SR_ERR;
-	}
-
-	if ((ret = libusb_claim_interface(usb->devhdl, USB_INTERFACE))) {
-		sr_err("Failed to claim interface: %s.", libusb_error_name(ret));
-		return SR_ERR;
-	}
-
-	return SR_OK;
-}
-
-static int dev_close(struct sr_dev_inst *sdi)
-{
-	struct sr_usb_dev_inst *usb;
-
-	usb = sdi->conn;
-
-	if (!usb->devhdl)
-		return SR_ERR_BUG;
-
-	libusb_release_interface(usb->devhdl, USB_INTERFACE);
-	libusb_close(usb->devhdl);
-	usb->devhdl = NULL;
-
-	return SR_OK;
-}
-
 static int config_get(uint32_t key, GVariant **data,
 	const struct sr_dev_inst *sdi, const struct sr_channel_group *cg)
 {
@@ -226,66 +180,32 @@ static int config_list(uint32_t key, GVariant **data,
 
 static int dev_acquisition_start(const struct sr_dev_inst *sdi)
 {
-	struct sr_dev_driver *di = sdi->driver;
-	struct drv_context *drvc;
 	struct dev_context *devc;
-	struct sr_usb_dev_inst *usb;
+	struct sr_serial_dev_inst *serial;
-	int len, ret;
+	uint8_t cmd;
-	unsigned char cmd[2];
 
-	drvc = di->context;
 	devc = sdi->priv;
-	usb = sdi->conn;
+	serial = sdi->conn;
 
 	sr_sw_limits_acquisition_start(&devc->limits);
 	devc->packet_len = 0;
-
-	/* Configure serial port parameters on USB-UART interface
-	 * chip inside the device (just baudrate 2400 actually). */
-	cmd[0] = 0x09;
-	cmd[1] = 0x60;
-	ret = libusb_control_transfer(usb->devhdl, 0x21, 0x09, 0x0300, 0x00,
-			cmd, 2, 5);
-	if (ret != 2) {
-		sr_dbg("Failed to configure CH9325: %s", libusb_error_name(ret));
-		return SR_ERR;
-	}
-
 	std_session_send_df_header(sdi);
 
-	if (!(devc->xfer = libusb_alloc_transfer(0)))
-		return SR_ERR;
-
-	/* Length of payload to follow. */
-	cmd[0] = 0x01;
 	if (devc->data_source == DATA_SOURCE_LIVE)
-		cmd[1] = CMD_GET_LIVE;
+		cmd = CMD_GET_LIVE;
 	else
-		cmd[1] = CMD_GET_STORED;
+		cmd = CMD_GET_STORED;
+	serial_write_blocking(serial, &cmd, sizeof(cmd), 0);
 
-	ret = libusb_bulk_transfer(usb->devhdl, EP_OUT, cmd, 2, &len, 5);
+	serial_source_add(sdi->session, serial, G_IO_IN, 10,
-	if (ret != 0 || len != 2) {
+			ut32x_handle_events, (void *)sdi);
-		sr_dbg("Failed to start acquisition: %s", libusb_error_name(ret));
-		libusb_free_transfer(devc->xfer);
-		return SR_ERR;
-	}
-
-	libusb_fill_bulk_transfer(devc->xfer, usb->devhdl, EP_IN, devc->buf,
-			8, uni_t_ut32x_receive_transfer, (void *)sdi, 15);
-	if (libusb_submit_transfer(devc->xfer) != 0) {
-		libusb_free_transfer(devc->xfer);
-		return SR_ERR;
-	}
-
-	usb_source_add(sdi->session, drvc->sr_ctx, 10,
-			uni_t_ut32x_handle_events, (void *)sdi);
 
 	return SR_OK;
 }
 
 static int dev_acquisition_stop(struct sr_dev_inst *sdi)
 {
-	/* Signal USB transfer handler to clean up and stop. */
+	/* Have the reception routine stop the acquisition. */
 	sdi->status = SR_ST_STOPPING;
 
 	return SR_OK;
@@ -303,8 +223,8 @@ static struct sr_dev_driver uni_t_ut32x_driver_info = {
 	.config_get = config_get,
 	.config_set = config_set,
 	.config_list = config_list,
-	.dev_open = dev_open,
+	.dev_open = std_serial_dev_open,
-	.dev_close = dev_close,
+	.dev_close = std_serial_dev_close,
 	.dev_acquisition_start = dev_acquisition_start,
 	.dev_acquisition_stop = dev_acquisition_stop,
 	.context = NULL,

src/hardware/uni-t-ut32x/protocol.c

@@ -212,88 +212,65 @@ static int process_buffer(struct sr_dev_inst *sdi)
 	return 0;
 }
 
-SR_PRIV void LIBUSB_CALL uni_t_ut32x_receive_transfer(struct libusb_transfer *transfer)
+/* Gets invoked when RX data is available. */
+static int ut32x_receive_data(struct sr_dev_inst *sdi)
 {
 	struct dev_context *devc;
-	struct sr_dev_inst *sdi;
+	struct sr_serial_dev_inst *serial;
-	int hid_payload_len, ret;
+	size_t len;
 
-	sdi = transfer->user_data;
 	devc = sdi->priv;
-	if (transfer->actual_length == 8) {
+	serial = sdi->conn;
-		/* CH9325 encodes length in low nibble of first byte, with
+
-		 * bytes 1-7 being the (padded) payload. */
-		hid_payload_len = transfer->buffer[0] & 0x0f;
-		memcpy(devc->packet + devc->packet_len, transfer->buffer + 1,
-				hid_payload_len);
-		devc->packet_len += hid_payload_len;
 	/*
 	 * Discard receive data when the buffer is exhausted. This shall
 	 * allow to (re-)synchronize to the data stream when we find it
-		 * in an arbitrary state. Check the receive buffer for packets.
+	 * in an arbitrary state. Drain more data from the serial port,
+	 * and check the receive buffer for packets.
 	 */
 	if (devc->packet_len == sizeof(devc->packet)) {
 		process_packet(sdi, &devc->packet[0], devc->packet_len);
 		devc->packet_len = 0;
 	}
+	len = sizeof(devc->packet) - devc->packet_len;
+	len = serial_read_nonblocking(serial,
+			&devc->packet[devc->packet_len], len);
+	if (!len)
+		return 0;
+
+	devc->packet_len += len;
 	process_buffer(sdi);
-	}
-
-	/* Get the next transfer (unless we're shutting down). */
-	if (sdi->status != SR_ST_STOPPING) {
-		if ((ret = libusb_submit_transfer(devc->xfer)) != 0) {
-			sr_dbg("Failed to resubmit transfer: %s", libusb_error_name(ret));
-			sdi->status = SR_ST_STOPPING;
-			libusb_free_transfer(devc->xfer);
-		}
-	} else
-		libusb_free_transfer(devc->xfer);
 
+	return 0;
 }
 
-SR_PRIV int uni_t_ut32x_handle_events(int fd, int revents, void *cb_data)
+/* Gets periodically invoked by the glib main loop. */
+SR_PRIV int ut32x_handle_events(int fd, int revents, void *cb_data)
 {
-	struct drv_context *drvc;
-	struct dev_context *devc;
-	struct sr_dev_driver *di;
 	struct sr_dev_inst *sdi;
-	struct sr_usb_dev_inst *usb;
+	struct sr_serial_dev_inst *serial;
-	struct timeval tv;
+	uint8_t cmd;
-	int len, ret;
-	unsigned char cmd[2];
 
 	(void)fd;
-	(void)revents;
 
-	if (!(sdi = cb_data))
+	sdi = cb_data;
+	if (!sdi)
+		return TRUE;
+	serial = sdi->conn;
+	if (!serial)
 		return TRUE;
 
-	di = sdi->driver;
+	if (revents & G_IO_IN)
-	drvc = di->context;
+		ut32x_receive_data(sdi);
-
-	if (!(devc = sdi->priv))
-		return TRUE;
-
-	memset(&tv, 0, sizeof(struct timeval));
-	libusb_handle_events_timeout_completed(drvc->sr_ctx->libusb_ctx, &tv,
-			NULL);
 
 	if (sdi->status == SR_ST_STOPPING) {
-		usb_source_remove(sdi->session, drvc->sr_ctx);
+		serial_source_remove(sdi->session, serial);
 		std_session_send_df_end(sdi);
-
-		/* Tell the device to stop sending USB packets. */
-		usb = sdi->conn;
-		cmd[0] = 0x01;
-		cmd[1] = CMD_STOP;
-		ret = libusb_bulk_transfer(usb->devhdl, EP_OUT, cmd, 2, &len, 5);
-		if (ret != 0 || len != 2) {
-			/* Warning only, doesn't matter. */
-			sr_dbg("Failed to send stop command: %s", libusb_error_name(ret));
-		}
-
 		sdi->status = SR_ST_ACTIVE;
-		return TRUE;
+
+		/* Tell the device to stop sending data. */
+		cmd = CMD_STOP;
+		serial_write_blocking(serial, &cmd, sizeof(cmd), 0);
 	}
 
 	return TRUE;

src/hardware/uni-t-ut32x/protocol.h

@@ -28,12 +28,6 @@
 #define LOG_PREFIX "uni-t-ut32x"
 
 #define DEFAULT_DATA_SOURCE DATA_SOURCE_LIVE
-#define USB_CONN "1a86.e008"
-#define USB_INTERFACE 0
-#define USB_CONFIGURATION 1
-
-#define EP_IN (0x80 | 2)
-#define EP_OUT 2
 
 #define PACKET_SIZE	19
 
@@ -51,14 +45,10 @@ enum ut32x_cmd_code {
 struct dev_context {
 	struct sr_sw_limits limits;
 	enum ut32x_data_source data_source;
-	unsigned char buf[8];
+	uint8_t packet[PACKET_SIZE];
-	struct libusb_transfer *xfer;
-
-	unsigned char packet[32];
 	size_t packet_len;
 };
 
-SR_PRIV int uni_t_ut32x_handle_events(int fd, int revents, void *cb_data);
+SR_PRIV int ut32x_handle_events(int fd, int revents, void *cb_data);
-SR_PRIV void LIBUSB_CALL uni_t_ut32x_receive_transfer(struct libusb_transfer *transfer);
 
 #endif