Write battery parameters about epevermodbus HOT 13 CLOSED

Please do raise any issues that you find, don't hold back. I appreciate your efforts to improve the library.
I've raised #8 and #9 and #10

from epevermodbus.

Hi @hrford I'm glad to hear you like the library! The reason I hadn't got further with this feature is that I wasn't sure if the parameters have to be written in a block rather than one by one. Did you find this to be the case in your testing?

I would be happy for you to work on a PR, I would support with review and testing on my controller as well 👍

from epevermodbus.

Thanks!
Literally the only reason I started looking for libraries was to write user-defined values for a LiFePo4 battery (Without Windows!). I succeeded in using the minimalmodbus library directly.
Yes, mutliple-write (func 0x10) is needed. I think this is because the values need to be tested against a set of criteria and it would not be possible to meet that while writing them one-by-one.
write_registers(registeraddress: int, values: List[int]) → None

One question remains, if an API function call took the 12 required values. How would you like to do this?
My preference would be to make them all optional non-positional arguments and if any of them are empty, read the values back from the instrument, update the array and write them all back. I'll demo what I mean in a PR :)
If the criteria aren't met, the Tracer returns an error, which we can throw.
I suspect most people would use the function call with all new values, once, and leave it alone.
Some experimenters might come back and tweak a single value. (I did)

from epevermodbus.

Your suggested API sounds very nice to use, I like it! It might be also good if there was an easy way to preview the initial values before setting anything (by calling the same function with no parameters?).

from epevermodbus.

minimalmodbus allows a read of multiple values (albeit without decimal point shifting). I propose three functions:

• get_battery_voltage_control_registers() returns dict:
• set_battery_voltage_control_registers(discharging_limit_voltage=None, low_voltage_disconnect_voltage=None, ...)
• set_battery_voltage_control_registers_dict(values: dict)

The user could learn the actual settings using get_battery_voltage_control_registers.
If they wanted to set a single value, then they'd use:
set_battery_voltage_control_registers(boost_charging_voltage=14.2)
This function would then read the 12 parameters (as 11 of them aren't given by the user), update the correct one and set all 12.
If the user then breaks one of the criteria, an error will be thrown.

set_battery_voltage_control_registers_dict would be a wrapper for set_battery_voltage_control_registers for users who wanted to pass in the dict obtained from get_battery_voltage_control_registers, otherwise, I expect most users just to use set_battery_voltage_control_registers

The wrapper would be like:
def set_battery_voltage_control_registers_dict(values):
self.set_battery_voltage_control_registers(
discharging_limit_voltage=values.get(discharging_limit_voltage),
....

This would allow an incomplete dict to be used as get returns None when key isn't found.

from epevermodbus.

Battery voltage control registers: {'over_voltage_disconnect_voltage': 14.7, 'charging_limit_voltage': 14.6, 'over_voltage_reconnect_voltages': 14.6, 'equalize_charging_voltage': 14.4, 'boost_charging_voltage': 14.6, 'float_charging_voltage': 13.6, 'boost_reconnect_charging_voltage': 13.3, 'low_voltage_reconnect_voltage': 11.5, 'under_voltage_recover_voltage': 11.6, 'under_voltage_warning_voltage': 11.5, 'low_voltage_disconnect_voltage': 11.0, 'discharging_limit_voltage': 10.5}

from epevermodbus.

@rosswarren
Would you mind testing my branch https://github.com/hrford/epevermodbus/tree/issue-2-write-battery-parameters before any PR?
First time I've used **kwargs too, so much nicer to define the magic 12 battery register names ONCE and then use them where needed.

I have it running locally hence the import being tweaked. Not sure how you went about debugging. Perhaps let me know?

from epevermodbus.

@hrford I really like the way you implemented the feature. Sorry but I can't help test for a few days, as I don't have everything I need here right now.

For debugging what I do is open a python shell in the directory of the code and run

from epevermodbus import EpeverChargeController
controller = EpeverChargeController("/dev/ttyUSB0", 1)
controller.get_solar_voltage()
# etc.

Is that what you meant?

from epevermodbus.

Thanks!
When I run the command line tool, it imports the installed version of EpeverChargeController. So, i find myself editing the import statement while testing it locally.
I understand your approach too, of course you have more control using python shell. (Do you know about ipython?).

from epevermodbus.

Hey @hrford I just tested this functionality with my charge controller and it is working very well. I do have to sometimes call the function multiple times due to an error No communication with the instrument (no answer) but then maybe it doesn't make sense to have a retry on setting something. I tried changing various different settings to different values and it all worked well, and the change also showed up in the EPever Windows software when I checked.

I see what you mean now about the command line tool, I can't get that to work either with the imports other than with the installed version. I am usually testing with only calling functions in the python shell directly. Thanks for the tip about ipython, I haven't used that! I am not normally a Python developer maybe you can tell 😄

from epevermodbus.

Great, thanks for checking, I'll raise a PR and if anyone else finds bugs/improvements, then we can fix it in time.

from epevermodbus.

I'm working again on this project/branch. Will need to work on some of the Signed values to find which calls need it.

I'm not sure about your issue with the comms, I've never had any failures, so perhaps you're experiencing a real hardware issue?

The command line tool is kind of my test harness (I should build another).
I coax it into working by importing without "epevermodbus." prefix on the imports, so it then looks in the current directory.

from epevermodbus.

Tested some more and found a bug in another area... Are you ok if I raise issues more often? Would it be too noisy?

Raised PR https://github.com/rosswarren/epevermodbus/pull/7/files which I couldn't work out how to link with this issue.

from epevermodbus.