Fix cmd for ecomode,bypass,experimental.ecomode.[start/stop].auto

[masterwishx]

Signed-off-by: DaRK AnGeL 28630321+masterwishx@users.noreply.github.com

experimental.ecomode.[start/stop].auto move to uprw + set NULL to last argument for bypass and ecomode commands (cmd)

For commands (cmd) we need set NULL at last parametr or more code to add ?
When we call function instead NULL, we got error:
hu_find_valinfo: no matching HID value for this INFO_* value (1)

So just set back to NULL for thouse functions to avoid errors and fix the commands functionality.

experimental.ecomode.start.auto
"experimental.ecomode.stop.auto

We move to variables for avoid same issue and enable to run the function at last parametr

Last parameter ( in struct "info_lkp_t" ) is : lookup table between HID and NUT values

nut/drivers/usbhid-ups.h

Lines 184 to 185 in 5597cc7

	info_lkp_t *hid2info; /* lookup table between HID and NUT values */
	/* if HU_FLAG_ENUM is set, hid2info is also used

nut/drivers/usbhid-ups.h

Lines 78 to 84 in 5597cc7

	typedef struct {
	const long hid_value; /* HID value */
	const char *nut_value; /* NUT value */
	const char *(*fun)(double hid_value); /* optional HID to NUT mapping */
	double (*nuf)(const char *nut_value); /* optional NUT to HID mapping */
	} info_lkp_t;

Final :

We move this idea to #2949 for continue to make it posibly work and clean this pr for variables and commands that already are working fine

[jimklimov]

@masterwishx : can you please elaborate a comment above on how this change fixes some behaviors?

Shouldn't this PR also change descriptions in data/cmdvartab and docs/nut-names.txt, if a command became a settable value?

[masterwishx]

@masterwishx : can you please elaborate a comment above on how this change fixes some behaviors?

Shouldn't this PR also change descriptions in data/cmdvartab and docs/nut-names.txt, if a command became a settable value?

I'm not really sure but seems commands need to be NULL at last parametr or need some more code as you can see it throw error in log of #2685.
With NULL it was working as I tested back then will try to check again but need to compile, but for this had some error Becouse unraid 7.x had some changes with some librarys.

I will check again cmdvartab and nut-names.txt for changes, yesterday didn't found if needed

[desertwitch]

Did you test this to work with your UPS?

Just asking because we changed around a lot of things with the Eaton subdrivers recently.
Did we ever establish a baseline of things working as they should with ECO/ABM, the other changes?

What is working right now and what is not working?
Will test some more with my 5P series as well, time permitting.

[masterwishx]

Did you test this to work with your UPS?

Just asking because we changed around a lot of things with the Eaton subdrivers recently.
Did we ever establish a baseline of things working as they should with ECO/ABM, the other changes?

What is working right now and what is not working?
Will test some more with my 5P series as well, time permitting.

with NULL tested back then , now tested and confirm not working without NULL , for test again i need to compile my pr but can 't becouse of missing/changed libs in unraid if you can help with setting file will be cool :)

[masterwishx]

Did you test this to work with your UPS?

Just asking because we changed around a lot of things with the Eaton subdrivers recently.
Did we ever establish a baseline of things working as they should with ECO/ABM, the other changes?

What is working right now and what is not working?
Will test some more with my 5P series as well, time permitting.

ABM (missing ABM in my unit 9E) tested back then all working fine for my model 9E (shown Online charging always )

[masterwishx]

Did you test this to work with your UPS?

Just asking because we changed around a lot of things with the Eaton subdrivers recently.
Did we ever establish a baseline of things working as they should with ECO/ABM, the other changes?

What is working right now and what is not working?
Will test some more with my 5P series as well, time permitting.

with NULL tested back then , now tested and confirm not working without NULL , for test again i need to compile my pr but can't becouse of missing/changed libs in unraid if you can help with setting file will be cool :)

[desertwitch]

Honestly, I'd just compile with Slackware, there's no benefit in setting up a building stage in Unraid all over. I use Slackware 15.0 and 15.1, haven't needed to change my setups for months, most of the things are contained in Slackware out of the box (for better or worse).

[masterwishx]

Honestly, I'd just compile with Slackware, there's no benefit in setting up a building stage in Unraid all over. I use Slackware 15.0 and 15.1, haven't needed to change my setups for months, most of the things are contained in Slackware out of the box (for better or worse).

Yep i also compiled like this but the problem was in file : slack-required that worked in 6.12 but not in 7.0 and you removed this file from package so not sure what libs i need here ....

aaa_glibc-solibs >= 2.37
e2fsprogs >= 1.47.0
eudev >= 3.2.12
gcc >= 13.2.0
gcc-g++ >= 13.2.0
keyutils >= 1.6.3
krb5 >= 1.21.2
libnsl >= 2.0.0
libtirpc >= 1.3.3
libtool >= 2.4.7
libusb >= 1.0.27
openssl >= 3.4.0 | openssl-solibs >= 3.4.0
zlib >= 1.3

slack-suggests:

brotli >= 1.0.9
bzip2 >= 1.0.8
cppunit >= 1.15.1
expat >= 2.5.0
fontconfig >= 2.13.92
freeipmi >= 1.6.9
freetype >= 2.13.2
gd >= 2.3.3
glib2 >= 2.76.4
graphite2 >= 1.3.14
harfbuzz >= 8.1.1
icu4c >= 73.2
libX11 >= 1.8.6
libXau >= 1.0.11
libXdmcp >= 1.1.4
libXpm >= 3.5.16
libgcrypt >= 1.10.2
libgpg-error >= 1.47
libjpeg-turbo >= 3.0.0
libmodbus >= 3.1.10
libpng >= 1.6.40
libproxy >= 0.4.18
libtiff >= 4.4.0
libwebp >= 1.3.1
libxcb >= 1.16
libxml2 >= 2.9.14
neon >= 0.32.5
net-snmp >= 5.9.4
pcre >= 8.45
powerman >= 2.3.27
sqlite >= 3.43.0
xz >= 5.4.4
zstd >= 1.5.5

[masterwishx]

So last time when tried to compile i had errors ...

[masterwishx]

im using slackware 15.0 just for info

[desertwitch]

I'd really advise you use Slackware 15.1 (current) for anything Unraid 7, and Slackware 15.0 (stable) for anything Unraid 6. Manually upgrading Slackware 15.0 packages to be compatible with Unraid 7 is a lot of (wasted) effort. So my recommendation is get the latest ISO from e.g. https://slackware.uk/people/alien-current-iso/slackware64-current-iso/ and install that, you'll find that 99% of packages required are already installed and this should work nicely with Unraid 7 with at the most 1-2 minor package changes.

P.S. You can then easily cross-reference e.g. this: https://docs.unraid.net/unraid-os/release-notes/7.1.0/
with this https://mirror.slackware.hr/slackware/slackware64-current/PACKAGES.TXT and see what needs changing.
It's also why I don't bother maintaining the slack-required file anymore, it's mostly static and a simple diff works... 😎

[masterwishx]

I don't bother maintaining the slack-required file anymore,

Thanks got it

[masterwishx]

I'd really advise you use Slackware 15.1 (current) for anything Unraid 7, and Slackware 15.0 (stable) for anything Unraid 6.

I really need it only for testing NUT to my prs, im using your NUT plugin and very happy with it !
So im on 7.1.1 now

[masterwishx]

@jimklimov

nut/drivers/mge-hid.c

Lines 2048 to 2049 in 6e5f92e

	{ "experimental.ecomode.auto", ST_FLAG_RW | ST_FLAG_STRING, 8, "UPS.PowerConverter.Input.[5].Switchable", NULL, "%.0f", HU_FLAG_SEMI_STATIC, eaton_input_eco_mode_auto_on_off_info },

maybe input.eco.switchable.auto is better ?

[jimklimov]

maybe input.eco.switchable.auto is better ?

Well, the relocated comment still says it is a command (as a settable value it is not a command anymore), and at that - one to switch something on/off.

The name like *.switchable.* implies a boolean to me, maybe hard-coded by the device (I can or can't do this) or changeable by user (I allow you to switch that); auto might be a further modifier (e.g. can be switched automatically vs. manually/by explicit command).

I am not that well-versed in this device series and their behaviors, so can't really say more than that the current text, mapping. and question seem to all "pull the cart" in different directions :)

/* Command to switch ECO(HE) Mode on/off with switch to Automatic Bypass Mode on/off */

[masterwishx]

maybe input.eco.switchable.auto is better ?

Well, the relocated comment still says it is a command (as a settable value it is not a command anymore), and at that - one to switch something on/off.

The name like *.switchable.* implies a boolean to me, maybe hard-coded by the device (I can or can't do this) or changeable by user (I allow you to switch that); auto might be a further modifier (e.g. can be switched automatically vs. manually/by explicit command).

I am not that well-versed in this device series and their behaviors, so can't really say more than that the current text, mapping. and question seem to all "pull the cart" in different directions :)

/* Command to switch ECO(HE) Mode on/off with switch to Automatic Bypass Mode on/off */

After a deeper investigation, it seems that this is not very suitable for a variable upsrw Becouse need to return back hid/nut values like (normal, ECO, ESS) same as input.eco.switchable.

in this case I need more testing and checking maybe to put as variable or maybe move all back to commands but to fix issue with missing hid/nut value for them...

In any case for now commands that fixed with NULL at the end should work fine, so maybe to move this checking /testing of
experimental.ecomode.start.auto to another pr?

[masterwishx]

I am not that well-versed in this device

For ecomode we need first enter to bypass

This command/variable just call both (bypass, ecomode) instead user run separate 2 commands /variables so maybe some think like:
"experimental.bypass.ecomode.start"

The question if you think we need command/variable like this at all? @jimklimov

[masterwishx]

@jimklimov can i try use coderabbitai in my pr maybe will help somehow ?

[AppVeyorBot]

❌ Build nut 2.8.3.3136-master failed (commit 199f8a61bf by @masterwishx)

[AppVeyorBot]

✅ Build nut 2.8.3.3137-master completed (commit 6d160d64f4 by @masterwishx)

[masterwishx]

I'd really advise you use Slackware 15.1 (current) for anything Unraid 7, and Slackware 15.0 (stable) for anything Unraid 6. Manually upgrading Slackware 15.0 packages to be compatible with Unraid 7 is a lot of (wasted) effort. So my recommendation is get the latest ISO from e.g. https://slackware.uk/people/alien-current-iso/slackware64-current-iso/ and install that, you'll find that 99% of packages required are already installed and this should work nicely with Unraid 7 with at the most 1-2 minor package changes.

Installed Slackware 15.1 from the link + packages (Powerman + Modbus + Freeipmi )
Compiled but seems have some issue when running with :

kernel: usbhid-ups[3111271]: segfault at 0 ip 000014e34d6563c7 sp 00007ffcb27daa28 error 4 in libc-2.41.so[17e3c7,14e34d500000+17c000] likely on CPU 5 (core 8, socket 0)

freeipmi-1.6.15-x86_64-1cf.txz or freeipmi-1.6.9-x86_64-1gds.txz
is needed ?

Maybe i missed somethink ?

[jimklimov]

@masterwishx : can you please retry building with the master branch first? If it still segfaults there, we might have some other bug to fix. If not - this may be about NULL changes of this PR...

Then you can probably use gdb and/or enable core dump files on the system, to get a stack trace of where the usbhid-ups program failed (I'd suspect the walk of mapping table and not expecting a NULL field in some circumstances nor checking for it).

[masterwishx]

this may be about NULL changes of this PR

OK i will but NULL at last parameter in commands is how it was working before Then without issues and how the all commands are working , so its CANT be becouse of it .

the issue can be because of missed wrong libraries that plugin use in Unraid 7.1.1 + @desertwitch maybe use some special setting maybe or files linked in plugin that i missed in my compilation ?!

[masterwishx]

I think i will remove all stuff with ecomode auto to another PR and we should answer :

Do we really need automatic ECO Mode variable or command experimental.ecomode.start.auto and experimental.ecomode.stop.auto?
that run Bypass On then ECO Mode Enable and back or Users are grown and can run two separate commands or variables :

bypass.start + experimental.ecomode.enable
bypass.stop + experimental.ecomode.disable

or

 Enable bypass mode:
upsrw -u admin -p pass -s input.bypass.switch.on=on eaton

Enable ECO mode:
upsrw -u admin -p pass -s input.eco.switchable=enable eaton

Disable bypass mode:
upsrw -u admin -p pass -s input.bypass.switch.off=off eaton

Disable ECO mode:
upsrw -u admin -p pass -s input.eco.switchable=normal eaton

@jimklimov please confirm

[jimklimov]

Sorry for the delays. Yes, it seems safer now to separate the part which is known/assumed working (and fixing a bug, so we can all benefit from leaving that one behind us), from the more questionable part...

bypass.start + experimental.ecomode.enable
bypass.stop + experimental.ecomode.disable

I think the latter would be in inverse order? e.g. experimental.ecomode.disable + bypass.stop? Does it matter in practice?

If so, this highlights the problem that even the best of us can stumble on this and being "grown" does not fully help :)

I am not too excited about mixing readable/writeable variables (whether they might be in driver's RAM only or forwarded to the device firmware) and instant commands (actually applying the variable). On one hand, they have different permissions in NUT security model; on another they might be conceptually separated in time.

For example, we have variables that set the delay for an UPS power-off after receiving the shutdown command which we can set any time we like, and separately we have actual commands to load.off or shutdown.return etc. that we call when the power action is wanted. Or we have thresholds like input.transfer.trim.high which we only write to the UPS firmware, and it starts/stops trimming by itself when the wall voltage is bad or good. Note we are not replacing the UPS firmware by NUT :)

• BTW, if the device should not have "ECO" enabled when bypass is not also activated, perhaps this combined operation is the only correct way to do it and no "auto" in the name is needed either (it may be seen as a toggle for something the device may decide to apply automatically or not, like trimming).

So if we have an action that causes the ECO mode to get into effect, or "normal" mode to get into effect, that change should be done by a command (which internally may set some UPS firmware variables and call its operations in a specific sequence).

Note that instant commands can have one optional argument (and may also require it - e.g. fail if nothing recognized is passed), so if there is some value that impacts this operation and is not required to be persisted in the driver or device, this can be used. For example, we might have a single INSTCMD to toggle the type of ECO mode (covering both "normal/off/disable", and "ECO", "ESS", "HE" or whatever buzzwords apply to this device).

[masterwishx]

I think the latter would be in inverse order? e.g. experimental.ecomode.disable + bypass.stop? Does it matter in practice?

As i was unable to test it because my unit 9E in ECO mode and cant go to online (tried all kind variations) but we were tested with user @ZivertX (with upsrw) back then before old pr with ECO was merged:

#2685 (comment)
#2697 (comment)

So it seems right order but maybe also working versa....

Anyway we will try to test again ...

[masterwishx]

So if we have an action that causes the ECO mode to get into effect, or "normal" mode to get into effect, that change should be done by a command (which internally may set some UPS firmware variables and call its operations in a specific sequence).

Yep that was basically the idea at start, but seems needs more testing etc ...

[masterwishx]

@ZivertX

im really sorry to ask you , but seems you are only user with 9SX who can help us with testing ,
i will also test on my 9E (stucked in ECO) but can only see that commands has no errors , but for real changes we need 9SX .

So if you will have time to check (we moved ecomode auto to another pr) we need to check that currently cmd/upsrw are working with no errors:

this pr to test:
https://github.com/masterwishx/nut/tree/fix-cmd-ecomode

with bypass and ecomode are works by :
cmd :

bypass.start + experimental.ecomode.enable
bypass.stop + experimental.ecomode.disable

by upsrw :

 Enable bypass mode:
upsrw -u admin -p pass -s input.bypass.switch.on=on eaton

Enable ECO mode:
upsrw -u admin -p pass -s input.eco.switchable=enable eaton

Disable bypass mode:
upsrw -u admin -p pass -s input.bypass.switch.off=off eaton

Disable ECO mode:
upsrw -u admin -p pass -s input.eco.switchable=normal eaton

if you can also please note if bypass was entered/exited befor ECO (by ups screeen or so)

Thanks

[AppVeyorBot]

❌ Build nut 2.8.3.3141-master failed (commit 4c7c6fc15d by @masterwishx)

[jimklimov]

Interim note: Not sure yet what CI complains about: AppVeyor spilled over its 1hr time slot; macos VM maybe just crashed (tends to), no log file is attached.

[ZivertX]

I've built from your branch:

driver.version: 2.8.2.2824.289-3113+g6cfd10d6f

this pr to test: https://github.com/masterwishx/nut/tree/fix-cmd-ecomode

with bypass and ecomode are works by : cmd :

bypass.start + experimental.ecomode.enable
bypass.stop + experimental.ecomode.disable

This is works

by upsrw :

 Enable bypass mode:
upsrw -u admin -p pass -s input.bypass.switch.on=on eaton

Enable ECO mode:
upsrw -u admin -p pass -s input.eco.switchable=enable eaton

Disable bypass mode:
upsrw -u admin -p pass -s input.bypass.switch.off=off eaton

Disable ECO mode:
upsrw -u admin -p pass -s input.eco.switchable=normal eaton

if you can also please note if bypass was entered/exited befor ECO (by ups screeen or so)

This is also works.

And yep, in both scenarios ups entered bypass then in eco mode, then exited from bypass before exit from eco mode

[ZivertX]

Why after entered bypass ups says exactly one or more active alarms: [**Automatic** bypass mode!].

ups.alarm: Automatic bypass mode!

Why "Automatic" , not "Manually" ? It would be more clearly for cmd command bypass.start

[masterwishx]

Why "Automatic" , not "Manually" ? It would be more clearly for cmd command bypass.start

It's just named so by Eaton vendor.
If I remember correctly it's also named so in manuals of 9E, 9SX.

And yep, in both scenarios ups entered bypass then in eco mode, then exited from bypass before exit from eco mode

Thanks a lot for testing, it's really very important. Just forgot to mention to include debug logs.

The only one thing I will ask you maybe to test next pr with automatic ecomode when it will be ready if you can and will have time for it.

@jimklimov confirmed all fine here
Can be merged?