which modbus library ?

Hi,
just wondering which is (currently) the definitive modbus implementation.

https://github.com/GivEnergy/givenergy-modbus2 appears the official one, but most references are to https://github.com/dewet22/givenergy-modbus

But then https://github.com/GivEnergy/giv_tcp has a bundled copy in a subdir, and that one has more recent updates, including changes to use crc which, I believe, has become a requirement. Perhaps it's better to use that instead. Is there any reason why it has its own copy of the code, rather than just referencing the other project as a dependency ?

So, being an early adopter some history -

1: Givenergy Modbus (Dewet22) was the original library to interface to the inverter. This was used by GivTCP originally. (Further back was britkats own modbus library but that's irrelevant as that was deprecated once this library came along)

2: Givenergy Modbus 2 (Givenergy) was a fork around Gen 2 release that modified Dewets code above, such that it recognises Gen2 serial numbers. (It used to just fail) At the time Dewet couldn't be contacted or something like that. That was then rolled out into GivTCP around that time so people with Gen2 could use it. (EA & ED Serials) although these were then rolled back into the original Dewet22 library anyway around the same time so was a sort of pointless endeavour in the end once they got in contact with him.

Note: Neither of these 2 libraries support Gen 3 or AIO or Gateways (i.e. FD serials and so on...)

3: The code from 2 above was then modded again when Gen 3 and AIO came along but this time it was done inside GivTCP repository as you've noticed.
It handles CRC check firmware and massages around some of the newer quirks for the AIO & newer/beta firmwares.

4: to compound further, there is a 4th version inside the '-Dev' builds in Britkats Dockerhub repository which now makes the library talk to the EV charger.

As for which one to use, depends what you want it to do. One would assume that option 3 is probably the most all encompassing / safe for most inverters & newer firmwares.
Admittedly it probably wants a merge doing.

CDPUK uses a version of it for their HA integration (Givenergy Local) - can't remember which version exactly they use (perhaps option 2, I see notes about it not working with newer inverters)

Option 4 is probably at your own risk as unless your auditing the code it's ongoing WIP.

Boils down to:

Britkat GivTCP Support (some under testing):

"""Known models of inverters."""
AC = 'AC'
Hybrid = 'Hybrid'
EMS = 'EMS'
Gateway = 'Gateway'
AllinOne = "All in One"

__dtc_to_models_lut__ = {
    2: Hybrid,
    3: AC,
    4: Hybrid,
    5: EMS,
    6: AC,
    7: Gateway,
    8: AllinOne,
}

Givenergy_Modbus & Givenergy Modbus2

            'CE': cls.AC,                  (AC3)
            'ED': cls.Gen2,                (Gen2)
            'EA': cls.Gen2,                (Gen2)
            'SA': cls.Hybrid,              (Gen1)
            'SD': cls.Hybrid,              (Gen1)

Thanks for all that... since I'll be on Gen 3, I'll probably go with that option. (I don't anticipate getting the GE charger, since Intelligent Go support is probably more valuable at the moment. And I'd prefer untethered at the moment.)

Britkat's fork is quite a way ahead of the original givenergy one. (I'm happy enough to use a tree under active development, and may be able to contribute back.)

Perhaps one for @Rubikcube (if you can be bothered) : what is the correct unit for input registers 8/9 (PV string current).

The modbus code says

    V_PV1 = (1, {'scaling': Scaling.DECI, 'unit': Unit.VOLTAGE_V})
    V_PV2 = (2, {'scaling': Scaling.DECI, 'unit': Unit.VOLTAGE_V})
    I_PV1 = (8, {'scaling': Scaling.CENTI, 'unit': Unit.CURRENT_A})
    I_PV2 = (9, {'scaling': Scaling.CENTI, 'unit': Unit.CURRENT_A})

Which says divide the raw register by 100 to get the given units. But I get

IR:001                               V_PV1: 243.00 V              |  UINT16           DECI   0x097e        2430
IR:002                               V_PV2: 237.50 V              |  UINT16           DECI   0x0947        2375
IR:008                               I_PV1: 0.05 A                |  UINT16           CENTI  0x0005           5
IR:009                               I_PV2: 0.05 A                |  UINT16           CENTI  0x0005           5

which says it has turned a value of 5 into 0.05A. But both your app and the data recorded on the portal say 0.5A. So I assume it should have been labelled as DECI (10ths of amps). (I included the voltage ones for context: they are scaled as DECI, and it does divide the raw register by 10 to get the correct result in volts.)

If I'm reading the code correctly, giv_tcp seems to adjusting it (in read.py):

        power_output['PV_Voltage_String_1'] = GEInv.v_pv1
        power_output['PV_Voltage_String_2'] = GEInv.v_pv2
        power_output['PV_Current_String_1'] = GEInv.i_pv1*10
        power_output['PV_Current_String_2'] = GEInv.i_pv2*10

which seems to confirm that givenergy_modbus is misreporting the current.

DD it should have been labelled as DECI

You are correct, it's an error in the library. Both voltage and current are to 1 decimal place (i.e. 100mV / 100mA resolution).

Rubikcube I think register 10, I_AC should also be DECI, assuming it's the AC output current.

I_PV1 = (8, {'scaling': Scaling.DECI, 'unit': Unit.CURRENT_A})
I_PV2 = (9, {'scaling': Scaling.DECI, 'unit': Unit.CURRENT_A})
I_AC1 = (10, {'scaling': Scaling.CENTI, 'unit': Unit.CURRENT_A})
 
IR:008                               I_PV1: 9.30 A                |  UINT16           DECI   0x005d          93
IR:009                               I_PV2: 9.30 A                |  UINT16           DECI   0x005d          93
IR:010                               I_AC1: 1.81 A                |  UINT16           CENTI  0x00b5         181

(I'm currently exporting around 4kW, which would be 18A)