Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ecomode_addon-2 #2684

Merged
merged 40 commits into from
Dec 4, 2024
Merged

Conversation

masterwishx
Copy link
Contributor

@masterwishx masterwishx commented Nov 18, 2024

add-on for #2637,#2680 ECO/Bypass for Eaton Models

@jimklimov In this new pr we can:

You think we can add ESS stuff also, however
Only enterprise UPSes have it?

  • [ Done ] We can add also post limits not passed in debug when check range eco/bypass func.

  • [ Done ] added variables:

input.bypass.switchable
input.transfer.bypass.overload
input.transfer.bypass.outlimits
  • [ Done ] ESS added to usbhid-ups.c

  • [ Done ] Refactoring : eaton_input_bypass_check_range() , eaton_input_eco_check_range()

CC @jimklimov please confirm if it needed .

Also for now can't leave eco mode by commands but maybe Becouse my ussue with timeout, not sure...

Signed-off-by: DaRK AnGeL <[email protected]>
Signed-off-by: DaRK AnGeL <[email protected]>
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@masterwishx
Copy link
Contributor Author

@arnaudquette-eaton Very sorry to bother you again ,maybe you can find info about:

UPS.PowerConverter.Input.[2].Switchable

@arnaudquette-eaton
Copy link
Contributor

no need to be sorry @masterwishx , I'm always happy to help ;)

Desc: Item of Bypass AC Input collection
Transfer on automatic Bypass enabled

RW

0: The automatic transfer on bypass is not allowed
1: The automatic transfer on bypass is allowed. (Refer to UPS.PowerConverter.Input.[2].OutOfToleranceTransferEnable and UPS.PowerConverter.Input.[2].OverloadTransferEnable for additional bypass rules)

Hope it helps

@masterwishx
Copy link
Contributor Author

masterwishx commented Nov 19, 2024

Transfer on automatic Bypass enabled

Thanks a lot , so its like UPS.PowerConverter.Input.[2].ForcedTransferEnable but like for UPS itself bypass autoswitching with rules?

as you know we now have ECO/Bypass enabled with your help for HID info!

the only problem i cant exit from ECO to bypass by r/w values or commands bypass.on/off , but maybe its local issue for my UPS with timeouts ....

Also will try to add function for auto enter to ECO mode ( bypass.on + ECO + bypass.off) but still cant test exit from ECO :(
Thanks again for your help

@masterwishx
Copy link
Contributor Author

Transfer on automatic Bypass enabled

added values if you think they usefull ?

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@masterwishx
Copy link
Contributor Author

masterwishx commented Nov 20, 2024

CC @arnaudquette-eaton can you please check and confirm that we can exit from ECO mode by :
"UPS.PowerConverter.Input.[2].SwitchOffControl"=1 input.bypass.switch.off or
"UPS.PowerConverter.Input.[2].SwitchOnControl"=1 input.bypass.switch.on
or maybe some other command ? (cant exit from ECO by this command, but maybe its my local issue ?!?)

Signed-off-by: DaRK AnGeL <[email protected]>
Signed-off-by: DaRK AnGeL <[email protected]>
Signed-off-by: DaRK AnGeL <[email protected]>
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@masterwishx
Copy link
Contributor Author

masterwishx commented Nov 30, 2024

@arnaudquette-eaton @jimklimov @desertwitch it seems if we return NULL from function its just not return value (not to get the value published) but proceeding the command or r/w variable or not?

return NULL; /* Do not enter bypass mode */

@jimklimov
Copy link
Member

 it seems if we return NULL from function its just not return value but proccess the command or r/w variable ?

Can't say quickly and reliably; please check the caller logic in usbhid-ups.c.

@masterwishx
Copy link
Contributor Author

it seems if we return NULL from function its just not return value but proccess the command or r/w variable ?

Can't say quickly and reliably; please check the caller logic in usbhid-ups.c.

I will try to check , asked becouse user tested ecomode in #2685 was need not able to enter to eco becouse of return NULL in case of no vars of eco limits ...

nut/drivers/mge-hid.c

Lines 711 to 733 in c83ebd4

/* Get the Eco mode voltage/frequency and transfer points */
const char* bypass_voltage_str = dstate_getinfo("input.bypass.voltage");
const char* eco_low_str = dstate_getinfo("input.transfer.eco.low");
const char* eco_high_str = dstate_getinfo("input.transfer.eco.high");
const char* out_nominal_str = dstate_getinfo("output.voltage.nominal");
const char* out_nominal_frequency_str = dstate_getinfo("output.frequency.nominal");
const char* frequency_range_str = dstate_getinfo("input.transfer.frequency.eco.range");
const char* bypass_frequency_str = dstate_getinfo("input.bypass.frequency");
NUT_UNUSED_VARIABLE(value);
if (eco_low_str == NULL || eco_high_str == NULL
|| bypass_voltage_str == NULL || bypass_frequency_str == NULL
|| out_nominal_str == NULL || out_nominal_frequency_str == NULL
|| frequency_range_str == NULL
) {
upsdebugx(1, "Failed to get values: %s, %s, %s, %s, %s, %s, %s",
eco_low_str, eco_high_str,
bypass_voltage_str, bypass_frequency_str,
out_nominal_str, out_nominal_frequency_str,
frequency_range_str);
return NULL; /* Handle the error appropriately */
}

@masterwishx
Copy link
Contributor Author

it seems if we return NULL from function its just not return value but proccess the command or r/w variable ?

Can't say quickly and reliably; please check the caller logic in usbhid-ups.c.

should be fixed in last commit

@masterwishx
Copy link
Contributor Author

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

@jimklimov jimklimov added the ready / gonna merge The PR is in final cycles leading to merge unless someone logs an objection before we hit the button label Dec 2, 2024
@jimklimov
Copy link
Member

Seems reasonable code-wise, applied a bit of cleaning and waiting for CI to agree :)
Also a good moment for @arnaudquette-eaton to chime in as an expert on subject matter :-)

@masterwishx
Copy link
Contributor Author

@jimklimov can be merged?
Do you want to change essmode to ECO in this or in next pr?

@jimklimov
Copy link
Member

I guess it would now be pending the discussion results in #2708, whether to remove status_set("ESS") (and perhaps "ECO" too), in favor of changing upsmon notifications. That does begin to sound more and more like the reasonable way forward.

The rest seems good though, so can merge it anyway and take care of removal and changes in a next PR.

@jimklimov jimklimov merged commit 10ad271 into networkupstools:master Dec 4, 2024
20 of 26 checks passed
@masterwishx
Copy link
Contributor Author

in favor of changing upsmon notifications. That does begin to sound more and more like the reasonable way forward.

OK. My main task was to add and enable the missing Bypass/ECO modes for Eaton usb driver.
The rest of the improvements with deep respect hope for your experience :)

@masterwishx masterwishx deleted the ecomode-addon-2 branch December 4, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Eaton ECO/ESS/HE/ABM modes Non-trivial power supply management modes (ECO, ESS, HE, ABM etc.) enhancement ready / gonna merge The PR is in final cycles leading to merge unless someone logs an objection before we hit the button USB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants