-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
Foscam Camera: Adding exception handling when fetching the camera image to avoid python exception errors when host is not reachable or rather any url error to camera #6964
Conversation
…hon errors when host is not reachable or any url errors to camera
@viswa-swami, thanks for your PR! By analyzing the history of the files in this pull request, we identified @heathbar, @fabaff and @balloob to be potential reviewers. |
Hi @viswa-swami, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
…he unused error handle
I think you should add a logging.waring to info the user that we "lost" a picture. |
Just wondering, Is that really required to log, because what we are doing
for a camera is basically streaming the content. Of course, depends on the
use of camera, but what purpose would that solve if we log the message?
Will user really benefit from a message that we lost one frame when the
next frame might work properly and user might be able to see the content
via hass.
…On Thu, Apr 6, 2017 at 3:32 PM Pascal Vizeli ***@***.***> wrote:
I think you should add a logging.waring to info the user that we "lost" a
picture.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6964 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGrhc_rIiBPKT3PYy1lYPsD9eHzemU_6ks5rtT2ygaJpZM4M1_L3>
.
|
Moreover, one use case is that, the camera could potentially be off when the user is at home for privacy reasons, and will only be armed when user is away. So, during the entire period when camera is off, communication will fail, during which time, log will again be filled with this 'lost a picture' message. |
…ge to avoid python exception errors when host is not reachable or rather any url error to camera (#6964) * Adding exception handling when fetching the camera image to avoid python errors when host is not reachable or any url errors to camera * Added exception as ConnectionError instead of plain except * Added exception as ConnectionError instead of plain except. Removed the unused error handle
Cherry-picked into the 0.42 branch |
Thank you for including this fix into the 0.42 release.
…On Sat, Apr 8, 2017, 07:55 Paulus Schoutsen ***@***.***> wrote:
Cherry-picked into the 0.42 branch
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6964 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGrhc0D4lEbypAxKzb_tH8i-9xrVGdQ_ks5rt3W-gaJpZM4M1_L3>
.
|
Description:
Added exception handling for the requests.get call in the foscam camera component. Earlier code was just trying to get the image from the camera url directly and without checking for response validity, just returns the response back to the parent caller. This caused the requests.get() to fail when host is not reachable. Added the exception handling around the requests.get() call.
Related issue (if applicable): fixes #6962
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml
(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass