Skip to content

Fix onvif writing to closing transport#33970

Closed
indykoning wants to merge 1 commit intohome-assistant:devfrom
indykoning:feature/fix_onvif_write_to_closing_transport
Closed

Fix onvif writing to closing transport#33970
indykoning wants to merge 1 commit intohome-assistant:devfrom
indykoning:feature/fix_onvif_write_to_closing_transport

Conversation

@indykoning
Copy link
Copy Markdown
Contributor

@indykoning indykoning commented Apr 10, 2020

Proposed change

(Probably) cheaper cameras do not seem to close the request quickly enough, resulting in it being closed while the next request is being made. First i saw duplicate code doing the same thing, retrieving profiles unnecessarily. So i made a helper function to retrieve the profile and save it to serve it next time this function gets called.

Then when multiple requests were being made in very quick succession the second request would fail with "Cannot write to closing transport". When i add a wait of 0.1s to this the transport will have been closed and a new one can be created.

Run less unnecessary requests, wait a little longer to allow camera to close the request before opening next request.

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

Example entry for configuration.yaml:

# Example configuration.yaml
camera:
  - platform: onvif
    host: 192.168.1.111

Additional information

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.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same issue here, PTZ was not working for me because two functions to get the PTZ service were being called in quick succession.

After reducing this to a single function i can now use PTZ

@MartinHjelmare MartinHjelmare changed the title Onvif - Fix write to closing transport Fix onvif writing to closing transport Apr 10, 2020
@stale
Copy link
Copy Markdown

stale Bot commented May 10, 2020

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale Bot added the stale label May 10, 2020
@MartinHjelmare
Copy link
Copy Markdown
Member

Please solve the merge conflict.

@hunterjm do you have time to review?

@stale stale Bot removed the stale label May 17, 2020
@hunterjm
Copy link
Copy Markdown
Member

A lot of these same concerns were addressed in the recent refactor. I did not add additional waits, however. Could you check this on 0.110 and see if your issues are resolved without the additional waits?

@indykoning
Copy link
Copy Markdown
Contributor Author

Sadly it did not fix all of them for me, luckily i could remove a ton of them so far.
I had two spots during the config_flow this seems to happen when a service gets created and instantly used. As an example:

devicemgmt = device.create_devicemgmt_service()
network_interfaces = await devicemgmt.GetNetworkInterfaces()

devicemgmt = device.create_devicemgmt_service()

I also got an error ClientPayloadError: Response payload is not completed on

presets = await ptz_service.GetPresets(profile.token)

Maybe my camera does not support the presets. Anyhow, i caught that exception to allow setup to be completed and PTZ does not seem to be affected for the camera

@indykoning
Copy link
Copy Markdown
Contributor Author

I have fixed the merge conflict 👍

@hunterjm
Copy link
Copy Markdown
Member

@indykoning - I'm sorry to do this to you again, but I've made a ton of bugfix releases, and one of them (#35932) had a large impact on how Transports work. That made it into 0.110.2. Could you see if that fixed your issue, or if the sleeps are still required in some places?

@hunterjm hunterjm self-assigned this May 24, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Jun 23, 2020

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale Bot added the stale label Jun 23, 2020
@stale stale Bot closed this Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Onvif cams not working anymore

4 participants