New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add textual representation entities for Fronius status codes #94155
Conversation
Hey there @nielstron, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
In case this gets adopted and there is a commitment to try this with the error codes too - these are part of the user manual. I assume they would be quite universal within the Fronius Ecosphere, at least for all "Symo" variants. Not all error codes may apply for all devices. If you do not have a hybrid model, "battery" errors do not apply. I have a different model than the listed document, but at least I successfully could match a handful of error numbers with this document. 306, 307 are to expected, 461 may happen occasionally. I was not able not verify this in detail however. Fortunately most of those errors are not that common. 😬 https://www.fronius.com/~/downloads/Solar%20Energy/Operating%20Instructions/42,0426,0222,DE.pdf I cannot help with the coding, but I could help with the boring stuff like translating the PDF-doc into a suitable table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest on having a look at handling invalid error code responses in the same way.
Not exposing the complexity and simplify use through a generic function is probably helpful for future expandability (should there be a need/thought to do so). Not crucial in my opinion though.
9f64eb3
to
328283e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. For those edge cases not covered by the test, they could be added later as well. Looks in working order.
670e943
to
6f971bd
Compare
6f971bd
to
77b9839
Compare
this makes it possible to have multiple entities for a single API response field
eg. to be able to map a API response value to a different value (status_code -> message)
None is handled before
e34dba5
to
81448ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @farmio 👍
Proposed change
Refactorings to make it possible to have multiple entities for a single API response field:
New entities:
staus_message
is a textual, translatable representation of "status_code" integermeter_location_description
is a textual, translatable representation of "meter_location" integerChanged features:
status_message
entity state is now translatable (this was previously hardcoded english in the libpyfronius
)Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: