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

Add support for handling ping responses on long op's #1

Merged
merged 1 commit into from
Jun 3, 2014

Conversation

stromnet
Copy link
Contributor

@stromnet stromnet commented Jun 2, 2014

When owserver is busy (i.e. high load/contention, or long ops), it will
send ping frames every second or so.

How to reproduce:

  1. Run pyownet, with verbose, and repeatedly read a temperature sensor (/uncached/nnnn/temperature).
  2. Run separate console which does while true; do owread /uncached/nnnn/temperature; done

(or just two pyownet I guess). You will eventually notice that it receives packages with -1 payload.

When owserver is busy (i.e. high load/contention, or long ops), it will
send ping frames every second or so.
@miccoli miccoli self-assigned this Jun 3, 2014
@miccoli miccoli added the bug label Jun 3, 2014
@miccoli miccoli merged commit 3553740 into miccoli:master Jun 3, 2014
@miccoli
Copy link
Owner

miccoli commented Jun 3, 2014

Thank stromnet for the fix. I was able to observe this behaviour (OwnetProxy.read sometimes returning an empty string from an owserver under stress) but not to track it down to the described ping frames.

@stromnet
Copy link
Contributor Author

stromnet commented Jun 3, 2014

You're welcome, thanks for merging :)
Btw, any plans for supporting persistent connections?

@stromnet stromnet deleted the pingfix branch June 3, 2014 12:30
@miccoli
Copy link
Owner

miccoli commented Jun 3, 2014

Persistent connections are on my todo list, to be implemented via a context manager.

@stromnet
Copy link
Contributor Author

stromnet commented Jun 3, 2014

Great, then I won't look further into doing that right now. I'm interested in the feature, but it's not a blocker at the moment.

Thanks for the library btw, works nice for me since I need to do alarm scans, which I haven't been able to get the native python library to do. And also, the native library is...native, depending on the .so and all.. Nice to not have that dependency!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants