[collectd] [PATCH 3/5] sigrok: Multiple device sections, and working acquisition
---
src/sigrok.c | 325 +--
1 file changed, 292 insertions(+), 33 deletions(-)
diff --git a/src/sigrok.c b/src/sigrok.c
index 7189366..ea1e95e 100644
--- a/src/sigrok.c
+++ b/src/sigrok.c
@@ -22,72 +22,336 @@
#include stdio.h
#include stdlib.h
#include string.h
+#include time.h
+#include glib.h
#include libsigrok/libsigrok.h
-static char *conn = NULL;
-static char *serialcomm = NULL;
+#define DEFAULT_MIN_WRITE_INTERVAL 1
+
+GSList *config_devices;
+static struct sr_session *session = NULL;
+static int num_devices;
static int loglevel = SR_LOG_WARN;
static struct sr_context *sr_ctx;
-static const char *config_keys[] = {
- loglevel,
- conn,
- serialcomm,
+struct config_device {
+ char *name;
+ char *driver;
+ char *conn;
+ char *serialcomm;
+ struct sr_dev_inst *sdi;
+ int min_write_interval;
+ struct timespec last_write;
};
-static int cd_logger(void *cb_data, int loglevel, const char *format, va_list
args)
+static int cd_logger(void *cb_data, int msg_loglevel, const char *format,
+ va_list args)
{
char s[512];
- vsnprintf(s, 512, format, args);
- plugin_log(LOG_INFO, sigrok: %s, s);
+ if (msg_loglevel = loglevel) {
+ vsnprintf(s, 512, format, args);
+ plugin_log(LOG_INFO, sigrok: %s, s);
+ }
return 0;
}
-static int plugin_config(const char *key, const char *value)
+static int plugin_config_device(oconfig_item_t *ci)
{
+ oconfig_item_t *item;
+ struct config_device *cfdev;
+ int i;
+
+ if (ci-values_num != 1 || ci-values[0].type != OCONFIG_TYPE_STRING) {
+ ERROR(Invalid device name.);
+ return 1;
+ }
- if (!strcmp(key, loglevel))
- loglevel = atoi(value);
- else if (!strcmp(key, conn))
- conn = strdup(value);
- else if (!strcmp(key, serialcomm))
- serialcomm = strdup(value);
- else
+ if (!(cfdev = malloc(sizeof(struct config_device {
+ ERROR(malloc() failed.);
return 1;
+ }
+ memset(cfdev, 0, sizeof(struct config_device));
+ cfdev-name = strdup(ci-values[0].value.string);
+ cfdev-min_write_interval = DEFAULT_MIN_WRITE_INTERVAL;
+
+ for (i = 0; i ci-children_num; i++) {
+ item = ci-children + i;
+ if (item-values_num != 1) {
+ ERROR(Missing value for '%s'., item-key);
+ return 1;
+ }
+ if (!strcasecmp(item-key, driver)) {
+ cfdev-driver = strdup(item-values[0].value.string);
+ } else if (!strcasecmp(item-key, conn)) {
+ cfdev-conn = strdup(item-values[0].value.string);
+ } else if (!strcasecmp(item-key, serialcomm)) {
+ cfdev-serialcomm =
strdup(item-values[0].value.string);
+ } else if (!strcasecmp(item-key, interval)) {
+ cfdev-min_write_interval =
item-values[0].value.number;
+ } else {
+ ERROR(Invalid keyword '%s'., item-key);
+ return 1;
+ }
+ }
+
+ config_devices = g_slist_append(config_devices, cfdev);
+
+ return 0;
+}
+
+static int plugin_config(oconfig_item_t *ci)
+{
+ oconfig_item_t *item;
+ int i;
+
+ for (i = 0; i ci-children_num; i++) {
+ item = ci-children + i;
+ if (!strcasecmp(item-key, loglevel)) {
+ if (item-values_num != 1
+ || item-values[0].type !=
OCONFIG_TYPE_NUMBER
+ || item-values[0].value.number 0
+ || item-values[0].value.number 5) {
+ ERROR(Invalid loglevel);
+ return 1;
+ }
+ loglevel = item-values[0].value.number;
+ } else if (!strcasecmp(item-key, Device)) {
+ if (plugin_config_device(item) != 0)
+ return 1;
+ } else {
+ ERROR(Invalid keyword '%s'., item-key);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static void free_drvopts(struct sr_config *src)
+{
+ g_variant_unref(src-data);
+ g_free(src);
+}
+
+static int elapsed_time(struct timespec *last_write, struct timespec *elapsed)
+{
+ struct timespec cur_time;
+
+ if (clock_gettime(CLOCK_REALTIME, cur_time) != 0) {
+ ERROR(clock_gettime() failed.);
+ return -1;
+ }
+
+ if (cur_time.tv_nsec = last_write-tv_nsec) {
+ elapsed-tv_nsec = cur_time.tv_nsec - last_write-tv_nsec;
+
[collectd] [PATCH 5/5] sigrok: Add documentation
--- src/collectd.conf.in | 14 +++ src/collectd.conf.pod | 66 +++ 2 files changed, 80 insertions(+) diff --git a/src/collectd.conf.in b/src/collectd.conf.in index 80aba6a..542be67 100644 --- a/src/collectd.conf.in +++ b/src/collectd.conf.in @@ -154,6 +154,7 @@ @LOAD_PLUGIN_RRDTOOL@LoadPlugin rrdtool #@BUILD_PLUGIN_SENSORS_TRUE@LoadPlugin sensors #@BUILD_PLUGIN_SERIAL_TRUE@LoadPlugin serial +#@BUILD_PLUGIN_SIGROK_TRUE@LoadPlugin sigrok #@BUILD_PLUGIN_SNMP_TRUE@LoadPlugin snmp #@BUILD_PLUGIN_SWAP_TRUE@LoadPlugin swap #@BUILD_PLUGIN_TABLE_TRUE@LoadPlugin table @@ -890,6 +891,19 @@ # IgnoreSelected false #/Plugin +#Plugin sigrok +# LogLevel 3 +# Device AC Voltage +# Driver fluke-dmm +# Interval 10 +# Conn /dev/ttyUSB2 +# /Device +# Device Sound Level +# Driver cem-dt-885x +# Conn /dev/ttyUSB1 +# /Device +#/Plugin + #Plugin snmp # Data powerplus_voltge_input # Type voltage diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod index 11db1cc..fe92aac 100644 --- a/src/collectd.conf.pod +++ b/src/collectd.conf.pod @@ -5148,6 +5148,72 @@ and all other sensors are collected. =back +=head2 Plugin sigrok + +The Isigrok plugin uses libsigrok to retrieve measurements from any device +supported by the Lsigrok|http://sigrok.org/ project. + +BSynopsis + + Plugin sigrok + LogLevel 3 + Device AC Voltage + Driver fluke-dmm + Interval 10 + Conn /dev/ttyUSB2 + /Device + Device Sound Level + Driver cem-dt-885x + Conn /dev/ttyUSB1 + /Device + /Plugin + +=over 4 + +=item BLogLevel B0-5 + +The sigrok logging level to pass on to the collectd log, as a number 0-5. +These levels correspond to None, Errors, Warnings, Informational, Debug +and Spew, respectively. The default is 2 (Warnings). The sigrok log messages, +regardless of their level, are always submitted to collectd at its INFO +log level. + +=item EltBDevice InameEgt + +A sigrok-supported device, uniquely identified by this section's options. The +Iname is passed to collectd as the Iplugin instance. + +=item BDriver + +The sigrok driver to use for this device. + +=item BConn + +If the device cannot be auto-discovered, or more than one might be discovered +by the driver, IConn specifies the connection string to the device. It can +be of the form of a serial port (I/dev/ttyUSB2), or, in case of a non-serial +USB-connected device, the USB VendorID/ProductID separated by a period +(I0403.6001). A USB device can also be specified as bus.address +(I1.41). + +=item BSerialComm + +For serial devices with non-standard port settings, this option can be used +to specify them in the form I9600/8n1. This should not be necessary; drivers +know how to communicate with devices they support. + +=item BInterval + +Specifies the minimum time between measurement dispatches to collectd, in +seconds. Since some sigrok-supported devices can acquire measurements many +times per second, it may be necessary to throttle these. For example, the +RRD plugin cannot process writes more than once per second. + +The default (and minimum) interval is 1 second. Unused measurements are +discarded. + +=back + =head2 Plugin Csnmp Since the configuration of the Csnmp plugin is a little more complicated than -- 1.8.1.2 ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
[collectd] [PATCH 1/5] sigrok: build system setup
---
configure.in| 71 +
src/Makefile.am | 10
2 files changed, 81 insertions(+)
diff --git a/configure.in b/configure.in
index 27c6cd3..0838de8 100644
--- a/configure.in
+++ b/configure.in
@@ -3767,6 +3767,74 @@ fi
AM_CONDITIONAL(BUILD_WITH_LM_SENSORS, test x$with_libsensors = xyes)
# }}}
+# --with-libsigrok {{{
+with_libsigrok_cflags=
+with_libsigrok_ldflags=
+AC_ARG_WITH(libsigrok, [AS_HELP_STRING([--with-libsigrok@:@=PREFIX@:@],
[Path to libsigrok.])],
+[
+ if test x$withval = xno
+ then
+ with_libsigrok=no
+ else
+ with_libsigrok=yes
+ if test x$withval != xyes
+ then
+ with_libsigrok_cflags=-I$withval/include
+ with_libsigrok_ldflags=-L$withval/lib
+ fi
+ fi
+],[])
+
+# libsigrok has a glib dependency
+if test x$with_libsigrok = xyes
+then
+ if test -z m4_ifdef([AM_PATH_GLIB_2_0], [yes], [])
+ then
+ with_libsigrok=no (glib not available)
+ else
+ AM_PATH_GLIB_2_0([2.28.0],
+ [with_libsigrok_cflags=$with_libsigrok_cflags
$GLIB_CFLAGS; with_libsigrok_ldflags=$with_libsigrok_ldflags $GLIB_LIBS])
+ fi
+fi
+
+# libsigrok headers
+if test x$with_libsigrok = xyes
+then
+ SAVE_CPPFLAGS=$CPPFLAGS
+ CPPFLAGS=$CPPFLAGS $with_libsigrok_cflags
+
+ AC_CHECK_HEADERS(libsigrok/libsigrok.h, [], [with_libsigrok=no
(libsigrok/libsigrok.h not found)])
+
+ CPPFLAGS=$SAVE_CPPFLAGS
+fi
+
+# libsigrok library
+if test x$with_libsigrok = xyes
+then
+ SAVE_CPPFLAGS=$CPPFLAGS
+ SAVE_LDFLAGS=$LDFLAGS
+ CPPFLAGS=$CPPFLAGS $with_libsigrok_cflags
+ LDFLAGS=$LDFLAGS $with_libsigrok_ldflags
+
+ AC_CHECK_LIB(sigrok, sr_init,
+ [
+ AC_DEFINE(HAVE_LIBSIGROK, 1, [Define to 1 if you have the
sigrok library (-lsigrok).])
+ ],
+ [with_libsigrok=no (libsigrok not found)])
+
+ CPPFLAGS=$SAVE_CPPFLAGS
+ LDFLAGS=$SAVE_LDFLAGS
+fi
+if test x$with_libsigrok = xyes
+then
+ BUILD_WITH_LIBSIGROK_CFLAGS=$with_libsigrok_cflags
+ BUILD_WITH_LIBSIGROK_LDFLAGS=$with_libsigrok_ldflags
+ AC_SUBST(BUILD_WITH_LIBSIGROK_CFLAGS)
+ AC_SUBST(BUILD_WITH_LIBSIGROK_LDFLAGS)
+fi
+AM_CONDITIONAL(BUILD_WITH_LIBSIGROK, test x$with_libsigrok = xyes)
+# }}}
+
# --with-libstatgrab {{{
with_libstatgrab_cflags=
with_libstatgrab_ldflags=
@@ -5098,6 +5166,7 @@ AC_PLUGIN([rrdcached], [$librrd_rrdc_update], [RRDTool
output plugin])
AC_PLUGIN([rrdtool], [$with_librrd], [RRDTool output plugin])
AC_PLUGIN([sensors], [$with_libsensors], [lm_sensors statistics])
AC_PLUGIN([serial], [$plugin_serial], [serial port traffic])
+AC_PLUGIN([sigrok], [$with_libsigrok],[sigrok acquisition sources])
AC_PLUGIN([snmp],[$with_libnetsnmp], [SNMP querying plugin])
AC_PLUGIN([swap],[$plugin_swap], [Swap usage statistics])
AC_PLUGIN([syslog], [$have_syslog], [Syslog logging plugin])
@@ -5331,6 +5400,7 @@ Configuration:
librouteros . . . . . $with_librouteros
librrd . . . . . . . $with_librrd
libsensors . . . . . $with_libsensors
+libsigrok . . . . . $with_libsigrok
libstatgrab . . . . . $with_libstatgrab
libtokyotyrant . . . $with_libtokyotyrant
libupsclient . . . . $with_libupsclient
@@ -5435,6 +5505,7 @@ Configuration:
rrdtool . . . . . . . $enable_rrdtool
sensors . . . . . . . $enable_sensors
serial . . . . . . . $enable_serial
+sigrok . . . . . . . $enable_sigrok
snmp . . . . . . . . $enable_snmp
swap . . . . . . . . $enable_swap
syslog . . . . . . . $enable_syslog
diff --git a/src/Makefile.am b/src/Makefile.am
index c3e596d..bdde5a3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1052,6 +1052,16 @@ collectd_LDADD += -dlopen serial.la
collectd_DEPENDENCIES += serial.la
endif
+if BUILD_PLUGIN_SIGROK
+pkglib_LTLIBRARIES += sigrok.la
+sigrok_la_SOURCES = sigrok.c
+sigrok_la_CFLAGS = $(AM_CFLAGS) $(BUILD_WITH_LIBSIGROK_CFLAGS)
+sigrok_la_LDFLAGS = -module -avoid-version $(BUILD_WITH_LIBSIGROK_LDFLAGS)
+sigrok_la_LIBADD = -lsigrok
+collectd_LDADD += -dlopen sigrok.la
+collectd_DEPENDENCIES += sigrok.la
+endif
+
if BUILD_PLUGIN_SNMP
pkglib_LTLIBRARIES += snmp.la
snmp_la_SOURCES = snmp.c
--
1.8.1.2
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd
[collectd] [PATCH 4/5] sigrok: Use separate thread for all I/O
---
src/sigrok.c | 110 +++
1 file changed, 66 insertions(+), 44 deletions(-)
diff --git a/src/sigrok.c b/src/sigrok.c
index ea1e95e..89ef61c 100644
--- a/src/sigrok.c
+++ b/src/sigrok.c
@@ -19,15 +19,22 @@
#include collectd.h
#include common.h
#include plugin.h
+
#include stdio.h
#include stdlib.h
#include string.h
#include time.h
+#include pthread.h
+
#include glib.h
#include libsigrok/libsigrok.h
+/* Minimum interval between dispatches coming from this plugin. The RRD
+ * plugin, at least, complains when written to with sub-second intervals.*/
#define DEFAULT_MIN_WRITE_INTERVAL 1
+static pthread_t sr_thread;
+static int sr_thread_running = FALSE;
GSList *config_devices;
static struct sr_session *session = NULL;
static int num_devices;
@@ -222,34 +229,27 @@ static void plugin_feed_callback(const struct sr_dev_inst
*sdi,
}
-static int plugin_read(user_data_t *ud)
-{
-
- sr_session_iteration(FALSE);
-
- return 0;
-}
-
-static int plugin_init(void)
+static void *thread_init(void *arg)
{
struct sr_dev_driver *drv, **drvlist;
struct sr_config *src;
GSList *devlist, *drvopts, *l;
struct config_device *cfdev;
- struct timespec ts;
int ret, i;
char hwident[512];
+ (void)arg;
+
sr_log_callback_set(cd_logger, NULL);
sr_log_loglevel_set(loglevel);
if ((ret = sr_init(sr_ctx)) != SR_OK) {
ERROR(Failed to initialize libsigrok: %s., sr_strerror(ret));
- return 1;
+ return NULL;
}
if (!(session = sr_session_new()))
- return 1;
+ return NULL;
num_devices = 0;
drvlist = sr_driver_list();
@@ -264,37 +264,37 @@ static int plugin_init(void)
}
if (!drv) {
ERROR(sigrok: Unknown driver '%s'., cfdev-driver);
- return 1;
+ return NULL;
}
if (sr_driver_init(sr_ctx, drv) != SR_OK)
- return 1;
+ return NULL;
drvopts = NULL;
if (cfdev-conn) {
if (!(src = malloc(sizeof(struct sr_config
- return 1;
+ return NULL;
src-key = SR_CONF_CONN;
src-data = g_variant_new_string(cfdev-conn);
drvopts = g_slist_append(drvopts, src);
}
if (cfdev-serialcomm) {
if (!(src = malloc(sizeof(struct sr_config
- return 1;
+ return NULL;
src-key = SR_CONF_SERIALCOMM;
src-data = g_variant_new_string(cfdev-serialcomm);
drvopts = g_slist_append(drvopts, src);
}
devlist = sr_driver_scan(drv, drvopts);
g_slist_free_full(drvopts, (GDestroyNotify)free_drvopts);
- if (!devlist) {
- INFO(sigrok: No devices found.);
- return 0;
- }
+ if (!devlist)
+ /* No devices found for this driver. */
+ continue;
+
if (g_slist_length(devlist) 1) {
INFO(sigrok: %d sigrok devices for device entry '%s':
must be 1.,
g_slist_length(devlist), cfdev-name);
- return 1;
+ return NULL;
}
cfdev-sdi = devlist-data;
g_slist_free(devlist);
@@ -305,28 +305,54 @@ static int plugin_init(void)
INFO(sigrok: Device '%s' is a %s., cfdev-name, hwident);
if (sr_dev_open(cfdev-sdi) != SR_OK)
- return 1;
+ return NULL;
if (sr_session_dev_add(cfdev-sdi) != SR_OK)
- return 1;
+ return NULL;
num_devices++;
}
if (num_devices 0) {
/* Do this only when we're sure there's hardware to talk to. */
- ts.tv_sec = 0;
- ts.tv_nsec = 100L;
- plugin_register_complex_read(sigrok, sigrok, plugin_read,
- ts, NULL);
-
if (sr_session_datafeed_callback_add(plugin_feed_callback,
NULL) != SR_OK)
- return 1;
+ return NULL;
+ /* Start acquisition on all devices. */
if (sr_session_start() != SR_OK)
- return 1;
- } else
- sr_session_destroy();
+ return NULL;
+
+ /* Main loop, runs forever. */
+
Re: [collectd] [PATCH 5/5] sigrok: Add documentation
wtf? why are commits coming to the list? On Mon, Jul 22, 2013 at 9:21 AM, Bert Vermeulen b...@biot.com wrote: --- src/collectd.conf.in | 14 +++ src/collectd.conf.pod | 66 +++ 2 files changed, 80 insertions(+) diff --git a/src/collectd.conf.in b/src/collectd.conf.in index 80aba6a..542be67 100644 --- a/src/collectd.conf.in +++ b/src/collectd.conf.in @@ -154,6 +154,7 @@ @LOAD_PLUGIN_RRDTOOL@LoadPlugin rrdtool #@BUILD_PLUGIN_SENSORS_TRUE@LoadPlugin sensors #@BUILD_PLUGIN_SERIAL_TRUE@LoadPlugin serial +#@BUILD_PLUGIN_SIGROK_TRUE@LoadPlugin sigrok #@BUILD_PLUGIN_SNMP_TRUE@LoadPlugin snmp #@BUILD_PLUGIN_SWAP_TRUE@LoadPlugin swap #@BUILD_PLUGIN_TABLE_TRUE@LoadPlugin table @@ -890,6 +891,19 @@ # IgnoreSelected false #/Plugin +#Plugin sigrok +# LogLevel 3 +# Device AC Voltage +# Driver fluke-dmm +# Interval 10 +# Conn /dev/ttyUSB2 +# /Device +# Device Sound Level +# Driver cem-dt-885x +# Conn /dev/ttyUSB1 +# /Device +#/Plugin + #Plugin snmp # Data powerplus_voltge_input # Type voltage diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod index 11db1cc..fe92aac 100644 --- a/src/collectd.conf.pod +++ b/src/collectd.conf.pod @@ -5148,6 +5148,72 @@ and all other sensors are collected. =back +=head2 Plugin sigrok + +The Isigrok plugin uses libsigrok to retrieve measurements from any device +supported by the Lsigrok|http://sigrok.org/ project. + +BSynopsis + + Plugin sigrok + LogLevel 3 + Device AC Voltage + Driver fluke-dmm + Interval 10 + Conn /dev/ttyUSB2 + /Device + Device Sound Level + Driver cem-dt-885x + Conn /dev/ttyUSB1 + /Device + /Plugin + +=over 4 + +=item BLogLevel B0-5 + +The sigrok logging level to pass on to the collectd log, as a number 0-5. +These levels correspond to None, Errors, Warnings, Informational, Debug +and Spew, respectively. The default is 2 (Warnings). The sigrok log messages, +regardless of their level, are always submitted to collectd at its INFO +log level. + +=item EltBDevice InameEgt + +A sigrok-supported device, uniquely identified by this section's options. The +Iname is passed to collectd as the Iplugin instance. + +=item BDriver + +The sigrok driver to use for this device. + +=item BConn + +If the device cannot be auto-discovered, or more than one might be discovered +by the driver, IConn specifies the connection string to the device. It can +be of the form of a serial port (I/dev/ttyUSB2), or, in case of a non-serial +USB-connected device, the USB VendorID/ProductID separated by a period +(I0403.6001). A USB device can also be specified as bus.address +(I1.41). + +=item BSerialComm + +For serial devices with non-standard port settings, this option can be used +to specify them in the form I9600/8n1. This should not be necessary; drivers +know how to communicate with devices they support. + +=item BInterval + +Specifies the minimum time between measurement dispatches to collectd, in +seconds. Since some sigrok-supported devices can acquire measurements many +times per second, it may be necessary to throttle these. For example, the +RRD plugin cannot process writes more than once per second. + +The default (and minimum) interval is 1 second. Unused measurements are +discarded. + +=back + =head2 Plugin Csnmp Since the configuration of the Csnmp plugin is a little more complicated than -- 1.8.1.2 ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd ___ collectd mailing list collectd@verplant.org http://mailman.verplant.org/listinfo/collectd
Re: [collectd] [PATCH 2/5] sigrok: bare bones plugin
On Mon, Jul 22, 2013 at 06:21:18PM +0200, Bert Vermeulen wrote:
+static int plugin_config(const char *key, const char *value)
+{
+
+ if (!strcmp(key, loglevel))
+ loglevel = atoi(value);
+ else if (!strcmp(key, conn))
+ conn = strdup(value);
This will leak memory if the config file defines these elements more than once.
+ else if (!strcmp(key, serialcomm))
+ serialcomm = strdup(value);
Here too.
+ else
+ return 1;
+
+ return 0;
+}
+
+static int plugin_init(void)
+{
+ int ret;
+
+ sr_log_callback_set(cd_logger, NULL);
+ sr_log_loglevel_set(loglevel);
+
+ if ((ret = sr_init(sr_ctx)) != SR_OK) {
+ INFO(Failed to initialize libsigrok: %s., sr_strerror(ret));
+ return 1;
+ }
+
+ return 0;
+}
+
+static int plugin_read(user_data_t *ud)
+{
+
+ return 0;
+}
Personally, I don't see the need to split this patch up into several pieces. A
skeleton by itself isn't very useful to review.
+
+static int plugin_shutdown(void)
+{
+ if (conn)
+ free(conn);
+ if (serialcomm)
+ free(serialcomm);
The if check for NULL is not needed, as free is guaranteed to be able to handle
it.
+
+ sr_exit(sr_ctx);
+
+ return 0;
+}
Dan
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd
Re: [collectd] [PATCH 2/5] sigrok: bare bones plugin
Hi Bert,
thank you very much for your patches!
On Mon, Jul 22, 2013 at 06:21:18PM +0200, Bert Vermeulen wrote:
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
Unfortunately, this is going to cause licensing issues: Many parts of
collectd are licenses under the GPL version 2, which is not compatible
to GPL version 3.
+static int cd_logger(void *cb_data, int loglevel, const char *format,
va_list args)
Macros are better suited for this kind of prefix implementation, because
you don't need a separate buffer / call to vsnprintf(). E.g.:
#define SR_INFO(...) INFO (sigrok plugin: __VA_ARGS__)
+ if (!strcmp(key, loglevel))
+ loglevel = atoi(value);
Please use parse_log_severity() from src/plugin.h to parse a string into
a log severity.
+void module_register(void)
+{
…
+ ts.tv_sec = 0;
+ ts.tv_nsec = 10L;
+ plugin_register_complex_read(sigrok, sigrok, plugin_read, ts,
NULL);
Please pass in NULL for the interval. The user can then configure the
interval used, e.g.:
LoadPlugin sigrok
Interval 42.0
/LoadPlugin
Best regards,
—octo
--
collectd – The system statistics collection daemon
Website: http://collectd.org
Google+: http://collectd.org/+
GitHub: https://github.com/collectd
Twitter: http://twitter.com/collectd
signature.asc
Description: Digital signature
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd
Re: [collectd] [PATCH 3/5] sigrok: Multiple device sections, and working acquisition
Hi again,
well, seing how the config function was rewritten, I think a squished
together patch might actually make more sense. If you have a Github
account, opening a Pull Request would also work, since I can comment on
the overall changes, not just indivitual patches.
On Mon, Jul 22, 2013 at 06:21:19PM +0200, Bert Vermeulen wrote:
+static int plugin_config_device(oconfig_item_t *ci)
{
…
+ if (!strcasecmp(item-key, driver)) {
+ cfdev-driver = strdup(item-values[0].value.string);
Please use cf_util_get_string() from src/configfile.h for string
arguments.
+ } else if (!strcasecmp(item-key, interval)) {
+ cfdev-min_write_interval =
item-values[0].value.number;
Please use cf_util_get_cdtime() from src/configfile.h for this.
+static int elapsed_time(struct timespec *last_write, struct timespec
*elapsed)
Please use cdtime_t from src/utils_time.h; you can simply do a
cdtime_t elapsed = now - previous;
with that time representation.
+ if (packet-type == SR_DF_END) {
+ /* TODO: try to restart acquisition after a delay? */
+ INFO(oops! ended);
There's a specialized logginf function in the code. Why not use it here?
+ if (!cfdev) {
+ ERROR(Unknown device instance in sigrok driver %s.,
sdi-driver-name);
Dito.
+ if (elapsed_time(cfdev-last_write, elapsed) != 0)
+ return;
+ if (elapsed.tv_sec cfdev-min_write_interval)
+ return;
Can't you use the timing code from plugin.c? For example, can't you
register one read callback for each device? Then you don't have to care
about the interval at all.
+ ERROR(malloc() failed.);
See above.
+ if (clock_gettime(CLOCK_REALTIME, cfdev-last_write) != 0) {
Please use cdtime() or, preferably, let the plugin infrastructure handle
all the timing.
static int plugin_init(void)
The plugin_ prefix is reserved for functions defined in src/plugin.h.
Maybe you can use csr_ or something like that instead?
+ /* Do this only when we're sure there's hardware to talk to. */
+ ts.tv_sec = 0;
+ ts.tv_nsec = 100L;
+ plugin_register_complex_read(sigrok, sigrok, plugin_read,
+ ts, NULL);
This is fine, if you register one read callback for each device you
find. As it is, I would simply register this in module_register() below.
If the user loaded this plugin and no devices are found, something is
usually wrong.
Best regards,
—octo
--
collectd – The system statistics collection daemon
Website: http://collectd.org
Google+: http://collectd.org/+
GitHub: https://github.com/collectd
Twitter: http://twitter.com/collectd
signature.asc
Description: Digital signature
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd
