Revise the stance on ECO/... modes in NUT standard, amend implementations

[jimklimov]

Following the long discussion in #2708, we decided that vendor-defined states named by marketing buzzwords amounting to some economic/ecological mode of operation should not be exposed in ups.status with a sense of urgency.

Instead, a new output.inverter.mode was introduced (with similar semantics) which drivers may eventually learn to populate with a number of standard states (line-interactive, online, bypass(?)) if we are sure we know them to be in place, as well as tokens for vendor:NAME:BUZZWORD to report the additional information that may be or not be of interest to the end user.

CC @masterwishx @gdt @desertwitch @arnaudquette-eaton : comments and practical testing would be welcome - e.g. that the state report would correlate with device state changes, that it would be still reported by upsmon, and remain reported while active (like only get reported when changing state, then disappear; and not flip-flop etc.)

I think the main review question here is about the spec, if that change feels acceptable as the compromise between different wishes in that discussion, and can be published as part of next NUT release.

Implementation, as long as it does not crash anything, may be improved later (after a release - i.e. minor flaws are not a blocker, it is experimental and constrained in practice to a couple of drivers so far)...

Closes: #2697
(Originally includes commits from that PR, but may post them rebased eventually, so linking for GitHub automatics this way)

[masterwishx]

I'm sorry but as posted in #2719 (comment)
I cant test it Becouse I can't get out from eco mode to online by commands seems some bug in firmware...

[jimklimov]

Well, at least the driver might pick up (or not) the currently active mode if reported by device. If some button switches the mode and we see notifications, that would be fun too...

[masterwishx]

Well, at least the driver might pick up (or not) the currently active mode if reported by device. If some button switches the mode and we see notifications, that would be fun too...

Yep the problem in my 9E no buttons to switch to /from eco mode.
Only to/from bypass buttons, but they also not working in current state. So I'm stuck in eco mode.
Only I hope I can disconnect batteries I think for enter to online back.

[masterwishx]

currently active mode if reported by device

Also was some issue when tryed to compile for unraid 7, so maybe @desertwitch can compile then I can check if current mode pickuped

[gdt]

Why are there 43 commits? I know opinions differ about PRs and other stuff, but I'm in the rebase camp, and I find unrebased hard to follow

[gdt]

I guess semantics wise, I wonder if this is really about "inverter". If it were just output.mode, that would seem better. As I am beginning to just barely understand, some eco modes are about not only a true bypass of out from in, but shutting down some other electronics. This is all about situations where the inverter is not running anyway. So maybe it's even "ups.mode".

The next thought I have, not sure it's helpful, is there is "what modes are set to operate as able" vs "what mode is it in now". I think we are talking about "what is it in now" and not configuration.

So I land on ups.mode as where this should live but not vigorously.

[jimklimov]

Naming might be addressed, the idea crossed my mind too especially as I went about looking for precedents and changing the handling of nutdrv_q voltronic (eco and eco_adv modes - the latter seems to be indeed about powering down electronics of the UPS).

[masterwishx]

Naming might be addressed, the idea crossed my mind too especially as I went about looking for precedents and changing the handling of nutdrv_q voltronic (eco and eco_adv modes - the latter seems to be indeed about powering down electronics of the UPS).

in ESS mode and i think maybe in ECO Advanced the inverter is not down but still ON and waiting to work , thats why we have about 2ms instead of 10ms when inverter is off in ECO

[jimklimov]

Naming might be addressed, the idea crossed my mind too especially as I went about looking for precedents and changing the handling of nutdrv_q voltronic (eco and eco_adv modes - the latter seems to be indeed about powering down electronics of the UPS).

in ESS mode and i think maybe in ECO Advanced the inverter is not down but still ON and waiting to work , thats why we have about 2ms instead of 10ms when inverter is off in ECO

Well, that highlights why the discussion in #2708 and this PR happened - different devices and vendors have different implementations and meanings for similar sounding buzzwords and technologies, which may even change over time for the same vendor, but we in NUT have to represent somehow in a vendor-agnostic data scheme, so that (ideally) the end-users would not care which UPS brand and model they have to know the physical impact of some configuration change whatever it is named today.

[masterwishx]

I agree, just maybe we need to mark when the inverter is working or waiting to work (like in standby) or off?

[jimklimov]

Why are there 43 commits? I know opinions differ about PRs and other stuff, but I'm in the rebase camp, and I find unrebased hard to follow

Looking at the list:

• about half are inherited from Ecomode addon 3.2 #2697 directly;
• half a dozen are merges from upstream/master to resolve conflicts as time went on (rebase might be nice about these but proved tedious to re-resolve those text mismatches);
• the remainder is the "payload" about:
  • changes of spec (may evolve further in this PR) following ideas from Clean-up or legalise: revise status_set() values vs. the NUT standard dictionary #2708;
  • refactoring of dstate to generalize what was happening in status_set and friends (might perhaps become a separate PR changeset)
  • introduction of invmode* methods and data (naming may evolve further in this PR);
  • changes of clients, drivers and tests for new concepts and methods (may evolve) and possibly for related issues found along the way (there were many PRs spawned by work on Clean-up or legalise: revise status_set() values vs. the NUT standard dictionary #2708, but maybe not everything not strictly on "ECO" topic went that way eventually).

I'll check what can get dumped overboard into agreeable PRs, to make the difficult and contentious PR easier to review :)

[jimklimov]

I agree, just maybe we need to mark when the inverter is working or waiting to work (like in standby) or off?

Maybe, at least that would be a finite amount of states/strings to report. If we know or have good reasons to assume its state.
And the vague extras with marketing buzzwords (what this PR named output.inverter.mode so far) renamed to something else?.. (Maybe even under experimental.* namespace for 2.8.3 release, as we still seem to have no good grasp about the syntax and name - just that it is something we have a way to know from some devices and might be interesting to represent to users).

[jimklimov]

Offloaded some "noise" into #2852 - will try to rebase this PR after merging that one (or at least limit to one merge-in commit of all upstream/master changes that happened after #2697).

[gdt]

I agree, just maybe we need to mark when the inverter is working or waiting to work (like in standby) or off?

Maybe, at least that would be a finite amount of states/strings to report. If we know or have good reasons to assume its state. And the vague extras with marketing buzzwords (what this PR named output.inverter.mode so far) renamed to something else?.. (Maybe even under experimental.* namespace for 2.8.3 release, as we still seem to have no good grasp about the syntax and name - just that it is something we have a way to know from some devices and might be interesting to represent to users).

experimental is a great idea. Let's call it experimental.mode and just dump in the brand:buzzword or whatever. That exposes what can be exposed, in a non-pleasing way, but then we can get some experience without having to answer the "you broke this variable" later.

As for inverter on or off, and other things, I think we can begin to define a unfied semantics, but I think that should start with documentation that has a definition and that maps state from as many implementations as possible, so that we are sure it's meaningful, clean, and captures what it should. Then we can think about variables vs flags. In general I lean to anything that's a detail in a variable, so that flags are as simple as possible. It's easier to ignore a variable you don't know about than deal with unknown or split flags.

"Is the inverter operating" sounds like a great first candidate. I don't think the possibility of coming up with a definition for that should deter us from having an experimental variable that simply has a vendor:buzzword state description.

[gdt]

I'll check what can get dumped overboard into agreeable PRs, to make the difficult and contentious PR easier to review :)

Thanks. Definitely all the merges should at least be easy to rebase out, and I think it's good to have an expectation/practice that PRs are rebased, not only dropping that, but folding in any 'make big change', 'fixup to that big change' commit sequences.

[jimklimov]

@masterwishx : during a weekend revision of changes I wondered if my changes between mge-hid.c and usbhid-ups.c to prefix vendor:mge-hid: to earlier ECO and ESS definitions could impact some logic you added - notably there seemed to have been some string comparisons (is current state normal or something else, etc.) so it got me worried if I missed some part of those equations and broke what worked for you originally?..

Can you please revise the currently proposed code base in this PR about that logic (whether visually or by testing with HW) to make sure no typos snuck in - before it is too late to fix them? :)

[masterwishx]

I will check the code later today I hope.
About checking on hardware, when tryed last time compile master for current version of Unraid 7.0.1 I had some missing packages etc
So was unable to compile until @desertwitch compiled for his plugin, but will try it again maybe today (I'm not at home all day) if not then tomorrow.

[gdt]

So we could make some handling for ups.mode.change vendor:eaton:ESS etc., I suppose.

Given time limits, I think moving anything eco to full experimental label immediately is good, as given the lack of broadly-shared clear semantics, it's a bug for it to have gone in. Medium term, using vendor tokens in a mode set command sounds good. In the glorious future, when all vendors publish clear specifications, we can figure out the common ground, while meeting on neutral territory. That's my hot take :-)

[jimklimov]

FWIW, tinkered a bit with further ideas, about a single command to rule this all, as mostly a skeleton, parked at https://github.com/jimklimov/nut/tree/issue-2708-instcmd

• UPDATE: I'll rebase that branch over current master as of when this PR gets merged, to ease later return to the sandbox

That branch needs the actual dispatching code (via eaton_ups_mode_change which needs to get defined and would call a number of instcmd() internally for those experimental implementations, depending on its own instant command argument) wedged into the mapping table somehow, but I did not have time or TBH interest to investigate that can of worms just yet. Takers welcome though :)

In this design, the currently implemented (and hopefully not broken by this PR) experimental.{ess,eco}mode.* commands would remain in code as they are, but would not get externally exposed (callable via networked protocol) anymore, and would work exactly same way as they do now, to tickle specific USB actions and set certain values into UPS.PowerConverter.Input.[5].Switchable etc.

[masterwishx]

LGTM

[jimklimov]

@masterwishx : Removed mentions of auto_on and experimental.ecomode.start.auto - but they are still in git history, so if you later come back to this idea, the commit ca27e06 can be git revert'ed.

Got any other last thoughts, or can this be merged when CI agrees? :) Did you have a chance to read/check that my changes to vendor:X:Y string tokens did not break your feature?

And thanks again for bearing with me (and others) over so many months, comments, and uncertainty :)

[masterwishx]

Did you have a chance to read/check that my changes to vendor:X:Y string tokens did not break your feature?

Yes it's looks fine.

Removed mentions of auto_on and experimental.ecomode.start.auto - but they are still in git history, so if you later come back to this idea, the commit ca27e06 can be git revert'ed.

Thanks, i beleave I can finish later this draft pr and add also
experimental.online.start.auto (back from eco mode), but can't test it Becouse issues posted before, maybe user that checked eco mode with 9sx can test it...

But all other changes seems all fine to me

[jimklimov]

Great, thanks! Note that THIS PR does include all your commits from your draft #2697 (rebased over later master branch state though), and includes this latest reversible commit to drop one experiment. So if/when you do follow up, that would be another draft PR, hopefully based from the master branch of that time :)

[masterwishx]

This should be fine ,as tested on 9SX ,
Last function needs to 9E model like my to enter ECO mode but cant get to online back then !