bohdan-s / SunGather

GNU General Public License v3.0
148 stars 61 forks source link

Mapping for running_state #53

Closed michbeck100 closed 2 years ago

michbeck100 commented 2 years ago

I started using SunGather and I'm thinking to move from modbus4mqtt. Unfortunately it seems that SunGather doesn't support a mapping for running_state. According to the sungrow documentation this is the mapping: image So running_state actually contains 8 values.

How can I achieve such a mapping?

danielHin commented 2 years ago

Hi, the merge introduced a bug.

Traceback (most recent call last): File "sungather.py", line 584, in <module> main() File "sungather.py", line 552, in main export.publish(inverter) File "/src/github/SunGather/SunGather/exports/console.py", line 26, in publish print("| {:<7} | {:<35} | {:<20} |".format(str(inverter.getRegisterAddress(register)), str(register), str(value) + " " + str(inverter.getRegisterUnit(register)))) File "sungather.py", line 265, in getRegisterAddress return register['address'] KeyError: 'address'

it happens when reading register running_state

michbeck100 commented 2 years ago

No it happens when reading the address of run_state, which is a custom register without address. It used to have a "virtual" address, but commit bc8493fee93e3c3488930ccc0660a5deef1a455d changed this. This is not because of my changes. @bohdan-s why did you remove the address?

michbeck100 commented 2 years ago

this fixes it again: https://github.com/bohdan-s/SunGather/pull/57

danielHin commented 2 years ago

thy for fixing.

@michbeck100 @bohdan-s another issue I've just saw. The newly introduced 'import_from_grid' register in the registers file clashes with the custom 'import_from_grid' register and overwrites the value.

Maybe we rename the running state registers by prefix them with state_ or runningstate ?

Alternatively we could rename import_from_grid and export_togrid to custom*.

But its more obvious at lest to me to rename the new running state registers to make it more clear

danielHin commented 2 years ago

proposed renaming in #59

michbeck100 commented 2 years ago

I think this can be closed now