genericdmm: Factor out USB functions.

Move sr_usb_connect() and sr_usb_open() to hardware/common/usb.c in a
slightly more generic form and add more error checks and logging.

Let genericdmm use the new/moved functions.

hardware/common/Makefile.am

@@ -22,7 +22,7 @@ SUBDIRS = dmm
 # Local lib, this is NOT meant to be installed!
 noinst_LTLIBRARIES = libsigrokhwcommon.la
 
-libsigrokhwcommon_la_SOURCES =
+libsigrokhwcommon_la_SOURCES = usb.c
 
 if NEED_EZUSB
 libsigrokhwcommon_la_SOURCES += ezusb.c

hardware/common/usb.c

@@ -0,0 +1,191 @@
+/*
+ * This file is part of the sigrok project.
+ *
+ * Copyright (C) 2012 Uwe Hermann <uwe@hermann-uwe.de>
+ * Copyright (C) 2012 Bert Vermeulen <bert@biot.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+ */
+
+#include <stdlib.h>
+#include <glib.h>
+#include <libusb.h>
+#include "libsigrok.h"
+#include "libsigrok-internal.h"
+
+/* SR_HWCAP_CONN takes one of these: */
+#define CONN_USB_VIDPID  "^([0-9a-z]{1,4})\\.([0-9a-z]{1,4})$"
+#define CONN_USB_BUSADDR "^(\\d+)\\.(\\d+)$"
+
+/* Message logging helpers with driver-specific prefix string. */
+#define DRIVER_LOG_DOMAIN "usb: "
+#define sr_log(l, s, args...) sr_log(l, DRIVER_LOG_DOMAIN s, ## args)
+#define sr_spew(s, args...) sr_spew(DRIVER_LOG_DOMAIN s, ## args)
+#define sr_dbg(s, args...) sr_dbg(DRIVER_LOG_DOMAIN s, ## args)
+#define sr_info(s, args...) sr_info(DRIVER_LOG_DOMAIN s, ## args)
+#define sr_warn(s, args...) sr_warn(DRIVER_LOG_DOMAIN s, ## args)
+#define sr_err(s, args...) sr_err(DRIVER_LOG_DOMAIN s, ## args)
+
+SR_PRIV GSList *sr_usb_connect(libusb_context *usb_ctx, const char *conn)
+{
+	struct libusb_device **devlist;
+	struct libusb_device_descriptor des;
+	GSList *devices;
+	GRegex *reg;
+	GMatchInfo *match;
+	int vid, pid, bus, addr, b, a, ret, i;
+	char *mstr;
+
+	vid = pid = bus = addr = 0;
+	reg = g_regex_new(CONN_USB_VIDPID, 0, 0, NULL);
+	if (g_regex_match(reg, conn, 0, &match)) {
+		/* Extract VID. */
+		if ((mstr = g_match_info_fetch(match, 1))) {
+			vid = strtoul(mstr, NULL, 16);
+			sr_spew("Extracted VID 0x%04x.", vid);
+		}
+		g_free(mstr);
+
+		/* Extract PID. */
+		if ((mstr = g_match_info_fetch(match, 2))) {
+			pid = strtoul(mstr, NULL, 16);
+			sr_spew("Extracted PID 0x%04x.", pid);
+		}
+		g_free(mstr);
+		sr_dbg("Trying to find USB device with VID:PID = %04x:%04x.",
+		       vid, pid);
+	} else {
+		g_match_info_unref(match);
+		g_regex_unref(reg);
+		reg = g_regex_new(CONN_USB_BUSADDR, 0, 0, NULL);
+		if (g_regex_match(reg, conn, 0, &match)) {
+			/* Extract bus. */
+			if ((mstr = g_match_info_fetch(match, 1))) {
+				bus = strtoul(mstr, NULL, 16);
+				sr_spew("Extracted bus %d.", bus);
+			}
+			g_free(mstr);
+
+			/* Extract address. */
+			if ((mstr = g_match_info_fetch(match, 2))) {
+				addr = strtoul(mstr, NULL, 16);
+				sr_spew("Extracted address %d.", addr);
+			}
+			g_free(mstr);
+			sr_dbg("Trying to find USB device with bus.address = "
+			       "%d.%d.", bus, addr);
+		}
+	}
+	g_match_info_unref(match);
+	g_regex_unref(reg);
+
+	if (vid + pid + bus + addr == 0) {
+		sr_err("Neither VID:PID nor bus.address was found.");
+		return NULL;
+	}
+
+	if (bus > 64) {
+		sr_err("Invalid bus specified: %d.", bus);
+		return NULL;
+	}
+
+	if (addr > 127) {
+		sr_err("Invalid address specified: %d.", addr);
+		return NULL;
+	}
+
+	/* Looks like a valid USB device specification, but is it connected? */
+	devices = NULL;
+	libusb_get_device_list(usb_ctx, &devlist);
+	for (i = 0; devlist[i]; i++) {
+		if ((ret = libusb_get_device_descriptor(devlist[i], &des))) {
+			sr_err("Failed to get device descriptor: %d.", ret);
+			continue;
+		}
+
+		b = libusb_get_bus_number(devlist[i]);
+		a = libusb_get_device_address(devlist[i]);
+
+		if (vid + pid &&
+		    (des.idVendor != vid || des.idProduct != pid)) {
+			sr_spew("VID:PID = %04x:%04x (%d.%d) doesn't match.",
+				des.idVendor, des.idProduct, b, a);
+			continue;
+		}
+
+		if (bus + addr && (b != bus || a != addr)) {
+			sr_spew("bus.address = %d.%d does not match.", b, a);
+			continue;
+		}
+
+		sr_dbg("Found USB device (VID:PID = %04x:%04x, bus.address = "
+		       "%d.%d).", des.idVendor, des.idProduct, b, a);
+
+		devices = g_slist_append(devices, devlist[i]);
+	}
+	libusb_free_device_list(devlist, 1);
+
+	sr_dbg("Found %d device(s).", g_slist_length(devices));
+	
+	return devices;
+}
+
+SR_PRIV int sr_usb_open(libusb_context *usb_ctx, struct sr_usb_dev_inst *usb)
+{
+	struct libusb_device **devlist;
+	struct libusb_device_descriptor des;
+	int ret, r, cnt, i, a, b;
+
+	sr_dbg("Trying to open USB device.");
+
+	if ((cnt = libusb_get_device_list(usb_ctx, &devlist)) < 0) {
+		sr_err("Failed to retrieve device list: %s.",
+		       libusb_error_name(cnt));
+		return SR_ERR;
+	}
+
+	ret = SR_ERR;
+	for (i = 0; i < cnt; i++) {
+		if ((r = libusb_get_device_descriptor(devlist[i], &des)) < 0) {
+			sr_err("Failed to get device descriptor: %s.",
+			       libusb_error_name(r));
+			continue;
+		}
+
+		b = libusb_get_bus_number(devlist[i]);
+		a = libusb_get_device_address(devlist[i]);
+
+		if (b != usb->bus || a != usb->address) {
+			sr_spew("bus.address = %d.%d does not match.", b, a);
+			continue;
+		}
+
+		if ((r = libusb_open(devlist[i], &usb->devhdl)) < 0) {
+			sr_err("Failed to open device: %s.",
+			       libusb_error_name(r));
+			break;
+		}
+
+		sr_dbg("Opened USB device (VID:PID = %04x:%04x, bus.address = "
+		       "%d.%d).", des.idVendor, des.idProduct, b, a);
+
+		ret = SR_OK;
+		break;
+	}
+
+	libusb_free_device_list(devlist, 1);
+
+	return ret;
+}

hardware/genericdmm/api.c

@@ -66,87 +66,43 @@ static libusb_context *genericdmm_usb_context = NULL;
 static int hw_dev_acquisition_stop(const struct sr_dev_inst *sdi,
 		void *cb_data);
 
-
+static GSList *connect_serial(const char *conn, const char *serialcomm)
-static GSList *connect_usb(const char *conn)
 {
-	struct sr_dev_inst *sdi;
-	struct drv_context *drvc;
-	struct dev_context *devc;
-	struct sr_probe *probe;
-	libusb_device **devlist;
-	struct libusb_device_descriptor des;
 	GSList *devices;
-	GRegex *reg;
+
-	GMatchInfo *match;
+	devices = NULL;
-	int vid, pid, bus, addr, devcnt, err, i;
+
-	char *mstr;
+	/* TODO */
+	sr_dbg("Not yet implemented.");
+
+	return devices;
+}
+
+GSList *genericdmm_connect(const char *conn, const char *serialcomm)
+{
+	GSList *devices, *l;
+	struct sr_dev_inst *sdi;
+	struct dev_context *devc;
+	struct drv_context *drvc;
+	struct sr_probe *probe;
+	struct libusb_device *ldev;
+	int i, devcnt;
 
 	drvc = gdi->priv;
 
-	vid = pid = bus = addr = 0;
-	reg = g_regex_new(DMM_CONN_USB_VIDPID, 0, 0, NULL);
-	if (g_regex_match(reg, conn, 0, &match)) {
-		/* Extract VID. */
-		if ((mstr = g_match_info_fetch(match, 1)))
-			vid = strtoul(mstr, NULL, 16);
-		g_free(mstr);
-
-		/* Extract PID. */
-		if ((mstr = g_match_info_fetch(match, 2)))
-			pid = strtoul(mstr, NULL, 16);
-		g_free(mstr);
-	} else {
-		g_match_info_unref(match);
-		g_regex_unref(reg);
-		reg = g_regex_new(DMM_CONN_USB_BUSADDR, 0, 0, NULL);
-		if (g_regex_match(reg, conn, 0, &match)) {
-			/* Extract bus. */
-			if ((mstr = g_match_info_fetch(match, 0)))
-				bus = strtoul(mstr, NULL, 16);
-			g_free(mstr);
-
-			/* Extract address. */
-			if ((mstr = g_match_info_fetch(match, 0)))
-				addr = strtoul(mstr, NULL, 16);
-			g_free(mstr);
-		}
-	}
-	g_match_info_unref(match);
-	g_regex_unref(reg);
-
-	if (vid + pid + bus + addr == 0)
-		return NULL;
-
-	if (bus > 64) {
-		sr_err("Invalid bus.");
-		return NULL;
-	}
-
-	if (addr > 127) {
-		sr_err("Invalid address.");
-		return NULL;
-	}
-
-	/* Looks like a valid USB device specification, but is it connected? */
 	devices = NULL;
-	libusb_get_device_list(genericdmm_usb_context, &devlist);
+
-	for (i = 0; devlist[i]; i++) {
+	if (serialcomm)
-		if ((err = libusb_get_device_descriptor(devlist[i], &des))) {
+		/* Must be a serial port. */
-			sr_err("Failed to get device descriptor: %d.", err);
+		return connect_serial(conn, serialcomm);
-			continue;
+
+	if (!(l = sr_usb_connect(genericdmm_usb_context, conn))) {
+		return NULL;
 	}
 
-		if (vid + pid && (des.idVendor != vid || des.idProduct != pid))
+	for (i = 0; i < (int)g_slist_length(l); i++) {
-			/* VID/PID specified, but no match. */
+		ldev = (struct libusb_device *)l->data;
-			continue;
 
-		if (bus + addr && (
-				libusb_get_bus_number(devlist[i]) != bus
-				|| libusb_get_device_address(devlist[i]) != addr))
-			/* Bus/address specified, but no match. */
-			continue;
-
-		/* Found one. */
 		if (!(devc = g_try_malloc0(sizeof(struct dev_context)))) {
 			sr_err("Device context malloc failed.");
 			return NULL;
@@ -163,38 +119,13 @@ static GSList *connect_usb(const char *conn)
 			return NULL;
 		sdi->probes = g_slist_append(sdi->probes, probe);
 		devc->usb = sr_usb_dev_inst_new(
-				libusb_get_bus_number(devlist[i]),
+				libusb_get_bus_number(ldev),
-				libusb_get_device_address(devlist[i]), NULL);
+				libusb_get_device_address(ldev), NULL);
 		devices = g_slist_append(devices, sdi);
-	}
-	libusb_free_device_list(devlist, 1);
 
 		return devices;
 	}
 
-static GSList *connect_serial(const char *conn, const char *serialcomm)
-{
-	GSList *devices;
-
-	devices = NULL;
-
-	/* TODO */
-	sr_dbg("Not yet implemented.");
-
-	return devices;
-}
-
-GSList *genericdmm_connect(const char *conn, const char *serialcomm)
-{
-	GSList *devices;
-
-	if (serialcomm)
-		/* Must be a serial port. */
-		return connect_serial(conn, serialcomm);
-
-	if ((devices = connect_usb(conn)))
-		return devices;
-
 	return NULL;
 }
 
@@ -223,52 +154,6 @@ static GSList *default_scan(GSList *options)
 	return devices;
 }
 
-static int open_usb(struct sr_dev_inst *sdi)
-{
-	libusb_device **devlist;
-	struct libusb_device_descriptor des;
-	struct dev_context *devc;
-	int ret, tmp, cnt, i;
-
-	devc = sdi->priv;
-
-	if (sdi->status == SR_ST_ACTIVE)
-		/* already in use */
-		return SR_ERR;
-
-	cnt = libusb_get_device_list(genericdmm_usb_context, &devlist);
-	if (cnt < 0) {
-		sr_err("Failed to retrieve device list (%d).", cnt);
-		return SR_ERR;
-	}
-
-	ret = SR_ERR;
-	for (i = 0; i < cnt; i++) {
-		if ((tmp = libusb_get_device_descriptor(devlist[i], &des))) {
-			sr_err("Failed to get device descriptor: %d.", tmp);
-			continue;
-		}
-
-		if (libusb_get_bus_number(devlist[i]) != devc->usb->bus
-			|| libusb_get_device_address(devlist[i]) != devc->usb->address)
-			/* this is not the one */
-			continue;
-
-		if ((tmp = libusb_open(devlist[i], &devc->usb->devhdl))) {
-			sr_err("Failed to open device: %d.", tmp);
-			break;
-		}
-
-		sr_info("Opened device %s on %d.%d.", devc->profile->modelid,
-			devc->usb->bus, devc->usb->address);
-		ret = SR_OK;
-		break;
-	}
-	libusb_free_device_list(devlist, 1);
-
-	return ret;
-}
-
 static int clear_instances(void)
 {
 	GSList *l;
@@ -458,7 +343,12 @@ static int hw_dev_open(struct sr_dev_inst *sdi)
 	ret = SR_OK;
 	switch (devc->profile->transport) {
 	case DMM_TRANSPORT_USBHID:
-		ret = open_usb(sdi);
+		if (sdi->status == SR_ST_ACTIVE) {
+			sr_err("Device already in use.");
+			ret = SR_ERR;
+		} else {
+			ret = sr_usb_open(genericdmm_usb_context, devc->usb);
+		}
 		break;
 	case DMM_TRANSPORT_SERIAL:
 		sr_dbg("Opening serial port '%s'.", devc->serial->port);

libsigrok-internal.h

@@ -148,6 +148,13 @@ SR_PRIV int ezusb_upload_firmware(libusb_device *dev, int configuration,
 				  const char *filename);
 #endif
 
+/*--- hardware/common/usb.c -------------------------------------------------*/
+
+#ifdef HAVE_LIBUSB_1_0
+SR_PRIV GSList *sr_usb_connect(libusb_context *usb_ctx, const char *conn);
+SR_PRIV int sr_usb_open(libusb_context *usb_ctx, struct sr_usb_dev_inst *usb);
+#endif
+
 /*--- hardware/common/dmm/fs9922.c ------------------------------------------*/
 
 SR_PRIV int sr_dmm_parse_fs9922(const uint8_t *buf, float *floatval,