Skip to content

Commit db53ed6

Browse files
jimklimovjimklimov
authored
Merge d27c8e7 into 876408d 
2 parents 876408d + d27c8e7 commit db53ed6

File tree

4 files changed

+112
-25
lines changed

4 files changed

+112
-25
lines changed

‎NEWS.adoc‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,15 @@ https://github.com/networkupstools/nut/milestone/9
179179
descriptors was extended to cover a newly reported case of nominal UPS
180180
power being incorrectly reported due to an unrealistically low maximum
181181
threshold, as seen with a EC850LCD device. [issue #2917, PR #2919]
182-
* Added APC BKxxxM2-CH to list of devices where `lbrb_log_delay_sec=N` may be
183-
necessary to address spurious LOWBATT and REPLACEBATT events. [PR #3007,
184-
issue #2347, issue #3006]
182+
* Added APC BVKxxxM2 and BKxxxM2-CH to list of devices where
183+
`lbrb_log_delay_sec=N` may be necessary to address spurious LOWBATT
184+
and REPLACEBATT events. [PR #2942, PR #3007, issue #2347, issue #3006]
185+
* The `powercom-hid` subdriver did not handle delayed shutdown commands
186+
properly, at least as of RAPTOR RPT-1500AP LCD models. The RPT series
187+
seem to have same USB protocol like Smart KING Pro series, and that
188+
only supports a specific set of possible delays for Shutdown commands
189+
(certain fractions or counts of whole minutes) -- which should now be
190+
handled if `powercom_sdcmd_discrete_delay` flag is set. [issue #3000]
185191

186192
- New NUT drivers:
187193
* Introduced a `ve-direct` driver for Victron Energy UPS/solar panels

‎docs/man/usbhid-ups.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,15 @@ firmwares out there with opposite behaviors, we provide this toggle to use
223223
old behavior in a particular deployment. Maybe it was just a bug and nobody
224224
needs this fall-back...
225225

226+
*powercom_sdcmd_discrete_delay*::
227+
Some devices (Raptor and Smart KING Pro series) follow the protocol
228+
where we can not set arbitrary value of shutdown delay in seconds
229+
(like in other powercom UPSes), but must use values only from the
230+
"Table of possible delays for Shutdown commands" specified in the
231+
protocol document. This option causes the driver to convert delays
232+
between seconds (in standard NUT variables) and nearest indexed
233+
entry from that table.
234+
226235
*explore*::
227236
With this option, the driver will connect to any device, including
228237
ones that are not yet supported. This must always be combined with the

‎drivers/powercom-hid.c‎

Lines changed: 91 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
#include "powercom-hid.h"
2727
#include "usb-common.h"
2828

29-
#define POWERCOM_HID_VERSION "PowerCOM HID 0.71"
29+
#define POWERCOM_HID_VERSION "PowerCOM HID 0.72"
3030
/* FIXME: experimental flag to be put in upsdrv_info */
3131

3232
/* PowerCOM */
@@ -62,10 +62,30 @@ static char powercom_scratch_buf[32];
6262
*/
6363
static char powercom_sdcmd_byte_order_fallback = 0;
6464

65+
/* Some devices (Raptor and Smart KING Pro series) follow the protocol
66+
* where we can not set arbitrary value of shutdown delay in seconds
67+
* (like in other powercom UPSes), but the shutdown delay can be set
68+
* only from table "Table of possible delays for Shutdown commands"
69+
* specified at page 17 of the protocol document:
70+
* https://networkupstools.org/protocols/powercom/Software_USB_communication_controller_SKP_series.doc
71+
*
72+
* "Table of possible delays for Shutdown commands" (mentioned for
73+
* `DelayBeforeShutdown` and `DelayBeforeStartup` HID Usages):
74+
* Index 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18
75+
* Value 12s 18s 24s 30s 36s 42s 48s 54s 1m 2m 3m 4m 5m 6m 7m 8m 9m 10m
76+
* Sent to UPS .2 .3 .4 .5 .6 .7 .8 .9 01 02 03 04 05 06 07 08 09 10
77+
*
78+
*/
79+
static char powercom_sdcmd_discrete_delay = 0;
80+
6581
static const char *powercom_startup_fun(double value)
6682
{
6783
uint16_t i = value;
6884

85+
/* For powercom_sdcmd_discrete_delay we also read minutes
86+
* from DelayBeforeStartup (same as for older dialects).
87+
* FIXME: ...but theoretically they can be 32-bit (1..99999)
88+
*/
6989
snprintf(powercom_scratch_buf, sizeof(powercom_scratch_buf), "%d", 60 * (((i & 0x00FF) << 8) + (i >> 8)));
7090
upsdebugx(3, "%s: value = %.0f, buf = %s", __func__, value, powercom_scratch_buf);
7191

@@ -75,20 +95,40 @@ static const char *powercom_startup_fun(double value)
7595
static double powercom_startup_nuf(const char *value)
7696
{
7797
const char *s = dstate_getinfo("ups.delay.start");
78-
uint16_t val, command;
98+
uint32_t val, command;
7999
int iv;
80100

81-
iv = atoi(value ? value : s) / 60;
82-
if (iv < 0 || (intmax_t)iv > (intmax_t)UINT16_MAX) {
83-
upsdebugx(0, "%s: value = %d is not in uint16_t range", __func__, iv);
101+
/* Start with seconds "as is" - convert into whole minutes */
102+
iv = atoi(value ? value : s) / 60; /* minutes */
103+
#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) )
104+
# pragma GCC diagnostic push
105+
#endif
106+
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS
107+
# pragma GCC diagnostic ignored "-Wtype-limits"
108+
#endif
109+
#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE
110+
# pragma GCC diagnostic ignored "-Wtautological-constant-out-of-range-compare"
111+
#endif
112+
if (iv < 0 || (intmax_t)iv > (intmax_t)UINT32_MAX) {
113+
#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) )
114+
# pragma GCC diagnostic pop
115+
#endif
116+
upsdebugx(0, "%s: value = %d is not in uint32_t range", __func__, iv);
84117
return 0;
85118
}
86119

87-
/* COMMENTME: What are we doing here, a byte-swap in the word? */
88-
val = (uint16_t)iv;
89-
command = (uint16_t)(val << 8);
90-
command += (uint16_t)(val >> 8);
91-
upsdebugx(3, "%s: value = %s, command = %04X", __func__, value, command);
120+
if (powercom_sdcmd_discrete_delay) {
121+
/* Per spec, DelayBeforeStartup reads and writes
122+
* 4-byte "mmmm" values in minutes (1..99999) */
123+
command = (uint32_t)iv;
124+
} else {
125+
/* COMMENTME: What are we doing here, a byte-swap in the word? */
126+
val = (uint16_t)iv;
127+
command = (uint16_t)(val << 8);
128+
command += (uint16_t)(val >> 8);
129+
}
130+
131+
upsdebugx(3, "%s: value = %s, command = 0x%08X", __func__, value, command);
92132

93133
return command;
94134
}
@@ -101,6 +141,12 @@ static const char *powercom_shutdown_fun(double value)
101141
{
102142
uint16_t i = value;
103143

144+
/* NOTE: for powercom_sdcmd_discrete_delay mode it seems we
145+
* do not read DelayBeforeShutdown at all (not for time),
146+
* the value is stored and retrieved as DelayBeforeStartup.
147+
* FIXME: Should anything be changed here?
148+
*/
149+
104150
if (powercom_sdcmd_byte_order_fallback) {
105151
/* Legacy behavior */
106152
snprintf(powercom_scratch_buf, sizeof(powercom_scratch_buf), "%d", 60 * (i & 0x00FF) + (i >> 8));
@@ -120,25 +166,46 @@ static double powercom_shutdown_nuf(const char *value)
120166
uint16_t val, command;
121167
int iv;
122168

123-
iv = atoi(value ? value : s);
169+
iv = atoi(value ? value : s); /* seconds */
124170
if (iv < 0 || (intmax_t)iv > (intmax_t)UINT16_MAX) {
125171
upsdebugx(0, "%s: value = %d is not in uint16_t range", __func__, iv);
126172
return 0;
127173
}
128174

129-
val = (uint16_t)iv;
130-
val = val ? val : 1; /* 0 sets the maximum delay */
131-
if (powercom_sdcmd_byte_order_fallback) {
132-
/* Legacy behavior */
133-
command = ((uint16_t)((val % 60) << 8)) + (uint16_t)(val / 60);
134-
command |= 0x4000; /* AC RESTART NORMAL ENABLE */
175+
if (powercom_sdcmd_discrete_delay) {
176+
if (iv <= 12) command = 1;
177+
else if (iv <= 18) command = 2;
178+
else if (iv <= 24) command = 3;
179+
else if (iv <= 30) command = 4;
180+
else if (iv <= 36) command = 5;
181+
else if (iv <= 42) command = 6;
182+
else if (iv <= 48) command = 7;
183+
else if (iv <= 54) command = 8;
184+
else if (iv <= 60) command = 9;
185+
else if (iv <= 120) command = 10;
186+
else if (iv <= 180) command = 11;
187+
else if (iv <= 240) command = 12;
188+
else if (iv <= 300) command = 13;
189+
else if (iv <= 360) command = 14;
190+
else if (iv <= 420) command = 15;
191+
else if (iv <= 480) command = 16;
192+
else if (iv <= 540) command = 17;
193+
else command = 18;
135194
} else {
136-
/* New default */
137-
command = ((uint16_t)((val / 60) << 8)) + (uint16_t)(val % 60);
138-
command |= 0x0040; /* AC RESTART NORMAL ENABLE */
195+
val = (uint16_t)iv;
196+
val = val ? val : 1; /* 0 sets the maximum delay */
197+
if (powercom_sdcmd_byte_order_fallback) {
198+
/* Legacy behavior */
199+
command = ((uint16_t)((val % 60) << 8)) + (uint16_t)(val / 60);
200+
command |= 0x4000; /* AC RESTART NORMAL ENABLE */
201+
} else {
202+
/* New default */
203+
command = ((uint16_t)((val / 60) << 8)) + (uint16_t)(val % 60);
204+
command |= 0x0040; /* AC RESTART NORMAL ENABLE */
205+
}
139206
}
140207

141-
upsdebugx(3, "%s: value = %s, command = %04X", __func__, value, command);
208+
upsdebugx(3, "%s: value = %s, command = 0x%04X", __func__, value, command);
142209

143210
return command;
144211
}
@@ -153,6 +220,7 @@ static double powercom_stayoff_nuf(const char *value)
153220
uint16_t val, command;
154221
int iv;
155222

223+
/* FIXME: Anything for powercom_sdcmd_discrete_delay? */
156224
iv = atoi(value ? value : s);
157225
if (iv < 0 || (intmax_t)iv > (intmax_t)UINT16_MAX) {
158226
upsdebugx(0, "%s: value = %d is not in uint16_t range", __func__, iv);
@@ -171,7 +239,7 @@ static double powercom_stayoff_nuf(const char *value)
171239
command |= 0x0080; /* AC RESTART NORMAL DISABLE */
172240
}
173241

174-
upsdebugx(3, "%s: value = %s, command = %04X", __func__, value, command);
242+
upsdebugx(3, "%s: value = %s, command = 0x%04X", __func__, value, command);
175243

176244
return command;
177245
}
@@ -592,6 +660,7 @@ static int powercom_claim(HIDDevice_t *hd)
592660

593661
accept:
594662
powercom_sdcmd_byte_order_fallback = testvar("powercom_sdcmd_byte_order_fallback");
663+
powercom_sdcmd_discrete_delay = testvar("powercom_sdcmd_discrete_delay");
595664

596665
return 1;
597666
}

‎drivers/usbhid-ups.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,9 @@ void upsdrv_makevartable(void)
11441144
addvar(VAR_FLAG, "powercom_sdcmd_byte_order_fallback",
11451145
"Set to use legacy byte order for Powercom HID shutdown commands. Either it was wrong forever, or some older devices/firmwares had it the other way around");
11461146

1147+
addvar(VAR_FLAG, "powercom_sdcmd_discrete_delay",
1148+
"Set to use discrete timing table for Powercom HID shutdown commands (Raptor and Smart KING Pro series)");
1149+
11471150
#if !((defined SHUT_MODE) && SHUT_MODE)
11481151
addvar(VAR_VALUE, "subdriver", "Explicit USB HID subdriver selection");
11491152

0 commit comments

Comments
 (0)