Skip to content
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

adds retry when pull subscriptions fails #319

Merged
merged 1 commit into from
May 18, 2024

Conversation

HarryEMartland
Copy link
Contributor

Hi, I am running https://shinobi.video/ which uses this library and have noticed that sometimes the ONVIF events don't get triggered. I narrowed this down to when there is a connection disruption e.g. my router updates or the camera is restarted.

This Pull Requests adds a retry mechanism if the connection fails. I have added a configurable wait time and used that as a kind of feature flag so this change is more backwards compatible.

I struggled to write tests for this due to the timeout and I think the mock server being in an external process. If someone could guide me the best way to test this I would be happy to write some tests.

I have tested this locally adding a listener, restarting my camera and then seeing if events continue to flow

@HarryEMartland
Copy link
Contributor Author

Just noticed #314 which may solve this too however it does not have a delay. I worry if there is no delay it could consume a lot of CPU retrying.

There may be a happy middle ground where we do some fancy exponential backoff but retry straight away.

@agsh agsh self-requested a review May 18, 2024 08:37
@agsh agsh merged commit e946bc4 into agsh:master May 18, 2024
@agsh
Copy link
Owner

agsh commented May 19, 2024

@HarryEMartland, hello! Thanks for researching and digging the old code! 😄 And for reminding me about #314, the connection socket timeout issue for the TP-Link Tapo C220
Yep, these seem to be similar connection issues. There are some points in the callback functions where we can check these errors:

  • in _eventRequest calling createPullPointSubscription
  • in _eventPull calling pullMessages

I've created a new PR which combines these two issues. So now it's your turn to review my code 😈
>>> HERE IT IS <<<
Maybe there are still places that stop the pullMessages loop, for example, running ONVIF commands that are not supported by a specific device (such as renew or unsubscribe). Unfortunately, I can’t test it on any device right now, I hope I haven’t forgotten how to program without errors 🤣

@HarryEMartland HarryEMartland deleted the retry-events branch May 19, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants