Serial number cannot be decoded on a 5.0 Gen 2 inverter

[cdpuk]

• GivEnergy Modbus version: 0.10.1
• Python version: 3.9, 3.10

Description

I am using the library as part of a Home Assistant integration. I've been working with someone who has a 5.0 Gen 2 inverter, and is unable to get my integration working.

The root cause appears to be in the serial number to model decoding function.

What I Did

The following script has been used to debug the issue: https://github.com/cdpuk/givenergy-local/blob/master/scripts/debug.py

We can successfully read the registers and print details with plant.inverter_rc, however decoding the values fails as the registers that would normally hold the serial number seem to hold other data.

Traceback (most recent call last):
  File "U:\debug.py", line 68, in <module>
    main()
  File "U:\debug.py", line 64, in main
    debugger.display_decoded_data()
  File "U:\debug.py", line 45, in display_decoded_data
    print(self.plant.inverter.json(indent=2))
  File "C:\Users\xxx\AppData\Local\Programs\Python\Python310\lib\site-packages\givenergy_modbus\model\plant.py", line 32, in inverter
    return Inverter.from_orm(self.inverter_rc)
  File "pydantic\main.py", line 562, in pydantic.main.BaseModel.from_orm
  File "pydantic\main.py", line 1048, in pydantic.main.validate_model
  File "C:\Users\xxx\AppData\Local\Programs\Python\Python310\lib\site-packages\givenergy_modbus\model\inverter.py", line 212, in compute_model
    values['inverter_model'] = Model.from_serial_number(values['inverter_serial_number'])
  File "C:\Users\xxx\AppData\Local\Programs\Python\Python310\lib\site-packages\givenergy_modbus\model\inverter.py", line 39, in from_serial_number
    raise UnknownModelError(f"Cannot determine model number from serial number {serial_number}")
givenergy_modbus.model.inverter.UnknownModelError: Cannot determine model number from serial number  D 1 1 0 2

Patching the library such that it returns an "Unknown" model when decoding fails gets everything working. All other decoded values appear correctly. I was unable to identify another set of registers that hold the serial number in the expected format.

I have the complete output from this script, including all register values, if required.

The "Unknown" approach certainly seems most straightforward, but I appreciate there may be other reasons to raise instead.

It's worth noting that the log messages contain the correct serial number (from the underlying modbus library?). Perhaps this would be a more reliable source for the serial number property.

givenergy_modbus INFO Decoded response 4/ReadInputRegistersResponse({check: 0xe55a, inverter_serial_number: EDnnnnnnnn, base_register: 0x0000, register_count: 0x003c, data_adapter_serial_number: WGnnnnnnnn, padding: 0x008a, slave_address: 0x0032})

[dewet22]

Fascinating, I've not seen that before. So the reason the log lines are correct is because the protocol contains the inverter serial twice 🤦 . Once as part of the standard header for reading back any set of registers, but then also as part of InputRegisters when you read back locations 13-17. It is the latter that seems to be failing, which would be quite interesting to debug. It should already be identifying serials starting with ED as Gen2, so throwing an exception here is actually good – clearly there's something funky with the register decoding going on based on the exception you pasted.

If you'd be up for it, supplying the raw RegisterCache would be useful for me to try and debug it – you should be able to do .inverter_rc.to_json() and pass that back to me. GitHub unfortunately doesn't seem to support private/confidential issues or comments so not sure what method you'd be most comfortable with to share it.

[dewet22]

FYI, the dump-registers command in the CLI does something similar that I've used before to help others debug their setup.

Also, just so you're aware, I'm massively refactoring the whole thing right now – the current synchronous client is quite flaky because it is quite sensitive to timing on messages incoming and misses a lot of updates as a result. I'm nearing completion of an asyncio-based version which has been super stable so far (querying real-time values every 5s) and will be part of a native Home Assistant custom component I've been working on at the same time.

[tanathka]

It's my inverter thats giving these errors. I've contacted you via email.

[dewet22]

This is a hardware issue from as much as I can see – I added some workarounds which should handle this more gracefully in future.