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

Move bled112 to use deque instead of queue for packet handling #908

Open
timburke opened this issue Nov 27, 2019 · 0 comments
Open

Move bled112 to use deque instead of queue for packet handling #908

timburke opened this issue Nov 27, 2019 · 0 comments

Comments

@timburke
Copy link
Member

Right now we have a simple way for packets to be read from the BLED112. We have a dedicated thread that reads packets and puts them on a queue. The BLED112CommandProcessor runs in its own thread and blocks when it needs a packet on queue.get(). Packets are always processed one at a time.

For moving to asyncio, we need to instead give packets to OperationManager as input so that they can be used to unblock any running coroutine operations. OperationManager.process_message() must only be called from within the event loop itself, so we need an efficient way to hand packets from the ReaderThread to the event loop.

The obvious way would be to call add_callback_threadsafe for each packet but that means many locked callback adds as packet volumes increase. An alternative would be to batch packets with a timeout but that risks increasing latency when we are waiting for just 1 packet in and not many packets are coming.

An intermediate option is to share packets using an unlocked deque and have a handler scheduled that empties out the deque whenever it is run. A separate shared boolean flag notes if the handler has run since the last invocation so that the ReaderThread can avoid queuing it again since it will get all packets once it runs:

image

Important: The ordering of setting is_sched is important to make sure there is no race condition where a packet could be added to the deque but no callback scheduled after that time. The logic has been thought through in the above diagram but not formally proven to be correct so it should be rechecked before implementation.

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

No branches or pull requests

1 participant