modbus plugin: Implement the "Dataset" config option.

Hello Florian,

First of all, I would like to thank you for writing collectd. It is a
very nice and useful application!

Secondly, thanks for changing the license of the collectd modbus plugin.
I would like to eventually get it included in the package of collectd
that is shipped by debian and I hope the new license is acceptable to
them.

In addition, I think it would be nice to group a number of registers, to
make it easier to configure a number of identical devices. I wrote a
small patch (attached) that adds a config option 'Dataset', that works
like in the example below. The patch is only tested lightly. Could you
comment on the (preliminary) patch?

Signed-off-by: Florian Forster <octo@huhu.verplant.org>

diff --git a/src/modbus.c b/src/modbus.c
index cad4b2c..7145c8a 100644 (file)
--- a/src/modbus.c
+++ b/src/modbus.c
@@ -88,6 +88,16 @@ struct mb_data_s /* {{{ */
   mb_data_t *next;
 }; /* }}} */
 
+struct mb_dataset_s;
+typedef struct mb_dataset_s mb_dataset_t;
+struct mb_dataset_s /* {{{ */
+{
+  char *name;
+  mb_data_t *collect;
+
+  mb_dataset_t *next;
+}; /* }}} */
+
 struct mb_slave_s /* {{{ */
 {
   int id;
@@ -131,6 +141,7 @@ struct mb_data_group_s /* {{{ */
  * Global variables
  */
 static mb_data_t *data_definitions = NULL;
+static mb_dataset_t *data_sets = NULL;
 
 /*
  * Functions
@@ -224,6 +235,78 @@ static int data_copy_by_name (mb_data_t **dst, mb_data_t *src, /* {{{ */
   return (data_copy (dst, ptr));
 } /* }}} int data_copy_by_name */
 
+static mb_dataset_t *dataset_get_by_name (mb_dataset_t *src, /* {{{ */
+    const char *name)
+{
+  mb_dataset_t *ptr;
+
+  if (name == NULL)
+    return (NULL);
+
+  for (ptr = src; ptr != NULL; ptr = ptr->next)
+    if (strcasecmp (ptr->name, name) == 0)
+      return (ptr);
+
+  return (NULL);
+} /* }}} mb_dataset_t *dataset_get_by_name */
+
+static int dataset_append (mb_dataset_t **dst, mb_dataset_t *src) /* {{{ */
+{
+  mb_dataset_t *ptr;
+
+  if ((dst == NULL) || (src == NULL))
+    return (EINVAL);
+
+  ptr = *dst;
+
+  if (ptr == NULL)
+  {
+    *dst = src;
+    return (0);
+  }
+
+  while (ptr->next != NULL)
+    ptr = ptr->next;
+
+  ptr->next = src;
+
+  return (0);
+} /* }}} int dataset_append */
+
+/* Copy a single mb_dataset_t and append it to another list. */
+static int dataset_copy (mb_dataset_t **dst, const mb_dataset_t *src) /* {{{ */
+{
+  mb_dataset_t *tmp;
+  int status;
+
+  if ((dst == NULL) || (src == NULL))
+    return (EINVAL);
+
+  tmp = malloc (sizeof (*tmp));
+  if (tmp == NULL)
+    return (ENOMEM);
+  memcpy (tmp, src, sizeof (*tmp));
+  tmp->name = NULL;
+  tmp->next = NULL;
+
+  tmp->name = strdup (src->name);
+  if (tmp->name == NULL)
+  {
+    sfree (tmp);
+    return (ENOMEM);
+  }
+
+  status = dataset_append (dst, tmp);
+  if (status != 0)
+  {
+    sfree (tmp->name);
+    sfree (tmp);
+    return (status);
+  }
+
+  return (0);
+} /* }}} int dataset_copy */
+
 /* Read functions */
 
 static int mb_submit (mb_host_t *host, mb_slave_t *slave, /* {{{ */
@@ -795,6 +878,24 @@ static int mb_config_add_slave (mb_host_t *host, oconfig_item_t *ci) /* {{{ */
         data_copy_by_name (&slave->collect, data_definitions, buffer);
       status = 0; /* continue after failure. */
     }
+    else if (strcasecmp ("Dataset", child->key) == 0)
+    {
+      char buffer[1024];
+      mb_dataset_t *ds;
+
+      status = cf_util_get_string_buffer (child, buffer, sizeof (buffer));
+      if (status == 0) {
+        ds = dataset_get_by_name (data_sets, buffer);
+        if (ds) {
+          mb_data_t *data;
+          for (data = ds->collect; data != NULL; data = data->next)
+          {
+            data_copy (&slave->collect, data);
+          }
+        }
+      }
+      status = 0; /* continue after failure. */
+       }
     else
     {
       ERROR ("Modbus plugin: Unknown configuration option: %s", child->key);
@@ -904,6 +1005,53 @@ static int mb_config_add_host (oconfig_item_t *ci) /* {{{ */
   return (status);
 } /* }}} int mb_config_add_host */
 
+
+static int mb_config_add_dataset (oconfig_item_t *ci) /* {{{ */
+{
+  mb_dataset_t dataset;
+  int status;
+  int i;
+
+  memset (&dataset, 0, sizeof (dataset));
+  dataset.name = NULL;
+  dataset.collect = NULL;
+  dataset.next = NULL;
+
+  status = cf_util_get_string (ci, &dataset.name);
+  
+  for (i = 0; i < ci->children_num; i++)
+  {
+    oconfig_item_t *child = ci->children + i;
+    status = 0;
+
+    if (strcasecmp ("Collect", child->key) == 0)
+    {
+      char buffer[1024];
+      status = cf_util_get_string_buffer (child, buffer, sizeof (buffer));
+      if (status == 0) {
+        data_copy_by_name (&dataset.collect, data_definitions, buffer);
+         }
+      status = 0; /* continue after failure. */
+    }
+    else
+    {
+      ERROR ("Modbus plugin: Unknown configuration option: %s", child->key);
+      status = -1;
+    }
+
+    if (status != 0)
+      break;
+  } /* for (i = 0; i < ci->children_num; i++) */
+
+  if ((status == 0) && (dataset.collect == NULL))
+    status = EINVAL;
+
+  if (status == 0)
+    dataset_copy (&data_sets, &dataset);
+
+  return (status);
+} /* }}} int mb_config_add_dataset */
+
 static int mb_config (oconfig_item_t *ci) /* {{{ */
 {
   int i;
@@ -919,6 +1067,8 @@ static int mb_config (oconfig_item_t *ci) /* {{{ */
       mb_config_add_data (child);
     else if (strcasecmp ("Host", child->key) == 0)
       mb_config_add_host (child);
+    else if (strcasecmp ("Dataset", child->key) == 0)
+      mb_config_add_dataset (child);
     else
       ERROR ("Modbus plugin: Unknown configuration option: %s", child->key);
   }