[collectd] [PATCH] Add modbus_read_input_registers() command to modbus plugin

[Eric Sandeen]

On 11/6/14 5:09 PM, Eric Sandeen wrote:
> The modbus plugin currently only does a modbus_read_registers()
> call (modbus command 0x03).  Some devices have registers which
> must be read with modbus_read_input_registers() (command 0x04).
> 
> This patch allows the config file to specify which command to
> use; the default remains the same (modbus_read_registers) but
> can be specified with:
> 
>     RegisterCmd ReadHolding
> or
>     RegisterCmd ReadInput
> 
> in each <Data> definition block.
> 
> I've tested this with newer libmodbus (v3.0+) but not the
> legacy code.  Works great with my Triangle Tube boiler.
> 
> Signed-off-by: Eric Sandeen <sandeen at sandeen.net>

I can't tell, is there any interest in this?  No comment & not
merged, should I keep trying?

Thanks,
-Eric

> ---
> 
> diff --git a/src/modbus.c b/src/modbus.c
> index 887c63c..b90266e 100644
> --- a/src/modbus.c
> +++ b/src/modbus.c
> @@ -48,6 +48,7 @@
>   * <Data "data_name">
>   *   RegisterBase 1234
>   *   RegisterType float
> + *   RegisterCmd ReadHolding # default, or: ReadInput
>   *   Type gauge
>   *   Instance "..."
>   * </Data>
> @@ -77,6 +78,12 @@ enum mb_register_type_e /* {{{ */
>  }; /* }}} */
>  typedef enum mb_register_type_e mb_register_type_t;
>  
> +enum mb_register_cmd_e /* {{{ */
> +{
> +  REG_CMD_READ_HOLDING,
> +  REG_CMD_READ_INPUT
> +}; /* }}} */
> +typedef enum mb_register_cmd_e mb_register_cmd_t;
>  struct mb_data_s;
>  typedef struct mb_data_s mb_data_t;
>  struct mb_data_s /* {{{ */
> @@ -84,6 +91,7 @@ struct mb_data_s /* {{{ */
>    char *name;
>    int register_base;
>    mb_register_type_t register_type;
> +  mb_register_cmd_t register_cmd;
>    char type[DATA_MAX_NAME_LEN];
>    char instance[DATA_MAX_NAME_LEN];
>  
> @@ -457,6 +465,8 @@ static int mb_read_data (mb_host_t *host, mb_slave_t *slave, /* {{{ */
>     * id to each call of "read_holding_registers". */
>  # define modbus_read_registers(ctx, addr, nb, dest) \
>    read_holding_registers (&(ctx), slave->id, (addr), (nb), (dest))
> +# define modbus_read_input_registers(ctx, addr, nb, dest) \
> +  read_input_registers (&(ctx), slave->id, (addr), (nb), (dest))
>  #else /* if !LEGACY_LIBMODBUS */
>    /* Version 2.9.2: Set the slave id once before querying the registers. */
>    status = modbus_set_slave (host->connection, slave->id);
> @@ -468,13 +478,20 @@ static int mb_read_data (mb_host_t *host, mb_slave_t *slave, /* {{{ */
>    }
>  #endif
>  
> -  status = modbus_read_registers (host->connection,
> -        /* start_addr = */ data->register_base,
> -        /* num_registers = */ values_num, /* buffer = */ values);
> +  if (data->register_cmd == REG_CMD_READ_INPUT)
> +    status = modbus_read_input_registers (host->connection,
> +          /* start_addr = */ data->register_base,
> +          /* num_registers = */ values_num, /* buffer = */ values);
> +  else
> +    status = modbus_read_registers (host->connection,
> +          /* start_addr = */ data->register_base,
> +          /* num_registers = */ values_num, /* buffer = */ values);
> +
>    if (status != values_num)
>    {
> -    ERROR ("Modbus plugin: modbus_read_registers (%s/%s) failed. status = %i, values_num = %i "
> -        "Giving up.", host->host, host->node, status, values_num);
> +    ERROR ("Modbus plugin: modbus_read_%sregisters (%s/%s) failed. status = %i, values_num = %i "
> +        "Giving up.", data->register_cmd == REG_CMD_READ_INPUT ? "input_" : "",
> +        host->host, host->node, status, values_num);
>  #if LEGACY_LIBMODBUS
>      modbus_close (&host->connection);
>  #else
> @@ -486,7 +503,8 @@ static int mb_read_data (mb_host_t *host, mb_slave_t *slave, /* {{{ */
>    }
>  
>    DEBUG ("Modbus plugin: mb_read_data: Success! "
> -      "modbus_read_registers returned with status %i.", status);
> +      "modbus_read_%sregisters returned with status %i.",
> +      data->register_cmd == REG_CMD_READ_INPUT ? "input_" : "", status);
>  
>    if (data->register_type == REG_TYPE_FLOAT)
>    {
> @@ -668,6 +686,7 @@ static int mb_config_add_data (oconfig_item_t *ci) /* {{{ */
>    memset (&data, 0, sizeof (data));
>    data.name = NULL;
>    data.register_type = REG_TYPE_UINT16;
> +  data.register_cmd = REG_CMD_READ_HOLDING;
>    data.next = NULL;
>  
>    status = cf_util_get_string (ci, &data.name);
> @@ -709,6 +728,22 @@ static int mb_config_add_data (oconfig_item_t *ci) /* {{{ */
>          status = -1;
>        }
>      }
> +    else if (strcasecmp ("RegisterCmd", child->key) == 0)
> +    {
> +      char tmp[16];
> +      status = cf_util_get_string_buffer (child, tmp, sizeof (tmp));
> +      if (status != 0)
> +        /* do nothing */;
> +      else if (strcasecmp ("ReadHolding", tmp) == 0)
> +        data.register_cmd= REG_CMD_READ_HOLDING;
> +      else if (strcasecmp ("ReadInput", tmp) == 0)
> +        data.register_cmd = REG_CMD_READ_INPUT; 
> +      else
> +      {
> +        ERROR ("Modbus plugin: The register command \"%s\" is unknown.", tmp);
> +        status = -1;
> +      }
> +    }
>      else
>      {
>        ERROR ("Modbus plugin: Unknown configuration option: %s", child->key);
> 
> 
> _______________________________________________
> collectd mailing list
> collectd at verplant.org
> http://mailman.verplant.org/listinfo/collectd
>