From: Alexandru Gagniuc Date: Sat, 10 Nov 2012 01:52:45 +0000 (-0600) Subject: ols: Do not randomly probe serial ports X-Git-Tag: dsupstream~565 X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=a99e0d2a0c9d1bb4db5623ba50f83486238ee793;p=libsigrok.git ols: Do not randomly probe serial ports ols driver used to probe a series of available serial ports obtained by regexp matching of common serial port names. There are a number of problems with this approach: 1. It will probe all serial devices, including devices that do not like to be probed, potentially causing them to act up. 2. It will try to probe serial ports which may already be opened in other applications for other purposes. 3. It assumes the naming of the serial ports is set in stone, and creates an unnecessary OS-specific list. 4. It produces unnecessary debug output even when an OLS device is not connected. 5. etc... Do not implicitly probe serial ports. Only probe the port specified by the frontend, if any; otherwise, just quit. Also get rid of all functionality in serial.c which was designed specifically for random probing. Signed-off-by: Alexandru Gagniuc --- diff --git a/hardware/common/serial.c b/hardware/common/serial.c index 220533a4..239c7181 100644 --- a/hardware/common/serial.c +++ b/hardware/common/serial.c @@ -49,47 +49,6 @@ static HANDLE hdl; #endif -static const char *serial_port_glob[] = { - /* Linux */ - "/dev/ttyS*", - "/dev/ttyUSB*", - "/dev/ttyACM*", - /* MacOS X */ - "/dev/ttys*", - "/dev/tty.USB-*", - "/dev/tty.Modem-*", - NULL, -}; - -SR_PRIV GSList *list_serial_ports(void) -{ - GSList *ports; - - sr_dbg("Getting list of serial ports on the system."); - -#ifdef _WIN32 - /* TODO */ - ports = NULL; - ports = g_slist_append(ports, g_strdup("COM1")); -#else - glob_t g; - unsigned int i, j; - - ports = NULL; - for (i = 0; serial_port_glob[i]; i++) { - if (glob(serial_port_glob[i], 0, NULL, &g)) - continue; - for (j = 0; j < g.gl_pathc; j++) { - ports = g_slist_append(ports, g_strdup(g.gl_pathv[j])); - sr_dbg("Found serial port '%s'.", g.gl_pathv[j]); - } - globfree(&g); - } -#endif - - return ports; -} - /** * Open the specified serial port. * @@ -253,74 +212,6 @@ SR_PRIV int serial_read(int fd, void *buf, size_t count) #endif } -/** - * Create a backup of the current parameters of the specified serial port. - * - * @param fd File descriptor of the serial port. - * - * @return Pointer to a struct termios upon success, NULL upon errors. - * It is the caller's responsibility to g_free() the pointer if no - * longer needed. - */ -SR_PRIV void *serial_backup_params(int fd) -{ - sr_dbg("FD %d: Creating serial parameters backup.", fd); - -#ifdef _WIN32 - /* TODO */ -#else - struct termios *term; - - if (!(term = g_try_malloc(sizeof(struct termios)))) { - sr_err("termios struct malloc failed."); - return NULL; - } - - /* Returns 0 upon success, -1 upon failure. */ - if (tcgetattr(fd, term) < 0) { - sr_err("FD %d: Error getting serial parameters: %s.", - fd, strerror(errno)); - g_free(term); - return NULL; - } - - return term; -#endif -} - -/** - * Restore serial port settings from a previously created backup. - * - * @param fd File descriptor of the serial port. - * @param backup Pointer to a struct termios which contains the settings - * to restore. - * - * @return 0 upon success, -1 upon failure. - */ -SR_PRIV int serial_restore_params(int fd, void *backup) -{ - sr_dbg("FD %d: Restoring serial parameters from backup.", fd); - - if (!backup) { - sr_err("FD %d: Cannot restore serial params (NULL).", fd); - return -1; - } - -#ifdef _WIN32 - /* TODO */ -#else - int ret; - - /* Returns 0 upon success, -1 upon failure. */ - if ((ret = tcsetattr(fd, TCSADRAIN, (struct termios *)backup)) < 0) { - sr_err("FD %d: Error restoring serial parameters: %s.", - fd, strerror(errno)); - } - - return ret; -#endif -} - /** * Set serial parameters for the specified serial port. * diff --git a/hardware/openbench-logic-sniffer/ols.c b/hardware/openbench-logic-sniffer/ols.c index 6f914a8c..d8ae2eed 100644 --- a/hardware/openbench-logic-sniffer/ols.c +++ b/hardware/openbench-logic-sniffer/ols.c @@ -378,144 +378,124 @@ static int hw_init(void) static GSList *hw_scan(GSList *options) { + struct sr_hwopt *opt; + GSList *l, *devices; + const char *conn, *serialcomm; struct sr_dev_inst *sdi; struct drv_context *drvc; struct dev_context *devc; struct sr_probe *probe; - GSList *devices, *ports, *l; - GPollFD *fds, probefd; - int devcnt, final_devcnt, num_ports, fd, ret, i, j; - char buf[8], **dev_names, **serial_params; + GPollFD probefd; + int final_devcnt, fd, ret, i; + char buf[8]; (void)options; drvc = odi->priv; final_devcnt = 0; devices = NULL; - /* Scan all serial ports. */ - ports = list_serial_ports(); - num_ports = g_slist_length(ports); - - if (!(fds = g_try_malloc0(num_ports * sizeof(GPollFD)))) { - sr_err("ols: %s: fds malloc failed", __func__); - goto hw_init_free_ports; /* TODO: SR_ERR_MALLOC. */ + conn = serialcomm = NULL; + for (l = options; l; l = l->next) { + opt = l->data; + switch (opt->hwopt) { + case SR_HWOPT_CONN: + conn = opt->value; + break; + case SR_HWOPT_SERIALCOMM: + serialcomm = opt->value; + break; + } + } + if (!conn) { + sr_err("ols: No serial port specified."); + return NULL; } - if (!(dev_names = g_try_malloc(num_ports * sizeof(char *)))) { - sr_err("ols: %s: dev_names malloc failed", __func__); - goto hw_init_free_fds; /* TODO: SR_ERR_MALLOC. */ + if (serialcomm == NULL) { + serialcomm = g_strdup("115200/8n1"); } - if (!(serial_params = g_try_malloc(num_ports * sizeof(char *)))) { - sr_err("ols: %s: serial_params malloc failed", __func__); - goto hw_init_free_dev_names; /* TODO: SR_ERR_MALLOC. */ + /* The discovery procedure is like this: first send the Reset + * command (0x00) 5 times, since the device could be anywhere + * in a 5-byte command. Then send the ID command (0x02). + * If the device responds with 4 bytes ("OLS1" or "SLA1"), we + * have a match. + * + * Since it may take the device a while to respond at 115Kb/s, + * we do all the sending first, then wait for all of them to + * respond with g_poll(). + */ + sr_info("ols: probing %s .", conn); + fd = serial_open(conn, O_RDWR | O_NONBLOCK); + if (fd < 0) { + sr_err("ols: could not open port %s .", conn); + return NULL; } - devcnt = 0; - for (l = ports; l; l = l->next) { - /* The discovery procedure is like this: first send the Reset - * command (0x00) 5 times, since the device could be anywhere - * in a 5-byte command. Then send the ID command (0x02). - * If the device responds with 4 bytes ("OLS1" or "SLA1"), we - * have a match. - * - * Since it may take the device a while to respond at 115Kb/s, - * we do all the sending first, then wait for all of them to - * respond with g_poll(). - */ - sr_info("ols: probing %s...", (char *)l->data); - fd = serial_open(l->data, O_RDWR | O_NONBLOCK); - if (fd != -1) { - serial_params[devcnt] = serial_backup_params(fd); - serial_set_params(fd, 115200, 8, SERIAL_PARITY_NONE, 1, 2); - ret = SR_OK; - for (i = 0; i < 5; i++) { - if ((ret = send_shortcommand(fd, - CMD_RESET)) != SR_OK) { - /* Serial port is not writable. */ - break; - } - } - if (ret != SR_OK) { - serial_restore_params(fd, - serial_params[devcnt]); - serial_close(fd); - continue; - } - send_shortcommand(fd, CMD_ID); - fds[devcnt].fd = fd; - fds[devcnt].events = G_IO_IN; - dev_names[devcnt] = g_strdup(l->data); - devcnt++; + serial_set_paramstr(fd, serialcomm); + ret = SR_OK; + for (i = 0; i < 5; i++) { + if ((ret = send_shortcommand(fd, CMD_RESET)) != SR_OK) { + /* Serial port is not writable. */ + sr_err("ols: port %s is not writable.", conn); } - g_free(l->data); } + if (ret != SR_OK) { + serial_close(fd); + sr_err("ols: Could not use port %s. Quitting.", conn); + return NULL; + } + send_shortcommand(fd, CMD_ID); /* 2ms isn't enough for reliable transfer with pl2303, let's try 10 */ usleep(10000); - g_poll(fds, devcnt, 1); + g_poll(&probefd/*fds*/, 1/*devcnt*/, 1); - for (i = 0; i < devcnt; i++) { - if (fds[i].revents != G_IO_IN) - continue; - if (serial_read(fds[i].fd, buf, 4) != 4) - continue; - if (strncmp(buf, "1SLO", 4) && strncmp(buf, "1ALS", 4)) - continue; + if (probefd.revents != G_IO_IN) + return NULL; + if (serial_read(probefd.fd, buf, 4) != 4) + return NULL; + if (strncmp(buf, "1SLO", 4) && strncmp(buf, "1ALS", 4)) + return NULL; - /* definitely using the OLS protocol, check if it supports - * the metadata command - */ - send_shortcommand(fds[i].fd, CMD_METADATA); - probefd.fd = fds[i].fd; - probefd.events = G_IO_IN; - if (g_poll(&probefd, 1, 10) > 0) { - /* got metadata */ - sdi = get_metadata(fds[i].fd); - sdi->index = final_devcnt; - devc = sdi->priv; - } else { - /* not an OLS -- some other board that uses the sump protocol */ - sdi = sr_dev_inst_new(final_devcnt, SR_ST_INACTIVE, - "Sump", "Logic Analyzer", "v1.0"); - sdi->driver = odi; - devc = ols_dev_new(); - for (j = 0; j < 32; j++) { - if (!(probe = sr_probe_new(j, SR_PROBE_LOGIC, TRUE, - probe_names[j]))) - return 0; - sdi->probes = g_slist_append(sdi->probes, probe); - } - sdi->priv = devc; + /* definitely using the OLS protocol, check if it supports + * the metadata command + */ + send_shortcommand(probefd.fd, CMD_METADATA); + probefd.fd = fd; + probefd.events = G_IO_IN; + if (g_poll(&probefd, 1, 10) > 0) { + /* got metadata */ + sdi = get_metadata(fd); + sdi->index = final_devcnt; + devc = sdi->priv; + } else { + /* not an OLS -- some other board that uses the sump protocol */ + sdi = sr_dev_inst_new(final_devcnt, SR_ST_INACTIVE, + "Sump", "Logic Analyzer", "v1.0"); + sdi->driver = odi; + devc = ols_dev_new(); + for (i = 0; i < 32; i++) { + if (!(probe = sr_probe_new(i, SR_PROBE_LOGIC, TRUE, + probe_names[i]))) + return 0; + sdi->probes = g_slist_append(sdi->probes, probe); } - devc->serial = sr_serial_dev_inst_new(dev_names[i], -1); - drvc->instances = g_slist_append(drvc->instances, sdi); - devices = g_slist_append(devices, sdi); - - final_devcnt++; - serial_close(fds[i].fd); - fds[i].fd = 0; + sdi->priv = devc; } + devc->serial = sr_serial_dev_inst_new("FIXME", -1); + drvc->instances = g_slist_append(drvc->instances, sdi); + devices = g_slist_append(devices, sdi); + + final_devcnt++; + serial_close(probefd.fd); /* clean up after all the probing */ - for (i = 0; i < devcnt; i++) { - if (fds[i].fd != 0) { - serial_restore_params(fds[i].fd, serial_params[i]); - serial_close(fds[i].fd); - } - g_free(serial_params[i]); - g_free(dev_names[i]); + if (fd != 0) { + serial_close(fd); } - g_free(serial_params); -hw_init_free_dev_names: - g_free(dev_names); -hw_init_free_fds: - g_free(fds); -hw_init_free_ports: - g_slist_free(ports); - return devices; } diff --git a/libsigrok-internal.h b/libsigrok-internal.h index c63dcd46..6c54cf6d 100644 --- a/libsigrok-internal.h +++ b/libsigrok-internal.h @@ -124,14 +124,11 @@ SR_PRIV int sr_session_send(const struct sr_dev_inst *sdi, /*--- hardware/common/serial.c ----------------------------------------------*/ -SR_PRIV GSList *list_serial_ports(void); SR_PRIV int serial_open(const char *pathname, int flags); SR_PRIV int serial_close(int fd); SR_PRIV int serial_flush(int fd); SR_PRIV int serial_write(int fd, const void *buf, size_t count); SR_PRIV int serial_read(int fd, void *buf, size_t count); -SR_PRIV void *serial_backup_params(int fd); -SR_PRIV int serial_restore_params(int fd, void *backup); SR_PRIV int serial_set_params(int fd, int baudrate, int bits, int parity, int stopbits, int flowcontrol); SR_PRIV int serial_set_paramstr(int fd, const char *paramstr);