Menu
▾
▴
Re: [sigrok-devel] [PATCH 09/11] hameg-hmo: use the new scpi scan API
|
From: Damir J. <po...@po...> - 2014-02-05 13:29:36
|
On Tue, Feb 04, 2014 at 11:45:24PM +0100, Aurelien Jacobs wrote:
> ---
> hardware/hameg-hmo/api.c | 206 +----------------------------------------------
> 1 file changed, 2 insertions(+), 204 deletions(-)
>
> diff --git a/hardware/hameg-hmo/api.c b/hardware/hameg-hmo/api.c
> index 5abd8c5..241e86b 100644
> --- a/hardware/hameg-hmo/api.c
> +++ b/hardware/hameg-hmo/api.c
> @@ -18,7 +18,6 @@
> */
>
> #include <stdlib.h>
> -#include <glib/gstdio.h>
> #include "protocol.h"
>
> #define SERIALCOMM "115200/8n1/flow=1"
Here is the default SERIALCOMM parameter that I mentioned already.
> @@ -35,16 +34,6 @@ static const int32_t hwopts[] = {
> SR_CONF_SERIALCOMM,
> };
>
> -struct usb_id_info {
> - uint16_t vendor_id;
> - uint16_t product_id;
> -};
> -
> -static struct usb_id_info ho_models[] = {
> - { 0x0403, 0xed72 }, /* HO720 */
> - { 0x0403, 0xed73 }, /* HO730 */
> -};
> -
> enum {
> PG_INVALID = -1,
> PG_NONE,
> @@ -57,145 +46,6 @@ static int init(struct sr_context *sr_ctx)
> return std_init(sr_ctx, di, LOG_PREFIX);
> }
>
> -/**
> - * Find USB serial devices via the USB vendor ID and product ID.
> - *
> - * @param vendor_id Vendor ID of the USB device.
> - * @param product_id Product ID of the USB device.
> - *
> - * @return A GSList of strings containing the path of the serial device or
> - * NULL if no serial device is found. The returned list must be freed
> - * by the caller.
> - */
> -static GSList *auto_find_usb(uint16_t vendor_id, uint16_t product_id)
> -{
> -#ifdef __linux__
> - const gchar *usb_dev;
> - const char device_tree[] = "/sys/bus/usb/devices/";
> - GDir *devices_dir, *device_dir;
> - GSList *l = NULL;
> - GSList *tty_devs;
> - GSList *matched_paths;
> - FILE *fd;
> - char tmp[5];
> - gchar *vendor_path, *product_path, *path_copy;
> - gchar *prefix, *subdir_path, *device_path, *tty_path;
> - unsigned long read_vendor_id, read_product_id;
> - const char *file;
> -
> - l = NULL;
> - tty_devs = NULL;
> - matched_paths = NULL;
> -
> - if (!(devices_dir = g_dir_open(device_tree, 0, NULL)))
> - return NULL;
> -
> - /*
> - * Find potential candidates using the vendor ID and product ID
> - * and store them in matched_paths.
> - */
> - while ((usb_dev = g_dir_read_name(devices_dir))) {
> - vendor_path = g_strconcat(device_tree,
> - usb_dev, "/idVendor", NULL);
> - product_path = g_strconcat(device_tree,
> - usb_dev, "/idProduct", NULL);
> -
> - if (!g_file_test(vendor_path, G_FILE_TEST_EXISTS) ||
> - !g_file_test(product_path, G_FILE_TEST_EXISTS))
> - goto skip_device;
> -
> - if ((fd = g_fopen(vendor_path, "r")) == NULL)
> - goto skip_device;
> -
> - if (fgets(tmp, sizeof(tmp), fd) == NULL) {
> - fclose(fd);
> - goto skip_device;
> - }
> - read_vendor_id = strtoul(tmp, NULL, 16);
> -
> - fclose(fd);
> -
> - if ((fd = g_fopen(product_path, "r")) == NULL)
> - goto skip_device;
> -
> - if (fgets(tmp, sizeof(tmp), fd) == NULL) {
> - fclose(fd);
> - goto skip_device;
> - }
> - read_product_id = strtoul(tmp, NULL, 16);
> -
> - fclose(fd);
> -
> - if (vendor_id == read_vendor_id &&
> - product_id == read_product_id) {
> - path_copy = g_strdup(usb_dev);
> - matched_paths = g_slist_prepend(matched_paths,
> - path_copy);
> - }
> -
> -skip_device:
> - g_free(vendor_path);
> - g_free(product_path);
> - }
> - g_dir_close(devices_dir);
> -
> - /* For every matched device try to find a ttyUSBX subfolder. */
> - for (l = matched_paths; l; l = l->next) {
> - subdir_path = NULL;
> -
> - device_path = g_strconcat(device_tree, l->data, NULL);
> -
> - if (!(device_dir = g_dir_open(device_path, 0, NULL))) {
> - g_free(device_path);
> - continue;
> - }
> -
> - prefix = g_strconcat(l->data, ":", NULL);
> -
> - while ((file = g_dir_read_name(device_dir))) {
> - if (g_str_has_prefix(file, prefix)) {
> - subdir_path = g_strconcat(device_path,
> - "/", file, NULL);
> - break;
> - }
> - }
> - g_dir_close(device_dir);
> -
> - g_free(prefix);
> - g_free(device_path);
> -
> - if (subdir_path) {
> - if (!(device_dir = g_dir_open(subdir_path, 0, NULL))) {
> - g_free(subdir_path);
> - continue;
> - }
> - g_free(subdir_path);
> -
> - while ((file = g_dir_read_name(device_dir))) {
> - if (g_str_has_prefix(file, "ttyUSB")) {
> - tty_path = g_strconcat("/dev/",
> - file, NULL);
> - sr_dbg("Found USB device %04x:%04x attached to %s.",
> - vendor_id, product_id, tty_path);
> - tty_devs = g_slist_prepend(tty_devs,
> - tty_path);
> - break;
> - }
> - }
> - g_dir_close(device_dir);
> - }
> - }
> - g_slist_free_full(matched_paths, g_free);
> -
> - return tty_devs;
> -#else
> - (void)vendor_id;
> - (void)product_id;
> -
> - return NULL;
> -#endif
> -}
> -
> static int check_manufacturer(const char *manufacturer)
> {
> unsigned int i;
> @@ -207,26 +57,16 @@ static int check_manufacturer(const char *manufacturer)
> return SR_ERR;
> }
>
> -static struct sr_dev_inst *hmo_probe_serial_device(const char *serial_device,
> - const char *serial_options)
> +static struct sr_dev_inst *hmo_probe_serial_device(struct sr_scpi_dev_inst *scpi)
> {
> struct sr_dev_inst *sdi;
> struct dev_context *devc;
> struct sr_scpi_hw_info *hw_info;
> - struct sr_scpi_dev_inst *scpi;
>
> sdi = NULL;
> devc = NULL;
> - scpi = NULL;
> hw_info = NULL;
>
> - if (!(scpi = scpi_dev_inst_new(di->priv, serial_device, serial_options)))
> - goto fail;
> -
> - sr_info("Probing %s.", serial_device);
> - if (sr_scpi_open(scpi) != SR_OK)
> - goto fail;
> -
> if (sr_scpi_get_hw_id(scpi, &hw_info) != SR_OK) {
> sr_info("Couldn't get IDN response.");
> goto fail;
> @@ -275,49 +115,7 @@ fail:
>
> static GSList *scan(GSList *options)
> {
> - GSList *devices;
> - struct drv_context *drvc;
> - struct sr_dev_inst *sdi;
> - const char *serial_device, *serial_options;
> - GSList *l, *tty_devs;
> - unsigned int i;
> -
> - serial_device = NULL;
> - serial_options = SERIALCOMM;
Here it would be used and only overwritten if the user specifies the
options.
> - sdi = NULL;
> - devices = NULL;
> - drvc = di->priv;
> - drvc->instances = NULL;
> -
> - if (sr_serial_extract_options(options, &serial_device,
> - &serial_options) == SR_OK) {
> - sdi = hmo_probe_serial_device(serial_device, serial_options);
> - if (sdi != NULL) {
> - devices = g_slist_append(devices, sdi);
> - drvc->instances = g_slist_append(drvc->instances, sdi);
> - }
> - } else {
> - tty_devs = NULL;
> -
> - for (i = 0; i < ARRAY_SIZE(ho_models); i++) {
> - if ((l = auto_find_usb(ho_models[i].vendor_id,
> - ho_models[i].product_id)) == NULL)
> - continue;
> - tty_devs = g_slist_concat(tty_devs, l);
> - }
> -
> - for (l = tty_devs; l; l = l->next) {
> - sdi = hmo_probe_serial_device(l->data, serial_options);
> - if (sdi != NULL) {
> - devices = g_slist_append(devices, sdi);
> - drvc->instances = g_slist_append(drvc->instances, sdi);
> - }
> - }
> -
> - g_slist_free_full(tty_devs, g_free);
> - }
> -
> - return devices;
> + return sr_scpi_scan(di->priv, options, hmo_probe_serial_device);
> }
>
How to fix this? You could pass the default options to scpi_scan() or
you could pair them with the USB VID/PID. I don't know which option should be
preferred, if any, maybe someone has a better idea.
Despite of this small issue I think this is really great work, it moves us
closer to having common driver/scpi boiler plate separated from the drivers.
> static GSList *dev_list(void)
> --
> 1.9.rc1
|