]> sigrok.org Git - libsigrok.git/commitdiff
ols: Do not randomly probe serial ports
authorAlexandru Gagniuc <redacted>
Sat, 10 Nov 2012 01:52:45 +0000 (19:52 -0600)
committerBert Vermeulen <redacted>
Sun, 11 Nov 2012 02:12:10 +0000 (03:12 +0100)
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 <redacted>
hardware/common/serial.c
hardware/openbench-logic-sniffer/ols.c
libsigrok-internal.h

index 220533a41b753f9d6844801574963b1d95f4f657..239c718125a911fb8f74d9e2e8820f0f7d987ec0 100644 (file)
 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.
  *
index 6f914a8c8991851dac884897d025fb3de3b773fe..d8ae2eedbbe104a2be8ed8ebea387be58b7b19bb 100644 (file)
@@ -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;
 }
 
index c63dcd46b858e2a52e11ccb1bab4ffb322422854..6c54cf6d316e6351bfa50d1152679e569f9975d2 100644 (file)
@@ -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);