Hi,
On Thu, Feb 05, 2015 at 05:30:57PM +0100, Bartosz Golaszewski wrote:
> + /**
> + * The device is a power-monitor supporting setting
> + * shunt resistance values.
> + */
> + SR_CONF_SHUNT_RESISTANCE,
Please split off the generic libsigrok additions into an extra patch.
The ACME implementation should be separate from that.
> diff --git a/src/hardware/acme/acme.c b/src/hardware/acme/acme.c
> index 3af77a4..6799428 100644
> --- a/src/hardware/acme/acme.c
> +++ b/src/hardware/acme/acme.c
> @@ -63,6 +63,7 @@ static const uint32_t devopts[] = {
> SR_CONF_LIMIT_SAMPLES | SR_CONF_GET | SR_CONF_SET,
> SR_CONF_LIMIT_MSEC | SR_CONF_GET | SR_CONF_SET,
> SR_CONF_SAMPLERATE | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
> + SR_CONF_SHUNT_RESISTANCE | SR_CONF_GET | SR_CONF_SET,
> };
>
> #define MAX_SAMPLE_RATE 500 /* In HZ */
> @@ -102,6 +103,8 @@ struct dev_context {
> int64_t last_sample_fin;
> int pipe_fds[2];
> GIOChannel *channel;
> +
> + struct channel_group_priv *probes[MAX_PROBES];
> };
>
> SR_PRIV struct sr_dev_driver acme_driver_info;
> @@ -240,8 +243,11 @@ static gboolean register_probe(struct sr_dev_inst *sdi,
> {
> struct sr_channel_group *chg;
> struct channel_group_priv *cgp;
> + struct dev_context *devc;
> int hwmon;
>
> + devc = sdi->priv;
> +
> /* Obtain the hwmon index. */
> hwmon = get_hwmon_index(addr);
> if (hwmon < 0)
> @@ -266,6 +272,7 @@ static gboolean register_probe(struct sr_dev_inst *sdi,
> }
>
> sdi->channel_groups = g_slist_append(sdi->channel_groups, chg);
> + devc->probes[prb_num - 1] = cgp;
>
> return TRUE;
> }
> @@ -398,12 +405,99 @@ static int cleanup(void)
> return SR_OK;
> }
>
> +static int read_shunt_values(const struct sr_dev_inst *sdi, GVariant **data)
> +{
> + struct sr_channel_group *chg;
> + struct channel_group_priv *chgp;
> + char path[256];
> + uint32_t in = 0, shunt;
> + GSList *chgl;
> + gboolean status;
> + gchar *contents, buf[512];
> + gsize size;
> + GError *err;
> +
> + for (chgl = sdi->channel_groups; chgl; chgl = chgl->next) {
> + chg = chgl->data;
> + chgp = chg->priv;
> +
> + in += snprintf(buf + in, sizeof(buf) - in, "%s: ", chg->name);
> + if (chgp->prb_type == PROBE_TEMP) {
> + in += snprintf(buf + in, sizeof(buf) - in, "N/A, ");
> + } else {
> + snprintf(path, sizeof(path),
> + "/sys/class/hwmon/hwmon%d/shunt_resistor",
> + chgp->hwmon_num);
> + status = g_file_get_contents(path, &contents,
> + &size, &err);
> + if (!status) {
> + sr_err("Error reading shunt resistance: %s",
> + err->message);
> + return -1;
> + }
> +
> + shunt = strtol(contents, NULL, 10) / 1000;
> + in += snprintf(buf + in, sizeof(buf) - in,
> + "%u mOhms, ", shunt);
> + g_free(contents);
> + }
> + }
> +
> + buf[in - 2] = '\0';
> + *data = g_variant_new_string(buf);
> +
> + return 0;
> +}
> +
> +static int set_shunt(const struct dev_context *devc,
> + unsigned probe, unsigned shunt)
> +{
[...]
> + fd = open(path, O_WRONLY);
> + if (fd < 0) {
> + sr_err("Error opening: %s\n", strerror(errno));
> + ret = -1;
> + goto out;
> + }
> +
> + write(fd, out, strlen(out));
> + close(fd);
Why no glib file open/write/close function here as in the rest of the
driver?
> +out:
> + return ret;
Why goto here? Simply returning in the resp. places is probably nicer,
unless I'm missing something?
> + case SR_CONF_SHUNT_RESISTANCE:
> + status = read_shunt_values(sdi, data);
> + if (status < 0)
> + ret = SR_ERR_BUG;
SR_ERR_BUG may not be the best fit here, it's meant to show bugs of the
libsigrok backend/drivers, not so much the fact that some hardware is
not responding or accessible or such.
> + unsigned probe, shunt;
"unsigned int" instead of "unsigned" everywhere please, looks nicer IMHO.
Uwe.
--
http://hermann-uwe.de | http://randomprojects.org | http://sigrok.org
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
