britkat1980 / giv_tcp

TCP connection (from inverter) and MQTT implementation
71 stars 32 forks source link

cli.py dump_registers missing required argument isAIO #131

Open divenal opened 8 months ago

divenal commented 8 months ago

dump_registers calls refresh_plant(), but that has had a new argument isAIO added to it. I assume the fix would be to add a new cmd-line flag --aio via @click ?

Incidentally, the client stuff seems to depend on old versions of pydactic and pymodbus (?) : they've had incompatible api changes in the latest versions, so I had to force a downgrade. But I guess you don't want to upgrade to the latest APIs since that could break existing users.

britkat1980 commented 8 months ago

The underlying library is unmaintained so I've had to modify it, and as I don't use the cli.py I haven't looked at it. If you want to propose a change that'd be great

divenal commented 8 months ago

I have a change that seems to work. Just need to figure out the security so I can actually push it up to github.

Another minor annoyance is that cli.py assumes that imports are relative to givenergy_modbus, whereas everything else fully-qualifies the imports. But I'm just using cli.py as a check that givenergy_modbus is mostly working. (The only maintained/developed version of this modbus module seems to be in this repository...)

hoggyhoggy commented 3 months ago

To add to this I currently use the CLI from the "old" Dewet library (with some mods of my own) - this works with my "non fast response/old firmware Hybrid" fine however, in an attempt to keep it updated I did try this newer Givenergy_Modbus library found internal to GivTCP. It doesn't appear to function at all with older firmware (no matter what switch is used, i.e. none, --ac / --aio) on anything, including "Dump Registers", so before I start picking it apart I'm curious as to how GivTCP handles this?

Does it (GivTCP) just default to the dewet library first unless the "newer firmware / AIO" toggle is selected? Therefore the internal library only functions by design for newer firmware?