Fix command-line tool solar power call, possibly use native power readings, add battery power.

[hrford]

1. The "solar power" line shows the solar current but should be:
   print(f"Solar power: {controller.get_solar_power()}W")
2. Is there any reason why the library makes two calls, one for voltage and one for current, when the power value is available as one call to the device? (Edit: Actually, it's two calls, or maybe even one call 0x10 for H & L, but I guess would be more accurate as the values for V and I would be calculated at the same time!?)
3. I'd like to add a power call for the battery (0x3106 and 0x3107), as there's already functions for solar (0x3102/3) and load (0x310E/F) power.
4. And a question about battery current. In another library, they use 0x3105 for current, but you use 0x331B/C. I'll try to get my head around this and see if we can make a two-register call using func 0x10.

This issue is for points 1 to 3

[rosswarren]

Hey, thanks for raising the issue.

1. this was a mistake, good spot.
2. After reading your comment, I think I had misinterpreted the documentation. I had thought that H and L meant high and low as in a minimum and maximum value received. But actually perhaps it is high and low bits of a single value. In which case we can use the function read_long from minimalmodbus to read a single value across both registers. It would make a lot more sense that way.
   In which case it makes no sense for the library to calculate the power based on voltage and current. Sorry about the confusion there.
3. We will need to fix the H/L registers here to read as single values. Then it would be great to have the battery power call available.
4. 0x3005 is rated current, so this is just informational about the max current it can support rather than a real time data value.

I am happy to work on these fixes in a couple of days or otherwise would also be grateful for any PRs.

Just as an example (would need to test further) I think this code

    def get_solar_power_l(self):
        """PV array input power L"""
        return self.retriable_read_register(0x3102, 2, 4)

    def get_solar_power_h(self):
        """PV array input power H"""
        return self.retriable_read_register(0x3103, 2, 4)

would be replaced by something like:

  def get_solar_power(self):
    """PV array input power"""
    return self.retriable_read_long(0x3102, 4, False, minimalmodbus.BYTEORDER_BIG) / 100

I'm not sure about the byte order due to the L and H

[hrford]

1. :)
2. No worries, not confused, just learned you had a different view! So the values (2-byte unsigned int, mutliplied by 100) can represent 0.00 to 655.36 (Some epever kit goes above 655W!). If we were to read back 2000.00W (x100=0x30D40), then H would contain 0x0003 and L would contain 0x0D40. There is no endianess to speak of as that's handled by minimalmodbus. We should just mutliply H by 0x10000, add it to L and divide by any scaling factor (e.g. 100 for power). Unless you mean endianess in that the L register comes before the H register?
3. Awesome.
4. 0x3005 is rated current. I used 0x3105 for measured current.

[hrford]

I just read the docs at https://minimalmodbus.readthedocs.io/en/stable/apiminimalmodbus.html#minimalmodbus.Instrument.read_long, I didn't see that minimalmodus handles multi-register values! Amazing library.

[rosswarren]

Yeah I meant the ordering of the L and H register! But hopefully it "just works" with the read_long method 👍

I see no mention of 0x3105 in the documentation that I have (see attached). Happy to use the register you mention though if it works.

ControllerProtocolV2.3.pdf

[hrford]

I'll find the manual I used, but it would be nice to understand why there might be two. It might be a legacy reason before they moved them into a space where they could have two registers? No idea yet...

I've just about coded the read_long into a retryable function...

Do you want this issue split out into the separate issues? I combined them for discussion :)

[rosswarren]

Makes sense for that one to be a separate issue yeah!

[hrford]

ok, I've coded points 1 to 3, just need the sun to be here to start testing it tomorrow. Expect a PR by Monday :D #promises

[hrford]

https://www.developpez.net/forums/attachments/p196506d1451307310/systemes/autres-systemes/automation/probleme-com-modbus-pl7-pro/controllerprotocolv2.3.pdf/ V2.3 (2015) shows:

• an example real-time battery voltage at 0x3104 but then doesn't define it again in any table
• measured battery power at 0x3106 (32 bits)
• 0x311A is measured battery voltage
• 0x311B (32-bits) is measured battery current

Also note:

• an erroneous entry for rated voltage at 0x311D, it defines it as power in degrees C! (It's actually rated voltage)

Then there's V1.1 of the document at http://www.solarpoweredhome.co.uk/1733_modbus_protocol.pdf which shows all the blocks together:

• 0x3100 PV voltage (16b), current (16b), power(32b)
• 0x3104 Battery voltage (16b), current (16b), power(32b)
• 0x310C Load voltage (16b), current (16b), power(32b)

So, I'm assuming some of these registers are legacy register which have replacement registers, and some aren't replaced but then aren't mentioned in future versions.

[hrford]

This ticket can be closed