From: Uwe Hermann Date: Wed, 28 Dec 2011 15:19:15 +0000 (+0100) Subject: device: Add Doxygen comments, improve error handling. X-Git-Tag: libsigrok-0.1.0~214 X-Git-Url: https://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=94799bc4dc6ab1ba64cdb7f0fb350d63d4fcee66;hp=cb1e389c72ead8cf6eb440f3194f5056d86069ec device: Add Doxygen comments, improve error handling. - Add Doxygen comments for all functions (some TODOs remain). - Check return code of more functions, handle invalid input. --- diff --git a/backend.c b/backend.c index c3c3087a..fc914e18 100644 --- a/backend.c +++ b/backend.c @@ -38,7 +38,6 @@ int sr_init(void) */ int sr_exit(void) { - sr_cleanup_hwplugins(); return SR_OK; diff --git a/device.c b/device.c index 6921ceb0..815e8066 100644 --- 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; }