Conversation
| ) | ||
|
|
||
| @staticmethod | ||
| def ThrowIf(response): |
There was a problem hiding this comment.
Could you adhere to the PEP8 styleguide? https://peps.python.org/pep-0008/#function-and-variable-names. In Python function names should be lower case with words separated by underscores.
There was a problem hiding this comment.
And good to run pre-commit to see if the linter is happy :-).
There was a problem hiding this comment.
Sorry, I thought I got all of these but it seems the linter had crashed somewhere and wasn't giving me the full picture, which a reboot has now solved :-/
There was a problem hiding this comment.
No worries. Still plan to move the formatting and linting to ruff and the project to uv, but will need some time first 😃
| ex = None | ||
|
|
||
| if action_error_desc == XMO_AUTHENTICATION_ERR: | ||
| ex = AuthenticationException(action_error) |
There was a problem hiding this comment.
| ex = AuthenticationException(action_error) | |
| return AuthenticationException(action_error) |
My preference would be to not set the ex variable and just return it directly when raised. For all if statements here.
There was a problem hiding this comment.
Mine too ... I thought I saw "too many return statements" in the linter output and just complied. But I think my setup was skewy for linting so I've changed it back to see if it still complains
There was a problem hiding this comment.
yeah its not happy about R0911 (too-many-returns) now, not sure which way you want to go?
There was a problem hiding this comment.
We can add an ignore for now, or change the linting preferences.
There was a problem hiding this comment.
@iMicknl - any chance you can kick off the workflow again? sorry about the chain of back-and-forth
ac9b48f to
7d1b9c6
Compare
Lift behaviour to higher level so caller can specify the behaviour Specifically to avoid throwing exceptions on missing path errors when getting DeviceInfo
TalkTalk (UK) branded F5364 does not recognise the ModelNumber attribute requested in GetDeviceInfo
This causes an exception to be thrown, because GetResponse throws for any ActionError.
End result is that HA cant connect to the router to retrieve the useful info, even though the ModelNumber is not essential info.
Fixed by:
There was a TODO around how to deal with multiple ActionErrors, which this PR also addresses - since errors can now be thrown at the point of reading the value from the response if required.
For calls other than GetDeviceInfo, the existing behaviour is retained.