-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix for W215_v2 FW1.24 #20
base: master
Are you sure you want to change the base?
Conversation
except: | ||
self.model_name = 'Unknown' | ||
else: | ||
self.model_name = self.SOAPAction("", "ModelName", "") |
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.
Could we change this to
self.model_name = self.SOAPAction(Action="", responseElement="ModelName", params = "")
As suggested in the PR from @clach04. It is much more readable.
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.
#18 is now merged
Thanks for the PR! Your work looks solid but it is really difficult to review a Christmas tree commit. I am guessing you use some auto pep8 plug-in? Would you mind turning it of and resubmit it as two commits (One with the core changes and one with style changes). I have no problem with pep8 commits as it makes the code more readable but it becomes very difficult to see style and function if everything is in a single commit. Then I will be happy to merge your improvements. |
I agree, it would be great to get a pep8 change and then a separate change for W215_v2 FW1.24. I'd recommend two different pull requests. |
Ok, I will do pep8 first and then the fix. |
bump... Commit #25 does the same? |
W215_A2 have different device id mapping than the default, so this fixes the W215_V2 issue.
However, this is still not smart enough to detect the model number and automatically apply the fix.
We need to set use_legacy_protocol to True during SmartPlug init.