EATON HID: add missing usages and paths (addon) - ABM Statuses #2660

masterwishx · 2024-10-16T09:59:48Z

ABM Statuses + Adding Missing usages and paths

See also: https://www.eaton.com/content/dam/eaton/products/backup-power-ups-surge-it-power-distribution/backup-power-ups/power-xpert-9395/Eaton-ABM-White-Paper-AP162001EN.pdf

Continue work/addon for #2637

Added:

Variables: 

battery.charger.status: discharging
battery.charger.type: ABM
etc ...

Added ABM statuses for Eaton Models with next paths:

UPS.BatterySystem.Charger.ChargerType
UPS.BatterySystem.Charger.Status

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

masterwishx · 2024-10-16T13:16:48Z

@arnaudquette-eaton is last commits OK for adding status ? UPS.BatterySystem.Charger.Status

arnaudquette-eaton · 2024-10-16T13:37:58Z

@arnaudquette-eaton is last commits OK for adding status ? UPS.BatterySystem.Charger.Status

seems good, though

a bit redundant (or overlapping) with UPS.BatterySystem.Charger.Mode
you may not limit to charging / discharging, and may expand words instead of using the ups.status "flag" mode

masterwishx · 2024-10-16T18:01:20Z

@arnaudquette-eaton is last commits OK for adding status ? UPS.BatterySystem.Charger.Status

seems good, though

a bit redundant (or overlapping) with UPS.BatterySystem.Charger.Mode

you may not limit to charging / discharging, and may expand words instead of using the ups.status "flag" mode

Thanks, if all fine from statuses perspective, I will try to combine them in one function with if check, as I think important not change the code logic if possible...

….status for bath in one functions Signed-off-by: DaRK AnGeL <28630321+masterwishx@users.noreply.github.com>

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

masterwishx · 2024-10-16T19:42:19Z

a bit redundant (or overlapping) with UPS.BatterySystem.Charger.Mode

Please look at 2 last commits combined to one functions, should look better now.

@jimklimov can we merge it also or to merge it in eco branch?

jimklimov · 2024-10-16T19:53:32Z

can we merge it also or to merge it in eco branch?

I think two PRs for two goals are okay. Does this one fully include the commits of the "eco branch"? If yes, merging this one will automatically close that PR as achieved in one shot.

masterwishx · 2024-10-16T19:59:27Z

can we merge it also or to merge it in eco branch?

I think two PRs for two goals are okay. Does this one fully include the commits of the "eco branch"? If yes, merging this one will automatically close that PR as achieved in one shot.

Yes it is

masterwishx · 2024-10-16T20:00:42Z

can we merge it also or to merge it in eco branch?

I think two PRs for two goals are okay. Does this one fully include the commits of the "eco branch"? If yes, merging this one will automatically close that PR as achieved in one shot.

but not include conversations ? Sorry still not good enough with git :(

jimklimov · 2024-10-16T20:06:55Z

Not sure I fully understand the question. Conversations will remain in github (if they are not part of Git committed code, e.g. new comments about logic), in respective issue/PR discussions. For easier history research later, I'll add a cross-link between these.

masterwishx · 2024-10-16T20:13:10Z

Member

Sorry , i meaned converation like here is 8 only , and in #2637 was 110 and also comments of the code .
but its OK if you will add links to that pr

jimklimov · 2024-10-16T20:20:46Z

drivers/mge-hid.c

 	/* ABM (Advanced Battery Monitoring) processing
 	 * Must be processed before the BOOL status */
 	/* Not published, just to store in internal var. advanced_battery_monitoring */
-	{ "battery.charger.abm.status", 0, 0, "UPS.BatterySystem.Charger.ABMEnable", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_enabled_info },
-	{ "battery.charger.abm.status", 0, 0, "UPS.BatterySystem.Charger.Status", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_status_enabled_info },
+	{ "battery.charger.status", 0, 0, "UPS.BatterySystem.Charger.ABMEnable", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_enabled_info },
+	{ "battery.charger.status", 0, 0, "UPS.BatterySystem.Charger.Status", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_enabled_info },
 	/* Same as the one above, but for legacy units */
 	/* Refer to Note 1 (This point will need more clarification!)
 	{ "battery.charger.status", 0, 0, "UPS.BatterySystem.Charger.PresentStatus.Used", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_enabled_legacy_info }, */
 	/* This data is the actual ABM status information */
-	{ "battery.charger.mode", 0, 0, "UPS.BatterySystem.Charger.Mode", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_mode_info }, /* needs both ? from https://github.com/networkupstools/nut/pull/2637#discussion_r1772730590 */
-	{ "battery.charger.status", 0, 0, "UPS.BatterySystem.Charger.Status", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_status_info }, /* checked on Eaton 9E Model */
+	{ "battery.charger.mode", 0, 0, "UPS.BatterySystem.Charger.Mode", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_info }, /* needs both ? from https://github.com/networkupstools/nut/pull/2637#discussion_r1772730590 */
+	{ "battery.charger.status", 0, 0, "UPS.BatterySystem.Charger.Status", NULL, "%.0f", HU_FLAG_QUICK_POLL, eaton_abm_info }, /* checked on Eaton 9E Model */


@arnaudquette-eaton : I am offline today and feel a bit vague on how the mapping tables are processed.

From this commit, it seems we can set battery.charger.status from several sources above (once from the same UPS.BatterySystem.Charger.Status source, but with eaton_abm_enabled_info instead of eaton_abm_info)... Would this line even work (or would the loop finish at first hit... and what counts as a hit - seeing a reading from the source, or specifically a non-NULL return (so both referenced functions have a chance to run even this way)?

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

AppVeyorBot · 2024-10-16T23:19:13Z

❌ Build nut 2.8.2.2256-master failed (commit d7d9767a3e by @masterwishx)

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

desertwitch · 2024-11-12T13:06:30Z

drivers/mge-hid.c

 * Synthesis table
 * HID data                                   |             Charger in ABM mode             | Charger in Constant mode | Error | Good |
 * UPS.BatterySystem.Charger.ABMEnable        |                      1                      |            0             |       |      |
+ * UPS.BatterySystem.Charger.ChargerType      |                      4                      |            5             |       |      |


Just for context: I removed this in my PR because it suggests that ABM is enabled when the charger type is ABM-capable (4), as opposed to ABM-incapable (5). In reality ABM enabled/disabled seems to be controlled only through UPS.BatterySystem.Charger.ABMEnable and not UPS.BatterySystem.Charger.ChargerType. My logic is that an ABM-capable charger type (4) could still have ABM disabled (by the user), so I felt that putting UPS.BatterySystem.Charger.ChargerType here with "Charger in ABM mode" is misleading as it's likely merely ABM-capable and not in fact explicitly in ABM mode (enabled). In general UPS.BatterySystem.Charger.ChargerType seems to be only the textual representation of the charger capabilities, and not the ABM state itself?

masterwishx · 2024-11-12T13:09:54Z

Path: UPS.PowerSummary.PresentStatus.Charging, Type: Feature, ReportID: 0x01, Offset: 16, Size: 8, Value: 1

if you will have time , still interesting if your ups in ABM disabled mode will always send x.Charging =1 even in 100% charged ?
that will OL CHRG.
and when on battery x.Discharging =1 that will OB DISCHRG ...
Thanks

desertwitch · 2024-11-12T13:37:47Z

Path: UPS.PowerSummary.PresentStatus.Charging, Type: Feature, ReportID: 0x01, Offset: 16, Size: 8, Value: 1

if you will have time , still interesting if your ups in ABM disabled mode will always send x.Charging =1 even in 100% charged ? that will OL CHRG. and when on battery x.Discharging =1 that will OB DISCHRG ... Thanks

5P850I - ABM disabled - OL CHRG:

Path: UPS.BatterySystem.Charger.ABMEnable, Type: Feature, ReportID: 0x29, Offset: 0, Size: 8, Value: 0
Path: UPS.BatterySystem.Charger.ChargerType, Type: Feature, ReportID: 0x26, Offset: 0, Size: 8, Value: 0
Path: UPS.PowerSummary.PresentStatus.Discharging, Type: Feature, ReportID: 0x01, Offset: 24, Size: 8, Value: 0
Path: UPS.PowerSummary.PresentStatus.Charging, Type: Feature, ReportID: 0x01, Offset: 16, Size: 8, Value: 1

5P850I - ABM enabled - OL CHRG (floating):

Path: UPS.BatterySystem.Charger.ABMEnable, Type: Feature, ReportID: 0x29, Offset: 0, Size: 8, Value: 1
Path: UPS.BatterySystem.Charger.ChargerType, Type: Feature, ReportID: 0x26, Offset: 0, Size: 8, Value: 4
Path: UPS.BatterySystem.Charger.Mode, Type: Feature, ReportID: 0x26, Offset: 8, Size: 8, Value: 3
Path: UPS.PowerSummary.PresentStatus.Discharging, Type: Feature, ReportID: 0x01, Offset: 24, Size: 8, Value: 0
Path: UPS.PowerSummary.PresentStatus.Charging, Type: Feature, ReportID: 0x01, Offset: 16, Size: 8, Value: 1

So we can see that in constant charge mode (with ABM disabled), we have OL CHRG (as I expected, always applying charge).

UPS.BatterySystem.Charger.ChargerType seems to change also (against my expectations), but I still think we should not trust it for ABM enabled/disabled deductions and let only UPS.BatterySystem.Charger.ABMEnable decide here. Because it didn't stay either the same or switch to 5 "Constant Charge (CC)", as we maybe would've expected it to, but rather switched to 0 "None". So I think we could keep this as a textual representation of the charger type in the variables, but without any special significance for ABM enabled-ness (so I'd keep it as it is in the code, and not let UPS.BatterySystem.Charger.ChargerType decide on anything).

masterwishx · 2024-11-12T14:25:35Z

and not let UPS.BatterySystem.Charger.ChargerType decide on anything

Thanks a lot, we will not let decide its just for the info of variable in table:
battery.charger.type Constant Charge (CC)

From copilot:

Charging Behavior:
The Eaton 9E UPS maintains constant charging even when the battery is at 100%.
This behavior is typical for online double-conversion UPS systems.
The UPS continuously charges the battery to ensure it’s ready for use during power outages.
Status Indicators:
When the UPS is operating normally (on utility power), you’ll see the status as “OL CHRG” (Online Charging).
When the UPS switches to battery power (during a power outage), you’ll see “OB DISCHRG” (On Battery Discharge).

masterwishx · 2024-11-12T14:29:35Z

Because it didn't stay either the same or switch to 5 "Constant Charge (CC)", as we maybe would've expected it to, but rather switched to 0 "None".

Yeah i see , but maybe in other Eaton models with ABM it switch to CC ?

masterwishx · 2024-11-12T14:32:50Z

5P850I - ABM enabled - OL CHRG (floating):

battery.charge != 100 ?

desertwitch · 2024-11-12T14:32:57Z

Because it didn't stay either the same or switch to 5 "Constant Charge (CC)", as we maybe would've expected it to, but rather switched to 0 "None".

Yeah i see , but maybe in other Eaton models with ABM it switch to CC ?

Maybe, we'll have to wait for more data on this (I think), so I'd keep it only as informational variable (for now).

5P850I - ABM enabled - OL CHRG (floating):

battery.charge != 100 ?

battery.charge = 100 (floating is applying charge so OL CHRG is right here, resting would be OL)

masterwishx · 2024-11-12T14:36:12Z

battery.charge = 100 (floating is applying charge so OL CHRG is right here, resting would be OL)

Thanks got it

masterwishx · 2024-11-13T15:58:20Z

Maybe, we'll have to wait for more data on this (I think), so I'd keep it only as informational variable (for now).

we can leave as is for now or add something like:

nut/drivers/mge-hid.c

Line 1629 in 18e1375

/* Published battery charger type (can be ABM, CC, ...) */

/* Published battery charger type (info variable that can be ABM, CC, ...) */

and:
https://github.com/networkupstools/nut/blob/18e137597a00cde4839f90961d5e1a3bbb6e6f5c/drivers/mge-hid.c#L162C2-L162C136

* UPS.BatterySystem.Charger.ChargerType | 4 | 5 or 0 | | |

but 0 is none in table of variable so not sure

desertwitch · 2024-11-15T11:45:26Z

drivers/mge-hid.c

Not sure what happened here during the merge, but this doesn't seem right. Now we're missing the published UPS.BatterySystem.Charger.ChargerType and have another published UPS.BatterySystem.Charger.Mode entry (but which was already published before)? Can you revert this change? I think it was fine as it was before.

desertwitch · 2024-11-15T11:59:46Z

Maybe @jimklimov can help out here with the merge/rebase so that nothing ABM-related is lost. 🙂

masterwishx · 2024-11-15T12:00:55Z

Maybe @jimklimov can help out here with the merge/rebase so that nothing ABM-related is lost. 🙂

Fixed

masterwishx · 2024-11-15T12:01:13Z

Maybe @jimklimov can help out here with the merge/rebase so that nothing ABM-related is lost. 🙂

Please check now

desertwitch · 2024-11-15T12:04:17Z

Maybe @jimklimov can help out here with the merge/rebase so that nothing ABM-related is lost. 🙂

Please check now

Looks good - thanks for fixing!

masterwishx · 2024-11-18T16:33:07Z

@desertwitch not sure where the error ...

desertwitch · 2024-11-19T06:33:02Z

@desertwitch not sure where the error ...

I can't see any specific error in the logs, although I'm no expert navigating the CI, maybe something unrelated to this PR.

masterwishx · 2024-11-19T06:36:10Z

@desertwitch not sure where the error ...

I can't see any specific error in the logs, although I'm no expert navigating the CI, maybe something unrelated to this PR.

Yes, sorry it's about CI so @jimklimov can help? I saw some new prs for CI

Signed-off-by: desertwitch <24509509+desertwitch@users.noreply.github.com>

desertwitch · 2024-11-19T06:59:47Z

Minor follow up to my code contributions: masterwishx#5 (if possible please merge into this PR)

…tatus+mode drivers/mge-hid.c: reset ABM path when ABM not enabled

masterwishx · 2024-11-20T07:13:03Z

CC @jimklimov please check when you will have time if can be merged

masterwishx added 6 commits October 16, 2024 12:03

add battery.charger.type

84fe0ca

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

some try for add battery.charger.status + battery.charger.mode

d9eaa4b

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

fix ABM Table

eac8c5c

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

add eaton_abm_status_fun()

384d123

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

added battery.charger.abm.status for UPS.BatterySystem.Charger.Status

4dcdc21

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

add eaton_abm_chrg_dischrg_status_fun()

1909210

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

masterwishx added 2 commits October 16, 2024 22:19

put battery.charger.status + battery.charger.mode and battery.charger…

9eb4ec2

….status for bath in one functions Signed-off-by: DaRK AnGeL <28630321+masterwishx@users.noreply.github.com>

same for eaton_abm_chrg_dischrg_info()

9d5f35f

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

jimklimov added enhancement Eaton USB ECO/ESS/HE/ABM modes Non-trivial power supply management modes (ECO, ESS, HE, ABM etc.) labels Oct 16, 2024

jimklimov added this to the 2.8.3 milestone Oct 16, 2024

jimklimov reviewed Oct 16, 2024

View reviewed changes

masterwishx added 6 commits October 16, 2024 23:34

battery.charger.type.status added

c52c9a2

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

fix eaton_abm_enabled_fun()

e435f4d

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

fix declaration

a41b41f

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

int to double

8451971

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

try simple eaton_abm_enabled_fun()

dc45ba2

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

add back missing advanced_battery_monitoring=value;

9f38882

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

add ChargerType to ABM Table

18e1375

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

desertwitch reviewed Nov 12, 2024

View reviewed changes

Merge branch 'master' into work/246-add-battery.charger.status+mode

5c202f7

desertwitch reviewed Nov 15, 2024

View reviewed changes

masterwishx force-pushed the work/246-add-battery.charger.status+mode branch from bbeeea6 to 5c202f7 Compare November 15, 2024 11:50

Merge branch 'master' into work/246-add-battery.charger.status+mode

302ce78

masterwishx changed the title ~~EATON HID: add missing usages and paths (addon)~~ EATON HID: add missing usages and paths (addon) - ABM Statuses Nov 15, 2024

Merge branch 'master' into work/246-add-battery.charger.status+mode

8390436

drivers/mge-hid.c: reset ABM path when ABM not enabled

b65623e

Signed-off-by: desertwitch <24509509+desertwitch@users.noreply.github.com>

Merge pull request #5 from desertwitch/work/246-add-battery.charger.s…

569c31e

…tatus+mode drivers/mge-hid.c: reset ABM path when ABM not enabled

jimklimov merged commit f99109d into networkupstools:master Nov 30, 2024
2 of 3 checks passed

masterwishx deleted the work/246-add-battery.charger.status+mode branch November 30, 2024 17:04

desertwitch mentioned this pull request Dec 1, 2024

drivers/mge-hid.c: comment regarding ChargerType #2704

Merged

Uh oh!

EATON HID: add missing usages and paths (addon) - ABM Statuses #2660

EATON HID: add missing usages and paths (addon) - ABM Statuses #2660

Uh oh!

Conversation

masterwishx commented Oct 16, 2024 • edited by jimklimov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

masterwishx commented Oct 16, 2024

Uh oh!

arnaudquette-eaton commented Oct 16, 2024

Uh oh!

masterwishx commented Oct 16, 2024

Uh oh!

masterwishx commented Oct 16, 2024

Uh oh!

jimklimov commented Oct 16, 2024

Uh oh!

masterwishx commented Oct 16, 2024

Uh oh!

masterwishx commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimklimov commented Oct 16, 2024

Uh oh!

masterwishx commented Oct 16, 2024

Uh oh!

jimklimov Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Oct 16, 2024

Uh oh!

desertwitch Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

masterwishx commented Nov 12, 2024

Uh oh!

desertwitch commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

masterwishx commented Nov 12, 2024

Uh oh!

masterwishx commented Nov 12, 2024

Uh oh!

masterwishx commented Nov 12, 2024

Uh oh!

desertwitch commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

masterwishx commented Nov 12, 2024

Uh oh!

masterwishx commented Nov 13, 2024

Uh oh!

desertwitch Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

desertwitch commented Nov 15, 2024

Uh oh!

masterwishx commented Nov 15, 2024

Uh oh!

masterwishx commented Nov 15, 2024

Uh oh!

desertwitch commented Nov 15, 2024

Uh oh!

masterwishx commented Nov 18, 2024

Uh oh!

desertwitch commented Nov 19, 2024

Uh oh!

masterwishx commented Nov 19, 2024

Uh oh!

desertwitch commented Nov 19, 2024

Uh oh!

masterwishx commented Nov 20, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

masterwishx commented Oct 16, 2024 •

edited by jimklimov

Loading

masterwishx commented Oct 16, 2024 •

edited

Loading

desertwitch Nov 12, 2024 •

edited

Loading

desertwitch commented Nov 12, 2024 •

edited

Loading

desertwitch commented Nov 12, 2024 •

edited

Loading

desertwitch Nov 15, 2024 •

edited

Loading