Skip to content

generic_modbus, adelsystem_cbi: use typed buffers for register reads #3645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
user01010111 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

generic_modbus, adelsystem_cbi: use typed buffers for register reads #3645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81ebf1d
generic_modbus, adelsystem_cbi: use typed buffers for register reads
user01010111 Sep 14, 2026
c947ed5
Merge branch 'master' into codex/modbus-typed-register-storage
jimklimov Sep 23, 2026
3ea0563
adelsystem_cbi: use uint16_t for register read values
user01010111 Oct 13, 2026
9c03c8a
Merge branch 'master' into codex/modbus-typed-register-storage
jimklimov Oct 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ https://github.com/networkupstools/nut/milestone/13
as would be default for current platform (so one was treated as not-set
and failed a sanity check). [issue #3513]

- `generic_modbus` and `adelsystem_cbi` now read Modbus bit and register
values into correctly sized buffers before converting them for driver use.

- USB drivers now prefer the longer of conflicting HID report descriptor
lengths for EcoFlow devices using USB ID `3746:ffff`. This makes the
complete descriptor available on affected River 3 Plus firmware while
Expand Down
42 changes: 23 additions & 19 deletions drivers/adelsystem_cbi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#endif

#define DRIVER_NAME "NUT ADELSYSTEM DC-UPS CB/CBI driver (libmodbus link type: " NUT_MODBUS_LINKTYPE_STR ")"
#define DRIVER_VERSION "0.09"
#define DRIVER_VERSION "0.10"

/* variables */
static modbus_t *mbctx = NULL; /* modbus memory context */
Expand Down Expand Up @@ -75,7 +75,7 @@ modbus_t *modbus_new(const char *port);
void modbus_reconnect(void);

/* modbus register read function */
int register_read(modbus_t *mb, int addr, regtype_t type, void *data);
int register_read(modbus_t *mb, int addr, regtype_t type, uint16_t *data);

/* modbus register write function */
int register_write(modbus_t *mb, int addr, regtype_t type, void *data);
Expand Down Expand Up @@ -633,30 +633,34 @@ int read_all_regs(modbus_t *mb, uint16_t *data)
}

/* Read a modbus register */
int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
int register_read(modbus_t *mb, int addr, regtype_t type, uint16_t *data)
{
int rval = -1;
uint8_t bit_value = 0;
uint16_t register_value = 0;

/* register bit masks */
uint16_t mask8 = 0x00FF;
uint16_t mask16 = 0xFFFF;

*data = 0;

switch (type) {
case COIL:
rval = modbus_read_bits(mb, addr, 1, (uint8_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask8;
rval = modbus_read_bits(mb, addr, 1, &bit_value);
*data = (uint16_t)(bit_value & mask8);
break;
case INPUT_B:
rval = modbus_read_input_bits(mb, addr, 1, (uint8_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask8;
rval = modbus_read_input_bits(mb, addr, 1, &bit_value);
*data = (uint16_t)(bit_value & mask8);
break;
case INPUT_R:
rval = modbus_read_input_registers(mb, addr, 1, (uint16_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask16;
rval = modbus_read_input_registers(mb, addr, 1, &register_value);
*data = (uint16_t)(register_value & mask16);
break;
case HOLDING:
rval = modbus_read_registers(mb, addr, 1, (uint16_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask16;
rval = modbus_read_registers(mb, addr, 1, &register_value);
*data = (uint16_t)(register_value & mask16);
break;
#include "nut-pragmas-covered-switch-default.h"
/* All enum cases defined as of the time of coding
Expand Down Expand Up @@ -686,7 +690,7 @@ int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
}
}
upsdebugx(3, "register addr: 0x%x, register type: %u read: %u",
(unsigned int)addr, type, *(unsigned int *)data);
(unsigned int)addr, type, (unsigned int)*data);
return rval;
}

Expand Down Expand Up @@ -831,7 +835,7 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
int n;
int rval; /* return value */
static char *ptr = NULL; /* temporary pointer */
unsigned int reg_val; /* register value */
uint16_t reg_val; /* register value */
#if READALL_REGS == 0
unsigned int num; /* register number */
regtype_t rtype; /* register type */
Expand All @@ -852,11 +856,11 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
return rval;
}
upsdebugx(3,
"get_dev_state: num: %d, addr: 0x%x, regtype: %d, data: %d",
"get_dev_state: num: %d, addr: 0x%x, regtype: %d, data: %u",
num,
addr,
rtype,
reg_val
(unsigned int)reg_val
);
#endif
/* process register data */
Expand Down Expand Up @@ -914,14 +918,14 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
size_t reg_val_sz;

state->reg.val.ui16 = reg_val;
n = snprintf(NULL, 0, "%u", reg_val);
n = snprintf(NULL, 0, "%u", (unsigned int)reg_val);
if (ptr != NULL) {
free(ptr);
}
reg_val_sz = sizeof(char) * (n + 1);
reg_val_s = (char *)xmalloc(reg_val_sz);
ptr = reg_val_s;
snprintf(reg_val_s, reg_val_sz, "%u", reg_val);
snprintf(reg_val_s, reg_val_sz, "%u", (unsigned int)reg_val);
state->reg.strval = reg_val_s;
} else {
state->reg.val.ui16 = 0;
Expand Down Expand Up @@ -1151,14 +1155,14 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
size_t reg_val_sz;

state->reg.val.ui16 = reg_val;
n = snprintf(NULL, 0, "%u", reg_val);
n = snprintf(NULL, 0, "%u", (unsigned int)reg_val);
if (ptr != NULL) {
free(ptr);
}
reg_val_sz = sizeof(char) * (n + 1);
reg_val_s = (char *)xmalloc(reg_val_sz);
ptr = reg_val_s;
snprintf(reg_val_s, reg_val_sz, "%u", reg_val);
snprintf(reg_val_s, reg_val_sz, "%u", (unsigned int)reg_val);
state->reg.strval = reg_val_s;
}
break;
Expand Down
28 changes: 16 additions & 12 deletions drivers/generic_modbus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
#endif

#define DRIVER_NAME "NUT Generic Modbus driver (libmodbus link type: " NUT_MODBUS_LINKTYPE_STR ")"
#define DRIVER_VERSION "0.11"
#define DRIVER_VERSION "0.12"

/* variables */
static modbus_t *mbctx = NULL; /* modbus memory context */
Expand Down Expand Up @@ -61,7 +61,7 @@ modbus_t *modbus_new(const char *port);
void modbus_reconnect(void);

/* modbus register read function */
int register_read(modbus_t *mb, int addr, regtype_t type, void *data);
int register_read(modbus_t *mb, int addr, regtype_t type, int *data);

/* instant command triggered by upsd */
int upscmd(const char *cmd, const char *arg);
Expand Down Expand Up @@ -407,30 +407,34 @@ void upsdrv_cleanup(void)
*/

/* Read a modbus register */
int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
int register_read(modbus_t *mb, int addr, regtype_t type, int *data)
{
int rval = -1;
uint8_t bit_value = 0;
uint16_t register_value = 0;

/* register bit masks */
uint16_t mask8 = 0x000F;
uint16_t mask16 = 0x00FF;

*data = 0;

switch (type) {
case COIL:
rval = modbus_read_bits(mb, addr, 1, (uint8_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask8;
rval = modbus_read_bits(mb, addr, 1, &bit_value);
*data = (int)(bit_value & mask8);
break;
case INPUT_B:
rval = modbus_read_input_bits(mb, addr, 1, (uint8_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask8;
rval = modbus_read_input_bits(mb, addr, 1, &bit_value);
*data = (int)(bit_value & mask8);
break;
case INPUT_R:
rval = modbus_read_input_registers(mb, addr, 1, (uint16_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask16;
rval = modbus_read_input_registers(mb, addr, 1, &register_value);
*data = (int)(register_value & mask16);
break;
case HOLDING:
rval = modbus_read_registers(mb, addr, 1, (uint16_t *)data);
*(uint16_t *)data = *(uint16_t *)data & mask16;
rval = modbus_read_registers(mb, addr, 1, &register_value);
*data = (int)(register_value & mask16);
break;

#include "nut-pragmas-covered-switch-default.h"
Expand Down Expand Up @@ -460,7 +464,7 @@ int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
}
}
upsdebugx(3, "register addr: 0x%x, register type: %u read: %u",
(unsigned int)addr, type, *(unsigned int *)data);
(unsigned int)addr, type, (unsigned int)*data);
return rval;
}

Expand Down
Loading