Skip to content

apc_modbus: simplify error handling with read retries #3414

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
EchterAgo wants to merge 2 commits into
base: master
Choose a base branch
from
+42 −48
Open

apc_modbus: simplify error handling with read retries #3414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ef4491
apc_modbus: simplify error handling with read retries
EchterAgo Apr 13, 2026
464772a
drivers/apc_modbus.c: addvar() the modbus_retries option [#3414]
jimklimov Apr 18, 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
2 changes: 2 additions & 0 deletions NEWS.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ https://github.com/networkupstools/nut/milestone/13

- `apc_modbus` driver updates:
* Fixed string join not doing zero termination. [PR #3413]
* Added `modbus_retries` option to configure the number of read retry
attempts on timeout errors, simplifying error recovery. [PR #3414]

- NUT client libraries:
* Complete support for actions documented in `docs/net-protocol.txt`
Expand Down
5 changes: 5 additions & 0 deletions docs/man/apc_modbus.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ Set the Modbus slave id. The default slave id is 1.
Set the Modbus response timeout. The default timeout is set by libmodbus. It can
be good to set a higher timeout on TCP connections with high latency.

*modbus_retries*='num'::
Set the number of retries for Modbus register reads on timeout errors.
The default is 3 retries. After exhausting retries (or on non-timeout errors),
the connection is closed and will be re-established on the next update cycle.

BUGS
----

Expand Down
83 changes: 35 additions & 48 deletions drivers/apc_modbus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ static int is_open = 0;
static double power_nominal;
static double realpower_nominal;
static int64_t last_send_time = 0;
static int modbus_retries = 3;

/* Function declarations */
static int _apc_modbus_read_inventory(void);
Expand Down Expand Up @@ -988,6 +989,7 @@ static apc_modbus_register_t* _apc_modbus_find_register_variable(const char *nut
static void _apc_modbus_close(int free_modbus)
{
if (modbus_ctx != NULL) {
upslogx(LOG_INFO, "%s: Closing connection", __func__);
modbus_close(modbus_ctx);
if (free_modbus) {
modbus_free(modbus_ctx);
Expand Down Expand Up @@ -1080,58 +1082,32 @@ static void _apc_modbus_interframe_delay(void)
last_send_time = current_time;
}

static void _apc_modbus_handle_error(modbus_t *ctx)
static int _apc_modbus_read_registers(modbus_t *ctx, int addr, int nb, uint16_t *dest)
{
static int flush_retries = 0;
int flush = 0;
#ifdef WIN32
int wsa_error;
#endif /* WIN32 */
int res;
int retries = modbus_retries;
int saved_errno;

/*
* We could enable MODBUS_ERROR_RECOVERY_LINK but that would just get stuck
* in libmodbus until recovery. The only indication of this is that the
* program is stuck and debug prints by libmodbus, which we don't want to
* enable on release.
*
* Instead we detect timout errors and do a sleep + flush, on every other
* error or when flush didn't work we do a reconnect.
*/
while (retries-- > 0) {
_apc_modbus_interframe_delay();

#ifdef WIN32
wsa_error = WSAGetLastError();
if (wsa_error == WSAETIMEDOUT) {
flush = 1;
}
#else /* !WIN32 */
if (errno == ETIMEDOUT) {
flush = 1;
}
#endif /* !WIN32 */
res = modbus_read_registers(ctx, addr, nb, dest);
saved_errno = errno;
_apc_modbus_interframe_delay_reset();
if (res > 0) {
return 1;
}

if (flush > 0 && flush_retries++ < 5) {
usleep(1000000);
modbus_flush(ctx);
} else {
flush_retries = 0;
upslogx(LOG_ERR, "%s: Closing connection", __func__);
/* Close without free, will retry connection on next update */
_apc_modbus_close(0);
upslogx(LOG_ERR, "%s: Read of %d:%d failed: %s (%s)", __func__, addr, addr + nb, modbus_strerror(saved_errno), device_path);

if (saved_errno != ETIMEDOUT) {
break;
}
}
}

static int _apc_modbus_read_registers(modbus_t *ctx, int addr, int nb, uint16_t *dest)
{
_apc_modbus_interframe_delay();
_apc_modbus_close(0);

if (modbus_read_registers(ctx, addr, nb, dest) > 0) {
_apc_modbus_interframe_delay_reset();
return 1;
} else {
upslogx(LOG_ERR, "%s: Read of %d:%d failed: %s (%s)", __func__, addr, addr + nb, modbus_strerror(errno), device_path);
_apc_modbus_handle_error(ctx);
return 0;
}
return 0;
}

static int _apc_modbus_update_value(apc_modbus_register_t *regs_info, const uint16_t *regs, const size_t regs_len)
Expand Down Expand Up @@ -1499,7 +1475,7 @@ static int _apc_modbus_handle_outlet_cmd(const char *nut_cmdname, const char *ex
if (modbus_write_registers(modbus_ctx, APC_MODBUS_OUTLETCOMMAND_BF_REG, 2, value) < 0) {
upslogx(LOG_ERR, "%s: Write of outlet command failed: %s (%s)",
__func__, modbus_strerror(errno), device_path);
_apc_modbus_handle_error(modbus_ctx);
_apc_modbus_close(0);
*result = STAT_INSTCMD_FAILED;
return 1;
}
Expand Down Expand Up @@ -1681,7 +1657,7 @@ static int _apc_modbus_setvar(const char *nut_varname, const char *str_value)
nb = apc_value->modbus_len;
if (modbus_write_registers(modbus_ctx, addr, nb, reg_value) < 0) {
upslogx(LOG_ERR, "%s: Write of %d:%d failed: %s (%s)", __func__, addr, addr + nb - 1, modbus_strerror(errno), device_path);
_apc_modbus_handle_error(modbus_ctx);
_apc_modbus_close(0);
return STAT_SET_FAILED;
}

Expand Down Expand Up @@ -1760,7 +1736,7 @@ static int _apc_modbus_instcmd(const char *nut_cmdname, const char *extra)
upslog_INSTCMD_POWERSTATE_CHECKED(nut_cmdname, extra);
if (modbus_write_registers(modbus_ctx, addr, nb, value) < 0) {
upslogx(LOG_INSTCMD_FAILED, "%s: Write of %d:%d failed: %s (%s)", __func__, addr, addr + nb, modbus_strerror(errno), device_path);
_apc_modbus_handle_error(modbus_ctx);
_apc_modbus_close(0);
return STAT_INSTCMD_FAILED;
}

Expand Down Expand Up @@ -1801,6 +1777,7 @@ void upsdrv_updateinfo(void)
dstate_datastale();
return;
}
upslogx(LOG_INFO, "Opened modbus successfully");
}

alarm_init();
Expand Down Expand Up @@ -1962,6 +1939,7 @@ void upsdrv_makevartable(void)
#endif /* defined NUT_MODBUS_HAS_USB */
addvar(VAR_VALUE, "slaveid", "Modbus slave id (default=1)");
addvar(VAR_VALUE, "response_timeout_ms", "Modbus response timeout in milliseconds");
addvar(VAR_VALUE, "modbus_retries", "Number of retries for Modbus register reads on timeout errors (default=3)");

/* Serial RTU parameters */
addvar(VAR_VALUE, "baudrate", "Modbus serial RTU communication speed in baud (default=9600)");
Expand Down Expand Up @@ -2311,6 +2289,15 @@ void upsdrv_initups(void)
#endif /* NUT_MODBUS_TIMEOUT_ARG_* */
}

val = getval("modbus_retries");
if (val != NULL) {
modbus_retries = atoi(val);
if (modbus_retries < 1) {
modbus_free(modbus_ctx);
fatalx(EXIT_FAILURE, "modbus_retries needs to be at least 1");
}
}

if (modbus_connect(modbus_ctx) == -1) {
modbus_free(modbus_ctx);
fatalx(EXIT_FAILURE, "modbus_connect: unable to connect: %s", modbus_strerror(errno));
Expand Down
Loading