]> sigrok.org Git - libsigrok.git/commitdiff
device: Add Doxygen comments, improve error handling.
authorUwe Hermann <redacted>
Wed, 28 Dec 2011 15:19:15 +0000 (16:19 +0100)
committerUwe Hermann <redacted>
Wed, 28 Dec 2011 15:55:24 +0000 (16:55 +0100)
 - Add Doxygen comments for all functions (some TODOs remain).

 - Check return code of more functions, handle invalid input.

backend.c
device.c

index c3c3087a9d11315a28744db990cc6da4bf31f7f2..fc914e18cd47710ee193f0d6045b032f9a2e6009 100644 (file)
--- a/backend.c
+++ b/backend.c
@@ -38,7 +38,6 @@ int sr_init(void)
  */
 int sr_exit(void)
 {
-
        sr_cleanup_hwplugins();
 
        return SR_OK;
index 6921ceb0fad60243a8eae13e06adfc2434af4167..815e8066d549dc19b829aab78892b03a53150202 100644 (file)
--- a/device.c
+++ b/device.c
@@ -26,13 +26,47 @@ extern struct sr_global *global;
 
 GSList *devices = NULL;
 
-
+/**
+ * Scan the system for attached logic analyzers / devices.
+ *
+ * This will try to autodetect all supported logic analyzer devices:
+ *
+ *  - Those attached via USB (can be reliably detected via USB VID/PID).
+ *
+ *  - Those using a (real or virtual) serial port (detected by sending
+ *    device-specific commands to all OS-specific serial port devices such
+ *    as /dev/ttyS*, /dev/ttyUSB*, /dev/ttyACM*, and others).
+ *    The autodetection for this kind of devices can potentially be unreliable.
+ *
+ *    Also, sending various bytes/commands to (all!) devices which happen to
+ *    be attached to the system via a (real or virtual) serial port can be
+ *    problematic. There is no way for libsigrok to know how unknown devices
+ *    react to the bytes libsigrok sends. Potentially they could lead to the
+ *    device getting into invalid/error states, losing/overwriting data, or...
+ *
+ * In addition to the detection, the devices that are found are also
+ * initialized automatically. On some devices, this involves a firmware upload,
+ * or other such measures.
+ *
+ * The order in which the system is scanned for devices is not specified. The
+ * caller should not assume or rely on any specific order.
+ *
+ * After the system has been scanned for devices, the list of detected (and
+ * supported) devices can be acquired via sr_device_list().
+ *
+ * TODO: Should return int.
+ * TODO: Error checks?
+ * TODO: Option to only scan for specific devices or device classes.
+ */
 void sr_device_scan(void)
 {
        GSList *plugins, *l;
        struct sr_device_plugin *plugin;
 
-       plugins = sr_list_hwplugins();
+       if (!(plugins = sr_list_hwplugins())) {
+               sr_err("dev: %s: no supported devices/hwplugins", __func__);
+               return; /* TODO? */
+       }
 
        /*
         * Initialize all plugins first. Since the init() call may involve
@@ -41,26 +75,62 @@ void sr_device_scan(void)
         */
        for (l = plugins; l; l = l->next) {
                plugin = l->data;
+               /* TODO: Handle 'plugin' being NULL. */
                sr_init_hwplugins(plugin);
        }
-
 }
 
+/**
+ * Return the list of logic analyzer devices libsigrok has detected.
+ *
+ * If the libsigrok-internal device list is empty, a scan for attached
+ * devices -- via a call to sr_device_scan() -- is performed first.
+ *
+ * TODO: Error handling?
+ *
+ * @return The list (GSList) of detected devices, or NULL if none were found.
+ */
 GSList *sr_device_list(void)
 {
-
        if (!devices)
                sr_device_scan();
 
        return devices;
 }
 
+/**
+ * Create a new device.
+ *
+ * TODO: 'plugin' can be const.
+ * TODO: num_probes should be uint16_t.
+ * TODO: Should return int, so that we can return SR_OK, SR_ERR_* etc.
+ *
+ * It is the caller's responsibility to g_free() the allocated memory when
+ * no longer needed. TODO: Using which API function?
+ *
+ * @param plugin TODO.
+ *               If 'plugin' is NULL, the created device is a "virtual" one.
+ * @param plugin_index TODO
+ * @param num_probes The number of probes (>= 1) this device has.
+ *                   TODO: 0 allowed?
+ *
+ * @return Pointer to the newly allocated device, or NULL upon errors.
+ */
 struct sr_device *sr_device_new(struct sr_device_plugin *plugin, int plugin_index,
-                            int num_probes)
+                               int num_probes)
 {
        struct sr_device *device;
        int i;
 
+       if (!plugin) {
+               sr_err("dev: %s: plugin was NULL", __func__);
+               return NULL; /* TODO: SR_ERR_ARG */
+       }
+
+       /* TODO: Check if plugin_index valid? */
+
+       /* TODO: Check if num_probes valid? */
+
        if (!(device = g_try_malloc0(sizeof(struct sr_device)))) {
                sr_err("dev: %s: device malloc failed", __func__);
                return NULL;
@@ -71,53 +141,134 @@ struct sr_device *sr_device_new(struct sr_device_plugin *plugin, int plugin_inde
        devices = g_slist_append(devices, device);
 
        for (i = 0; i < num_probes; i++)
-               sr_device_probe_add(device, NULL);
+               sr_device_probe_add(device, NULL); /* TODO: Check return value. */
 
        return device;
 }
 
+/**
+ * Clear all probes of the specified device.
+ *
+ * This removes/clears the 'name' and 'trigger' fields of all probes of
+ * the device.
+ *
+ * The order in which the probes are cleared is not specified. The caller
+ * should not assume or rely on a specific order.
+ *
+ * TODO: Should return int.
+ * TODO: Rename to sr_device_clear_probes() or sr_device_probe_clear_all().
+ *
+ * @param device The device whose probes to clear. Must not be NULL.
+ *               Note: device->probes is allowed to be NULL (in that case,
+ *               there are no probes, thus none have to be cleared).
+ */
 void sr_device_clear(struct sr_device *device)
 {
        unsigned int pnum;
 
-       if (!device->probes)
-               return;
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return; /* TODO: SR_ERR_ARG. */
+       }
+
+       /* Note: device->probes can be NULL, this is handled correctly. */
 
        for (pnum = 1; pnum <= g_slist_length(device->probes); pnum++)
                sr_device_probe_clear(device, pnum);
+
+       /* TODO: return SR_OK; */
 }
 
+/**
+ * Clear the specified probe in the specified device.
+ *
+ * The probe itself still exists afterwards, but its 'name' and 'trigger'
+ * fields are g_free()'d and set to NULL.
+ *
+ * TODO: Should return int.
+ *
+ * @param device The device in which the specified (to be cleared) probe
+ *               resides. Must not be NULL.
+ * @param probenum The number of the probe to clear.
+ *                 Note that the probe numbers start at 1 (not 0!).
+ */
 void sr_device_probe_clear(struct sr_device *device, int probenum)
 {
        struct sr_probe *p;
 
-       p = sr_device_probe_find(device, probenum);
-       if (!p)
-               return;
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return; /* TODO: SR_ERR_ARG. */
+       }
+
+       /* TODO: Sanity check on 'probenum'. */
+
+       if (!(p = sr_device_probe_find(device, probenum))) {
+               sr_err("dev: %s: probe %d not found", __func__, probenum);
+               return; /* TODO: SR_ERR*. */
+       }
 
+       /* If the probe has a name, remove it. */
        if (p->name) {
                g_free(p->name);
                p->name = NULL;
        }
 
+       /* If the probe has a trigger, remove it. */
        if (p->trigger) {
                g_free(p->trigger);
                p->trigger = NULL;
        }
+
+       /* TODO: return SR_OK; */
 }
 
+/**
+ * Add a probe with the specified name to the specified device.
+ *
+ * The added probe is automatically enabled (the 'enabled' field is TRUE).
+ *
+ * The 'trigger' field of the added probe is set to NULL. A trigger can be
+ * added via sr_device_trigger_set().
+ *
+ * TODO: Should return int.
+ * TODO: Are duplicate names allowed?
+ * TODO: Do we enforce a maximum probe number for a device?
+ * TODO: Error if the max. probe number for the specific LA is reached, e.g.
+ *       if the caller tries to add more probes than the device actually has.
+ *
+ * @param device The device to which to add a probe with the specified name.
+ *               Must not be NULL.
+ * @param name The name of the probe to add to this device. Must not be NULL.
+ *             TODO: Maximum length, allowed characters, etc.
+ *
+ * @return SR_OK upon success, SR_ERR_MALLOC upon memory allocation errors,
+ *         or SR_ERR_ARG upon invalid arguments.
+ *         If something other than SR_OK is returned, 'device' is unchanged.
+ */
 void sr_device_probe_add(struct sr_device *device, const char *name)
 {
        struct sr_probe *p;
-       char probename[16];
+       char probename[16]; /* FIXME: Don't hardcode 16? #define? */
        int probenum;
 
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return; /* SR_ERR_ARG; */
+       }
+
+       if (!name) {
+               sr_err("dev: %s: name was NULL", __func__);
+               return; /* SR_ERR_ARG; */
+       }
+
+       /* TODO: Further checks to ensure name is valid. */
+
        probenum = g_slist_length(device->probes) + 1;
 
        if (!(p = g_try_malloc0(sizeof(struct sr_probe)))) {
                sr_err("dev: %s: p malloc failed", __func__);
-               // return SR_ERR_MALLOC;
-               return; /* FIXME: should return int. */
+               return; /* SR_ERR_MALLOC; */
        }
 
        p->index = probenum;
@@ -130,16 +281,42 @@ void sr_device_probe_add(struct sr_device *device, const char *name)
        }
        p->trigger = NULL;
        device->probes = g_slist_append(device->probes, p);
+
+       return SR_OK;
 }
 
+/**
+ * Find the probe with the specified number in the specified device.
+ *
+ * TODO
+ *
+ * @param device TODO. Must not be NULL.
+ * @param probenum The number of the probe whose 'struct sr_probe' we want.
+ *                 Note that the probe numbers start at 1 (not 0!).
+ *
+ * TODO: Should return int.
+ * TODO: device can be const.
+ * TODO: probenum should be unsigned.
+ *
+ * @return A pointer to the requested probe's 'struct sr_probe', or NULL
+ *         if the probe could not be found.
+ */
 struct sr_probe *sr_device_probe_find(struct sr_device *device, int probenum)
 {
        GSList *l;
        struct sr_probe *p, *found_probe;
 
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return NULL; /* TODO: SR_ERR_ARG */
+       }
+
+       /* TODO: Sanity check on probenum. */
+
        found_probe = NULL;
        for (l = device->probes; l; l = l->next) {
                p = l->data;
+               /* TODO: Check for p != NULL. */
                if (p->index == probenum) {
                        found_probe = p;
                        break;
@@ -149,32 +326,74 @@ struct sr_probe *sr_device_probe_find(struct sr_device *device, int probenum)
        return found_probe;
 }
 
-/* TODO: return SR_ERR if probenum not found */
+/**
+ * Set the name of the specified probe in the specified device.
+ *
+ * If the probe already has a different name assigned to it, it will be
+ * removed, and the new name will be saved instead.
+ *
+ * TODO: Should return int.
+ * TODO: device can be const?
+ * TODO: Rename to sr_device_set_probe_name().
+ *
+ * @param device TODO
+ * @param probenum The number of the probe whose name to set.
+ *                 Note that the probe numbers start at 1 (not 0!).
+ * @param name The new name that the specified probe should get.
+ */
 void sr_device_probe_name(struct sr_device *device, int probenum,
                          const char *name)
 {
        struct sr_probe *p;
 
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return; /* TODO: SR_ERR_ARG */
+       }
+
        p = sr_device_probe_find(device, probenum);
-       if (!p)
-               return;
+       if (!p) {
+               sr_err("dev: %s: probe %d not found", __func__, probenum);
+               return; /* TODO: SR_ERR*. */
+       }
+
+       /* TODO: Sanity check on 'name'. */
 
+       /* If the probe already has a name, kill it first. */
        if (p->name)
                g_free(p->name);
+
        p->name = g_strdup(name);
 }
 
-/* TODO: return SR_ERR if probenum not found */
+/**
+ * Remove all triggers set up for the specified device.
+ *
+ * TODO: Better description.
+ *
+ * TODO: Should return int.
+ * TODO: device can be const?
+ *
+ * @param device TODO
+ */
 void sr_device_trigger_clear(struct sr_device *device)
 {
        struct sr_probe *p;
-       unsigned int pnum;
+       unsigned int pnum; /* TODO: uint6_t? */
 
-       if (!device->probes)
-               return;
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return; /* TODO: SR_ERR_ARG */
+       }
+
+       if (!device->probes) {
+               sr_err("dev: %s: device->probes was NULL", __func__);
+               return; /* TODO: SR_ERR*. */
+       }
 
        for (pnum = 1; pnum <= g_slist_length(device->probes); pnum++) {
                p = sr_device_probe_find(device, pnum);
+               /* TODO: Silently ignore probes which cannot be found? */
                if (p && p->trigger) {
                        g_free(p->trigger);
                        p->trigger = NULL;
@@ -182,34 +401,92 @@ void sr_device_trigger_clear(struct sr_device *device)
        }
 }
 
-/* TODO: return SR_ERR if probenum not found */
+/**
+ * Add a trigger to the specified device.
+ *
+ * TODO: Better description.
+ * TODO: Describe valid format of the 'trigger' string.
+ *
+ * TODO: Should return int.
+ * TODO: device can be const?
+ *
+ * @param device TODO. Must not be NULL.
+ * @param probenum The number of the probe. TODO.
+ *                 Note that the probe numbers start at 1 (not 0!).
+ * @param trigger TODO.
+ *                TODO: Is NULL allowed?
+ */
 void sr_device_trigger_set(struct sr_device *device, int probenum,
                           const char *trigger)
 {
        struct sr_probe *p;
 
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return; /* TODO: SR_ERR_ARG */
+       }
+
+       /* TODO: Sanity check on 'probenum'. */
+
+       /* TODO: Sanity check on 'trigger'. */
+
        p = sr_device_probe_find(device, probenum);
-       if (!p)
-               return;
+       if (!p) {
+               sr_err("dev: %s: probe %d not found", __func__, probenum);
+               return; /* TODO: SR_ERR*. */
+       }
 
+       /* If the probe already has a trigger, kill it first. */
        if (p->trigger)
                g_free(p->trigger);
 
        p->trigger = g_strdup(trigger);
-
 }
 
+/**
+ * Determine whether the specified device has the specified capability.
+ *
+ * TODO: Should return int?
+ * TODO: device can be const.
+ *
+ * @param device Pointer to the device to be checked. Must not be NULL.
+ *               The device's 'plugin' field must not be NULL either.
+ * @param hwcap The capability that should be checked (whether it's supported
+ *              by the specified device).
+ *
+ * @return TRUE, if the device has the specified capability, FALSE otherwise.
+ *         FALSE is also returned upon invalid input parameters or other
+ *         error conditions.
+ */
 gboolean sr_device_has_hwcap(struct sr_device *device, int hwcap)
 {
        int *capabilities, i;
 
-       if (!device || !device->plugin)
-               return FALSE;
+       if (!device) {
+               sr_err("dev: %s: device was NULL", __func__);
+               return FALSE; /* TODO: SR_ERR_ARG. */
+       }
+
+       if (!device->plugin) {
+               sr_err("dev: %s: device->plugin was NULL", __func__);
+               return FALSE; /* TODO: SR_ERR_ARG. */
+       }
+
+       /* TODO: Sanity check on 'hwcap'. */
+
+       if (!(capabilities = device->plugin->get_capabilities())) {
+               sr_err("dev: %s: device has no capabilities", __func__);
+               return FALSE; /* TODO: SR_ERR*. */
+       }
+
+       for (i = 0; capabilities[i]; i++) {
+               if (capabilities[i] != hwcap)
+                       continue;
+               sr_spew("dev: %s: found hwcap %d", __func__, hwcap);
+               return TRUE;
+       }
 
-       if ((capabilities = device->plugin->get_capabilities()))
-               for (i = 0; capabilities[i]; i++)
-                       if (capabilities[i] == hwcap)
-                               return TRUE;
+       sr_spew("dev: %s: hwcap %d not found", __func__, hwcap);
 
        return FALSE;
 }