-
Notifications
You must be signed in to change notification settings - Fork 62
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
Refactor discovery #145
Refactor discovery #145
Conversation
420eeb5
to
4c44063
Compare
Nice! |
looks good to me! nice work mate. |
@@ -15,28 +16,20 @@ class X1HybridGen4(Inverter): | |||
"sn", | |||
): str, | |||
vol.Required("ver"): str, | |||
vol.Required("Data"): vol.Schema( | |||
vol.Required("data"): vol.Schema( |
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.
It is Data
, not data
in JSON. Just like Information
below. Why did you change it?
UPD: Hm... it does seem to work regardless though.
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.
Because I lowercased all keys in a previous PR.
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 accedentilly hit "Comment" in the mobile app...
I meant to write commit
not PR
, but both are wrong, it was in the same commit.
Anyway, I assume #161 is why you decided to comment.
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.
Yes, I was going through recent changes to see if something broke, but then remembered about firmware upgrade and found the root cause
First commit:
sys.version_info < (3, 10)
checks to be used in the code base.Second commit:
solax.discover()
solax.discover()
can also return a list of all inverter models/classes instead of returning the fist that succeeded.Which can be used by Home Assistant to have the user resolve such situations.
Third commit:
Measurement
, but that can easily be added if desired.Fourth commit:
@squishykid It took me longer to finish than expected. Hope you enjoyed your holiday!
PS. I assume you are the one who started following me on Trakt :p