Skip to content

Fix Canary camera stream blocking call#71369

Merged
balloob merged 2 commits intohome-assistant:devfrom
0bmay:dev
May 6, 2022
Merged

Fix Canary camera stream blocking call#71369
balloob merged 2 commits intohome-assistant:devfrom
0bmay:dev

Conversation

@0bmay
Copy link
Copy Markdown
Contributor

@0bmay 0bmay commented May 5, 2022

fixes a "detected blocking call to putrequest inside the event loop. This is causing stability issues. Please report issue for canary doing blocking calls at homeassistant/components/canary/camera.py, line 149: self._live_stream_session.live_stream_url, extra_cmd=self._ffmpeg_arguments" from log file.

Proposed change

While fixing the py-canary code to work after Canary changed return data from their API calls, tesitng the live video stream of the Canary Pro camera logged an error in the HA logs, requesting that the call be put in an await self.hass.async_add_executor_job call. So I changed the code to do so and the video stream worked as expected.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue: 44830, 71273

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

fixes a "detected blocking call to putrequest inside the event loop. This is causing stability issues. Please report issue for canary doing blocking calls at homeassistant/components/canary/camera.py, line 149: self._live_stream_session.live_stream_url, extra_cmd=self._ffmpeg_arguments" from log file.
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@balloob balloob added this to the 2022.5.2 milestone May 5, 2022
@balloob
Copy link
Copy Markdown
Member

balloob commented May 5, 2022

If you are fixing the code of the library, might be an idea to get rid of properties that do I/O. So easy to get it wrong.

@0bmay
Copy link
Copy Markdown
Contributor Author

0bmay commented May 5, 2022

@balloob, the fix that I submitted was the only property that does any type of I/O.. I'll see if I can refactor to make it more clean.

@gibbiemonster
Copy link
Copy Markdown

gibbiemonster commented May 6, 2022

Confirming this works with Canary Flex and Canary Pro.

Can't really do much else because I'm afraid I'm going to mess something up lol

@0bmay
Copy link
Copy Markdown
Contributor Author

0bmay commented May 6, 2022

Thanks @BlockArchitech! I don't have a flex to test with, so this is great news!

@gibbiemonster
Copy link
Copy Markdown

Yeah, no problem. From what I know, flex and pro use the same streaming. Cannot say same about view as i don't have one.

sent using GitHub mobile

@MartinHjelmare MartinHjelmare changed the title fix: Canary stream camera, fix blocker Fix Canary camera stream blocking call May 6, 2022
@0bmay
Copy link
Copy Markdown
Contributor Author

0bmay commented May 6, 2022

I must have done something wrong... not sure how the codecov/patch error is dinging my pull request erroring on files I didn't touch..

@0bmay
Copy link
Copy Markdown
Contributor Author

0bmay commented May 6, 2022

I'm going to redo this pull request... and make changes in a specific branch. Sorry for the trouble...

@0bmay 0bmay closed this May 6, 2022
@balloob balloob reopened this May 6, 2022
@balloob
Copy link
Copy Markdown
Member

balloob commented May 6, 2022

@0bmay thats an issue on our end. This PR is good to go!

@balloob balloob merged commit 1a00bb9 into home-assistant:dev May 6, 2022
@0bmay
Copy link
Copy Markdown
Contributor Author

0bmay commented May 6, 2022

@balloob ok! I know better for next time, so it was a good learning experience on the HA workflow. Thanks!

balloob pushed a commit that referenced this pull request May 6, 2022
* fix: Canary stream camera, fix blocker

fixes a "detected blocking call to putrequest inside the event loop. This is causing stability issues. Please report issue for canary doing blocking calls at homeassistant/components/canary/camera.py, line 149: self._live_stream_session.live_stream_url, extra_cmd=self._ffmpeg_arguments" from log file.

* refactor: black formatting changes

tsia
@balloob balloob mentioned this pull request May 6, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants