Skip to content

Commit f2cd302

Browse files
haulfihaulfi
committed
Fix issues from previous PR review
- Bumped internal version to 0.09 (two-digit format) - Replaced single-line // comments with /* ... */ block comments - Added missing variable descriptions to the man page - Preserved original authors in metadata - Standardized config variable names to align with nut-names.txt where possible - Declared all variables at top of scope as per C89 style Feedback originally provided by @jimklimov on PR #2986 (superseded by this commit)
1 parent 1a24576 commit f2cd302

File tree

2 files changed

+82
-46
lines changed

2 files changed

+82
-46
lines changed

‎docs/man/phoenixcontact_modbus.txt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,39 @@ INSTANT COMMANDS
8686

8787
This driver doesn't support any instant commands.
8888

89+
CONFIGURABLE VARIABLES
90+
----------------------
91+
92+
The following variables can be set in the NUT configuration to override or monitor
93+
UPS behavior using the `phoenixcontact_modbus` driver:
94+
95+
.B delay.shutdown
96+
Delay before initiating PC shutdown (in seconds).
97+
98+
.B delay.pc_shutdown
99+
Time allowed for PC to shutdown (in seconds). Maximum is 300 seconds.
100+
101+
.B delay.pc_reset
102+
Duration for which output power remains off before rebooting the system (in seconds).
103+
104+
.B delay.mains_return
105+
Delay before switching back to mains after power is restored (in seconds).
106+
107+
.B mode.selector
108+
UPS operating mode. For example, `9` corresponds to PC-Mode.
109+
110+
.B bat.warning_soh
111+
Battery warning threshold for State of Health, in percent.
112+
113+
.B bat.voltage_low
114+
Threshold voltage (in V) below which the UPS will switch to battery mode.
115+
116+
.B mains.voltage_high
117+
Voltage threshold (in V) above which the UPS will consider mains power restored.
118+
119+
.B mains.return_delay
120+
Time (in seconds) that mains voltage must remain above the `voltage_high` threshold before switching back.
121+
89122
AUTHOR
90123
------
91124

‎drivers/phoenixcontact_modbus.c

Lines changed: 49 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
#endif
3333

3434
#define DRIVER_NAME "NUT PhoenixContact Modbus driver (libmodbus link type: " NUT_MODBUS_LINKTYPE_STR ")"
35-
#define DRIVER_VERSION "0.1"
35+
#define DRIVER_VERSION "0.09"
3636

3737
#define CHECK_BIT(var,pos) ((var) & (1<<(pos)))
3838
#define MODBUS_SLAVE_ID 192
@@ -73,34 +73,34 @@
7373

7474
/* --- Shutdown & Mode Configuration --- */
7575

76-
// Macro for fetching a uint16_t config value with fallback
76+
/* Macro for fetching a uint16_t config value with fallback*/
7777
#define GETVAL_U16(name, fallback) (getval(name) ? (uint16_t)atoi(getval(name)) : (fallback))
7878

79-
// Time [s] after entering battery mode before shutdown signal (bit 15) is triggered
80-
#define REG_PC_SHUTDOWN_DELAY 0x105A // default: 60s, range: 1–65535
79+
/* Time [s] after entering battery mode before shutdown signal (bit 15) is triggered */
80+
#define REG_PC_SHUTDOWN_DELAY 0x105A /*default: 60s, range: 1–65535*/
8181

82-
// Time [s] allowed for PC to shut down before output turns off
83-
#define REG_PC_SHUTDOWN_TIME 0x105D // default: 120s, range: 1–3600
82+
/*Time [s] allowed for PC to shut down before output turns off*/
83+
#define REG_PC_SHUTDOWN_TIME 0x105D /*default: 120s, range: 1–3600*/
8484

85-
// Time [s] output is off after PC shutdown before reboot
86-
#define REG_PC_RESET_TIME 0x105E // default: 10s, range: 0–60
85+
/*Time [s] output is off after PC shutdown before reboot*/
86+
#define REG_PC_RESET_TIME 0x105E /*default: 10s, range: 0–60*/
8787

88-
// Time [s] input voltage must be above threshold to return to mains
89-
#define REG_MAINS_RETURN_DELAY 0x1058 // default: 10s
88+
/*Time [s] input voltage must be above threshold to return to mains*/
89+
#define REG_MAINS_RETURN_DELAY 0x1058 /*default: 10s*/
9090

91-
// Selector mode switch: PC-Mode = 9
92-
#define REG_MODE_SELECTOR_SWITCH 0x1074 // default: 0–9
91+
/*Selector mode switch: PC-Mode = 9*/
92+
#define REG_MODE_SELECTOR_SWITCH 0x1074 /*default: 0–9*/
9393

9494
/* --- Battery Monitoring --- */
9595

96-
// SOH warning threshold [%]
97-
#define REG_WARNING_SOH_THRESHOLD 0x1071 // default: 0 (disabled), range: 1–100
96+
/*SOH warning threshold [%]*/
97+
#define REG_WARNING_SOH_THRESHOLD 0x1071 /*default: 0 (disabled), range: 1–100*/
9898

99-
// Switch to battery mode if below this input voltage [mV]
100-
#define REG_VOLTAGE_BELOW_BATTERY 0x1056 // example: 21000
99+
/*Switch to battery mode if below this input voltage [mV]*/
100+
#define REG_VOLTAGE_BELOW_BATTERY 0x1056 /*example: 21000*/
101101

102-
// Switch to mains mode if above this voltage [mV]
103-
#define REG_VOLTAGE_ABOVE_MAINS 0x1057 // example: 23000
102+
/*Switch to mains mode if above this voltage [mV]*/
103+
#define REG_VOLTAGE_ABOVE_MAINS 0x1057 /*example: 23000*/
104104

105105
typedef enum
106106
{
@@ -126,13 +126,13 @@
126126

127127
static const delay_param_t delay_params[] = {
128128
{ "ups.delay.shutdown", REG_PC_SHUTDOWN_DELAY},
129-
{ "ups.delay.pc_shutdown", REG_PC_SHUTDOWN_TIME},
130-
{ "ups.delay.pc_reset", REG_PC_RESET_TIME},
131-
{ "ups.delay.mains_return", REG_MAINS_RETURN_DELAY},
129+
{ "ups.timer.shutdown", REG_PC_SHUTDOWN_TIME},
130+
{ "ups.timer.start", REG_PC_RESET_TIME},
131+
{ "ups.delay.start", REG_MAINS_RETURN_DELAY},
132132
{ "ups.mode.selector", REG_MODE_SELECTOR_SWITCH},
133-
{ "ups.bat.warning_soh", REG_WARNING_SOH_THRESHOLD},
134-
{ "ups.bat.voltage_low", REG_VOLTAGE_BELOW_BATTERY},
135-
{ "ups.mains.voltage_high", REG_VOLTAGE_ABOVE_MAINS }
133+
{ "battery.warning_soh", REG_WARNING_SOH_THRESHOLD},
134+
{ "input.voltage.low.critical", REG_VOLTAGE_BELOW_BATTERY},
135+
{ "input.voltage.high.critical", REG_VOLTAGE_ABOVE_MAINS }
136136
};
137137

138138

@@ -148,16 +148,16 @@
148148
upsdrv_info_t upsdrv_info = {
149149
DRIVER_NAME,
150150
DRIVER_VERSION,
151-
"Ulfat Hasangarayev <ulfathasangarayev@gmail.com>\n",
151+
"Spiros Ioannou <sivann@inaccess.com>\n",
152152
DRV_BETA,
153153
{NULL}
154154
};
155155

156156
int write_uint32_register(modbus_t *ctx, int reg, uint32_t value)
157157
{
158158
uint16_t regs[2];
159-
regs[0] = value >> 16; // High word
160-
regs[1] = value & 0xFFFF; // Low word
159+
regs[0] = value >> 16; /*High word*/
160+
regs[1] = value & 0xFFFF; /*Low word*/
161161

162162
int ret = modbus_write_registers(ctx, reg, 2, regs);
163163
if (ret == -1) {
@@ -170,12 +170,14 @@
170170
int write_uint32_reg_bit(modbus_t *ctx, int reg, int bit_index, bool bit_value)
171171
{
172172
uint16_t regs[2];
173+
uint32_t val;
174+
173175
if (modbus_read_registers(ctx, reg, 2, regs) != 2) {
174176
upslogx(LOG_ERR, "Failed to read 32-bit register 0x%04X: %s", reg, modbus_strerror(errno));
175177
return -1;
176178
}
177179

178-
uint32_t val = ((uint32_t)regs[0] << 16) | regs[1];
180+
val = ((uint32_t)regs[0] << 16) | regs[1];
179181

180182
if (bit_value)
181183
val |= (1U << bit_index);
@@ -198,14 +200,14 @@
198200

199201
write_uint32_reg_bit(ctx, 0x1040, 5, false);
200202

201-
modbus_write_register(ctx, REG_PC_SHUTDOWN_DELAY, GETVAL_U16("delay.shutdown", 60));
202-
modbus_write_register(ctx, REG_PC_SHUTDOWN_TIME, GETVAL_U16("delay.pc_shutdown", 60));
203-
modbus_write_register(ctx, REG_PC_RESET_TIME, GETVAL_U16("delay.pc_reset", 5));
204-
modbus_write_register(ctx, REG_WARNING_SOH_THRESHOLD, GETVAL_U16("bat.warning_soh", 20));
205-
modbus_write_register(ctx, REG_MODE_SELECTOR_SWITCH, GETVAL_U16("mode.selector", 9));
206-
modbus_write_register(ctx, REG_VOLTAGE_BELOW_BATTERY, GETVAL_U16("bat.voltage_low", 21000));
207-
modbus_write_register(ctx, REG_VOLTAGE_ABOVE_MAINS, GETVAL_U16("mains.voltage_high", 23000));
208-
modbus_write_register(ctx, REG_MAINS_RETURN_DELAY, GETVAL_U16("mains.return_delay", 10));
203+
modbus_write_register(ctx, REG_PC_SHUTDOWN_DELAY, GETVAL_U16("delay.shutdown", 60));
204+
modbus_write_register(ctx, REG_PC_SHUTDOWN_TIME, GETVAL_U16("timer.shutdown", 60));
205+
modbus_write_register(ctx, REG_PC_RESET_TIME, GETVAL_U16("timer.start", 5));
206+
modbus_write_register(ctx, REG_WARNING_SOH_THRESHOLD, GETVAL_U16("battery.warning_soh", 20));
207+
modbus_write_register(ctx, REG_MODE_SELECTOR_SWITCH, GETVAL_U16("mode.selector", 9));
208+
modbus_write_register(ctx, REG_VOLTAGE_BELOW_BATTERY, GETVAL_U16("voltage.low.critical", 21000));
209+
modbus_write_register(ctx, REG_VOLTAGE_ABOVE_MAINS, GETVAL_U16("voltage.high.critical", 29000));
210+
modbus_write_register(ctx, REG_MAINS_RETURN_DELAY, GETVAL_U16("delay.start", 10));
209211

210212
/* the value 0xFFFDFFFF sets bit 17 low so that the mode selector switch is overwritten in software */
211213
write_uint32_register(ctx, 0x1076, 0xFFFDFFFF);
@@ -232,11 +234,13 @@
232234

233235
mrir(modbus_ctx, 0x0005, 4, tab_reg);
234236

235-
/* Method provided from Phoenix Conatct to establish the UPS model:
237+
/*
238+
Method provided from Phoenix Conatct to establish the UPS model:
236239
Read registers from 0x0005 to 0x0008 and "concatenate" them with the order
237240
0x0008 0x0007 0x0006 0x0005 in hex form, convert the obtained number from hex to dec.
238241
The first 7 most significant digits of the number in dec form are the part number of
239-
the UPS.*/
242+
the UPS.
243+
*/
240244

241245
PartNumber = (tab_reg[3] << 16) + tab_reg[2];
242246
PartNumber = (PartNumber << 16) + tab_reg[1];
@@ -337,6 +341,7 @@
337341
uint16_t battery_runtime;
338342
uint16_t battery_capacity;
339343
uint16_t output_current;
344+
uint16_t value = 0;
340345

341346
errcount = 0;
342347

@@ -348,7 +353,6 @@
348353
case QUINT4_UPS:
349354

350355
for (size_t i = 0; i < sizeof(delay_params) / sizeof(delay_params[0]); i++) {
351-
uint16_t value = 0;
352356
if (modbus_read_registers(modbus_ctx, delay_params[i].reg_addr, 1, &value) != -1) {
353357
dstate_setinfo(delay_params[i].nut_name, "%d", value);
354358
} else {
@@ -790,14 +794,13 @@
790794
void upsdrv_makevartable(void)
791795
{
792796
addvar(VAR_VALUE, "delay.shutdown", "Delay before initiating PC shutdown (in seconds)");
793-
addvar(VAR_VALUE, "delay.pc_shutdown", "Time allowed for PC to shutdown (in seconds)");
794-
addvar(VAR_VALUE, "delay.pc_reset", "Duration of output off before reboot (in seconds)");
795-
addvar(VAR_VALUE, "delay.mains_return", "Delay before switching back to mains (in seconds)");
797+
addvar(VAR_VALUE, "timer.shutdown", "Time allowed for PC to shutdown (in seconds)");
798+
addvar(VAR_VALUE, "timer.start", "Duration of output off before reboot (in seconds)");
799+
addvar(VAR_VALUE, "delay.start", "Delay before switching back to mains (in seconds)");
796800
addvar(VAR_VALUE, "mode.selector", "UPS operating mode (e.g. 9 = PC-Mode)");
797-
addvar(VAR_VALUE, "bat.warning_soh", "Battery warning SOH threshold (%)");
798-
addvar(VAR_VALUE, "bat.voltage_low", "Threshold [V] to switch to battery mode");
799-
addvar(VAR_VALUE, "mains.voltage_high", "Threshold [V] to return to mains mode");
800-
addvar(VAR_VALUE, "mains.return_delay", "Time [s] above threshold before switching back to mains");
801+
addvar(VAR_VALUE, "battery.warning_soh", "Battery warning SOH threshold (%)");
802+
addvar(VAR_VALUE, "voltage.low.critical", "Threshold [V] to switch to battery mode");
803+
addvar(VAR_VALUE, "voltage.high.critical", "Threshold [V] to return to mains mode");
801804
}
802805

803806
void upsdrv_initups(void)

0 commit comments

Comments
 (0)