Fix pjlink issue#20510
Conversation
Some projectors do not respond to pjlink requests during a short period after the status changes or when its in standby, resulting in pypjlink2 throwing an error. This fix catches these errors. Furthermore, only the status 'on' and 'warm-up' is interpreted as switched on, because 'cooling' is actually a switched off status.
homeassistant
added
cla-needed
platform: media_player.pjlink
small-pr
ghost
added
the
| self._muted = projector.get_mute()[1] | ||
| self._current_source = \ | ||
| format_input_source(*projector.get_input()) | ||
| except: |
| self._muted = projector.get_mute()[1] | ||
| self._current_source = \ | ||
| format_input_source(*projector.get_input()) | ||
| except: |
There was a problem hiding this comment.
Make sure to only catch exceptions we expect.
There was a problem hiding this comment.
I could make the changes you mentioned, but I'm new to Github and don't know the processes yet. Do I have to open a new pull request or is it possible to modify the existing pull request?
There was a problem hiding this comment.
Just commit and push to the same branch and this PR will be updated.
Improved error handling
| else: | ||
| raise | ||
| except Exception as err: | ||
| if type(err).__name__ == 'ProjectorError' and str(err) == 'unavailable time': |
There was a problem hiding this comment.
line too long (93 > 79 characters)
| self._current_source = None | ||
| else: | ||
| raise | ||
| except Exception as err: |
There was a problem hiding this comment.
If the expected exception is called ProjectorError we should catch that here, instead of Exception.
There was a problem hiding this comment.
I tried that first, of course. But I was not able to import the ProjectorError class for some reason. I'll try again later.
Improved error handling
|
I'm afraid that my changes only fix my own problem. I've noticed that the error is also triggered, when the methods 'turn_on' and 'turn_off' are called, while my projector is in this 'unknown mode'. But unlike ChronicledMonocle's projector, this 'unknown mode' does not occur permanently in standby mode, but only for a short time after switching on or off. In my opinion, issue #16606 probably can't be solved by modifying homeassistants PJLink component. To really fix this, the error handling of "pypjlink2" needs to be improved. If that happens, my changes will also become obsolete. |
| self._muted = projector.get_mute()[1] | ||
| self._current_source = \ | ||
| format_input_source(*projector.get_input()) | ||
| except KeyError as err: |
There was a problem hiding this comment.
When would we get the KeyError? Is it if projector.get_mute() returns a dict instead of a list?
There was a problem hiding this comment.
No, the KeyError is also caused by get_power(). At least with my projector. When I turn on the projector and call the method all the time, I get the status warm-up for a while. Then at some point an answer takes about one second longer. This is the KeyError, which appears only once. Immediately afterwards, I get ProjectorError with message "unavailable time" for about 20-30 seconds. After this time, again I get for a short time the status warm-up and then on.
There was a problem hiding this comment.
The only thing I'd change right now is to swap if pwstate == 'on' or pwstate == 'warm-up': with if pwstate in {'on', 'warm-up'}:. Seems like my little experience in python isn't enough to solve the problem. Do you have another suggestion?
There was a problem hiding this comment.
Yeah, that clean up is good, but use a tuple:
if pwstate in ('on', 'warm-up'):There was a problem hiding this comment.
The library shouldn't let KeyError slip through. That's a bug in the library. It could raise a library specific exception if needed.
But it's ok to fix here for now.
There was a problem hiding this comment.
If you find the cause, please try to fix this in the library and make a PR to them.
There was a problem hiding this comment.
Thanks for your advice.
Clean up
|
Can be merged when build passes. |
* Fix issue #16606 Some projectors do not respond to pjlink requests during a short period after the status changes or when its in standby, resulting in pypjlink2 throwing an error. This fix catches these errors. Furthermore, only the status 'on' and 'warm-up' is interpreted as switched on, because 'cooling' is actually a switched off status. * Update pjlink.py Improved error handling * Update pjlink.py Improved error handling * Update pjlink.py Clean up
5 participants
Description:
Some projectors do not respond to pjlink requests during a short period after the status changes or when its in standby, resulting in pypjlink2 throwing an error. This fix catches these errors. Furthermore, only the status 'on' and 'warm-up' is interpreted as switched on, because 'cooling' is actually a switched off status.
Related issue (if applicable): fixes #16606
Checklist:
tox. Your PR cannot be merged unless tests pass