From: Gerhard Sittig <redacted>
Date: Fri, 6 Oct 2017 00:52:29 +0000 (+0100)
Subject: uni-t-ut32x: migrate from USB transfers to serial-over-HID communication
X-Git-Url: http://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=164c5ae537f1ae60641cd496b8332436b49c6557

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.
---

diff --git a/configure.ac b/configure.ac
index 1d146283..9601dbe3 100644
--- a/configure.ac
+++ b/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])
diff --git a/src/hardware/uni-t-ut32x/api.c b/src/hardware/uni-t-ut32x/api.c
index 47211fa4..199fe473 100644
--- a/src/hardware/uni-t-ut32x/api.c
+++ b/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,108 +56,61 @@ static const char *data_sources[] = {
 
 static GSList *scan(struct sr_dev_driver *di, GSList *options)
 {
-	struct drv_context *drvc;
-	struct dev_context *devc;
-	struct sr_dev_inst *sdi;
+	const char *conn, *serialcomm;
 	struct sr_config *src;
-	GSList *usb_devices, *devices, *l;
-	unsigned int i;
-	const char *conn;
-
-	drvc = di->context;
+	GSList *l, *devices;
+	struct sr_serial_dev_inst *serial;
+	int rc;
+	struct sr_dev_inst *sdi;
+	struct dev_context *devc;
+	size_t i;
 
-	conn = "1a86.e008";
+	conn = "hid/ch9325";
+	serialcomm = "2400/8n1";
 	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))) {
-		/* We have a list of sr_usb_dev_inst matching the connection
-		 * string. Wrap them in sr_dev_inst and we're done. */
-		for (l = usb_devices; l; l = l->next) {
-			sdi = g_malloc0(sizeof(struct sr_dev_inst));
-			sdi->status = SR_ST_INACTIVE;
-			sdi->vendor = g_strdup("UNI-T");
-			sdi->model = g_strdup("UT32x");
-			sdi->inst_type = SR_INST_USB;
-			sdi->conn = l->data;
-			for (i = 0; i < ARRAY_SIZE(channel_names); i++)
-				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);
-		}
-		g_slist_free(usb_devices);
-	} else
-		g_slist_free_full(usb_devices, g_free);
-
-	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;
-		}
+	serial = sr_serial_dev_inst_new(conn, serialcomm);
+	rc = serial_open(serial, SERIAL_RDWR);
+	serial_flush(serial);
+	/* Cannot query/identify the device. Successful open shall suffice. */
+	serial_close(serial);
+	if (rc != SR_OK) {
+		sr_serial_dev_inst_free(serial);
+		return devices;
 	}
-#endif
 
-	if ((ret = libusb_set_configuration(usb->devhdl, USB_CONFIGURATION))) {
-		sr_err("Failed to set configuration: %s.", libusb_error_name(ret));
-		return SR_ERR;
+	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_SERIAL;
+	sdi->conn = serial;
+	devc = g_malloc0(sizeof(*devc));
+	sdi->priv = devc;
+	sr_sw_limits_init(&devc->limits);
+	devc->data_source = DEFAULT_DATA_SOURCE;
+	for (i = 0; i < ARRAY_SIZE(channel_names); i++) {
+		sr_channel_new(sdi, i, SR_CHANNEL_ANALOG, TRUE,
+				channel_names[i]);
 	}
+	devices = g_slist_append(devices, sdi);
 
-	if ((ret = libusb_claim_interface(usb->devhdl, USB_INTERFACE))) {
-		sr_err("Failed to claim interface: %s.", libusb_error_name(ret));
-		return SR_ERR;
-	}
+	serial_close(serial);
+	if (!devices)
+		sr_serial_dev_inst_free(serial);
 
-	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;
+	return std_scan_complete(di, devices);
 }
 
 static int config_get(uint32_t key, GVariant **data,
@@ -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;
-	int len, ret;
-	unsigned char cmd[2];
+	struct sr_serial_dev_inst *serial;
+	uint8_t cmd;
 
-	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;
-
-	ret = libusb_bulk_transfer(usb->devhdl, EP_OUT, cmd, 2, &len, 5);
-	if (ret != 0 || len != 2) {
-		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;
-	}
+		cmd = CMD_GET_STORED;
+	serial_write_blocking(serial, &cmd, sizeof(cmd), 0);
 
-	usb_source_add(sdi->session, drvc->sr_ctx, 10,
-			uni_t_ut32x_handle_events, (void *)sdi);
+	serial_source_add(sdi->session, serial, G_IO_IN, 10,
+			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_close = dev_close,
+	.dev_open = std_serial_dev_open,
+	.dev_close = std_serial_dev_close,
 	.dev_acquisition_start = dev_acquisition_start,
 	.dev_acquisition_stop = dev_acquisition_stop,
 	.context = NULL,
diff --git a/src/hardware/uni-t-ut32x/protocol.c b/src/hardware/uni-t-ut32x/protocol.c
index 4da24662..bd5a5cb0 100644
--- a/src/hardware/uni-t-ut32x/protocol.c
+++ b/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;
-	int hid_payload_len, ret;
+	struct sr_serial_dev_inst *serial;
+	size_t len;
 
-	sdi = transfer->user_data;
 	devc = sdi->priv;
-	if (transfer->actual_length == 8) {
-		/* 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.
-		 */
-		if (devc->packet_len == sizeof(devc->packet)) {
-			process_packet(sdi, &devc->packet[0], devc->packet_len);
-			devc->packet_len = 0;
-		}
-		process_buffer(sdi);
+	serial = sdi->conn;
+
+	/*
+	 * 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. 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;
 
-	/* 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);
+	devc->packet_len += len;
+	process_buffer(sdi);
 
+	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 timeval tv;
-	int len, ret;
-	unsigned char cmd[2];
+	struct sr_serial_dev_inst *serial;
+	uint8_t cmd;
 
 	(void)fd;
-	(void)revents;
 
-	if (!(sdi = cb_data))
+	sdi = cb_data;
+	if (!sdi)
 		return TRUE;
-
-	di = sdi->driver;
-	drvc = di->context;
-
-	if (!(devc = sdi->priv))
+	serial = sdi->conn;
+	if (!serial)
 		return TRUE;
 
-	memset(&tv, 0, sizeof(struct timeval));
-	libusb_handle_events_timeout_completed(drvc->sr_ctx->libusb_ctx, &tv,
-			NULL);
+	if (revents & G_IO_IN)
+		ut32x_receive_data(sdi);
 
 	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;
diff --git a/src/hardware/uni-t-ut32x/protocol.h b/src/hardware/uni-t-ut32x/protocol.h
index eb10a65b..83a9da19 100644
--- a/src/hardware/uni-t-ut32x/protocol.h
+++ b/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];
-	struct libusb_transfer *xfer;
-
-	unsigned char packet[32];
+	uint8_t packet[PACKET_SIZE];
 	size_t packet_len;
 };
 
-SR_PRIV int uni_t_ut32x_handle_events(int fd, int revents, void *cb_data);
-SR_PRIV void LIBUSB_CALL uni_t_ut32x_receive_transfer(struct libusb_transfer *transfer);
+SR_PRIV int ut32x_handle_events(int fd, int revents, void *cb_data);
 
 #endif