V1.1.11 Home Assistant Discovery, EG4 Protocol Definition, and other small misc Improvements#123
Conversation
- Switch to device discovery over sensor discovery - Add ability to add additional discovery options
- Switch to device discovery over sensor discovery - Add ability to add additional discovery options
Fixed minor issues with protocol verification not applying unit mod leading to extraneous failures. Also some corrections to EG4_v58 registers to reflect Flexboss21 ranges.
|
This should address #121 with "Protocol write validation now accounts for unit mod" |
| ,,117,PtoUserStartchg,1W,- -50W,W,Ptouser: starts AC charging less than this value,,,, | ||
| ,,118,VbatStartDerating,0.1V,>CutVoltForDisc hg+2V,W,For lead-acid battery,,,, | ||
| ,short,119,wCT_PowerOffset,1W,-1000~1000,W,"signed short int; CT Power compensation, PtoUser direction is positive.",,,, | ||
| ,short,119,wCT_PowerOffset,1W,- 1000W~1000W,W,"signed short int; CT Power compensation, PtoUser direction is positive.",,,, |
There was a problem hiding this comment.
i'm not sure if "- 1000W~1000W" is correct.
"-1000~1000"
might work better
There was a problem hiding this comment.
Yeah this was something I was trying to get work to help with the protocol validation because it wasn't handling the -1000 as a negative and the allowed range just ended up being 1000 to 1000 instead of -1000 to 1000.
Adding a W makes PPG treat it as ASCII and it ends up just bypassing the check.
Not intentional to leave that in so it could get switched back to how it was. I might also spend a bit of time to get that parsing work correctly.
There was a problem hiding this comment.
df7be15
should allow -1000~1000 ( untested )
variable types already ints, so should work i think...
but, something like -1000~-250 is a problem... for now.
| ,,147,Battery capacity,Ah,0-10000,W,Battery capacity,,,, | ||
| ,,148,Battery nominal Voltage,0.1V,400-590,W,Battery rating voltage,,,, | ||
| ,,149,EqualizationVolt,,500-590,W,Battery equalization voltage,,,, | ||
| ,,148,Battery nominal Voltage,0.1V,"0,400-590",W,Battery rating voltage,,,, |
There was a problem hiding this comment.
"0,400-590"
combination of lists & ranges.
not sure if ppg handles this properly.
There was a problem hiding this comment.
ahh, nice, i guess it is already there... hopefully i have the logic done right as well :S
i donno how thorough past me was.
ed6462c
( add neg fix to list ranges )
| pyserial | ||
| python-can | ||
| influxdb | ||
| tomli No newline at end of file |
There was a problem hiding this comment.
i think this should be in requirements-dev.txt?
There was a problem hiding this comment.
I added this so that I could read the pyproject.toml file directly to get the project details for mqtt discovery. See here and here
data = project_details()
origin = {}
origin["name"] = data['project']['name']
origin["sw"] = data['project']['version']
origin["url"] = data['project']['urls']['Homepage']
There's probably a better way to go about doing this.
Looking at the Dockerfile, pyproject.toml doesn't make it into the Docker Image so this method doesn't work for that unless the Dockerfile is updated to include it.
It also probably wouldn't work when installed using pip install python-protocol-gateway either.
Do you have any thoughts on the best way to handle this? Hardcoding this info is an option but then you'd have to update it in 2+ places. importlib.metadata is an option when its installed through pip but a check could maybe be added for when it isn't? Or including the pyproject.toml file in the places it needs to be referenced is also a possibility.
|
very nice. just did a quick light review. i don't fully understand how the "enabled_by_default: false" thing works. i haven't had much time for ppg lately. busy. thanks for the contribution! |
From the Home Assistant Side: From the PPG Side: This then gets overwritten by anything defined in the protocol CSV file in the ha disc column a little later. See here This lets people enable/disable things by default that they want sent over MQTT but not ingested by HA or to set different discovery settings if the default by unit isn't correct or isn't included. In the end the CSV file wins.
No worries. Life gets busy for sure. I had planned on making this PR a few weeks ago but got delayed by various things.
Of course! I'm excited to be able to use this with my inverter and Gridboss and these changes are going to make it easier to do. |
Co-authored-by: HotNoob <ewbobman@gmail.com>

Home Assistant Discovery
EG4 Protocol Definition
Other Misc Stuff
I think that's all but I might have forgotten something.