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 Read cancellation #121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add Read cancellation #121

wants to merge 2 commits into from

Conversation

kroppt
Copy link

@kroppt kroppt commented Sep 22, 2021

Fixes #105
Related to #117

In this solution, I gave serial.Serial a new function called ReadContext, which is identical to Read, except it also takes in a context.Context as its first parameter.

I understand that there may be some concern over the API, given that Go developers as a whole have not decided on how to resolve the problem of cancellation for abstract file systems, but it seems OK to add this feature, because the interface is defined in this library and is not a generalized interface that is a part of the standard library. I am happy to talk through any concerns.

I tested this on both Linux and Windows.
It probably also works for other Unix platforms, since their implementation is the same as Linux.

I cleaned up the Linux tests, since they weren't very careful.
Additionally, I added Windows tests, which were a bit trickier. I didn't see an obvious way to create a dummy serial port, so the tests will run using the first serial port on the system, otherwise skipping the tests.

@soypat
Copy link

soypat commented Feb 18, 2024

I'll note that this could best be solved by an external library that implements cancellation for any kind of io.Reader implementation. The way I've gone about dealing with time-limited readers is by using this API: https://github.com/soypat/cereal/blob/main/nonblocking.go

@kroppt
Copy link
Author

kroppt commented Mar 8, 2024

I'll note that this could best be solved by an external library that implements cancellation for any kind of io.Reader implementation. The way I've gone about dealing with time-limited readers is by using this API: https://github.com/soypat/cereal/blob/main/nonblocking.go

It looks like it creates a long-running goroutine that handles reading and then buffers it for the primary code to read. I think that's a good approach. If I recall, I might have done something like this in the code I was writing. If you use channels to pass along those bytes, it seamlessly integrates with select semantics from the cancellation channel (which avoids having to poll). And if you need it buffered potentially infinitely, you can insert an intermediate goroutine that just buffers the incoming bytes. Boy, this is getting complicated! But I remember it worked well for me.

@KiddoV
Copy link

KiddoV commented Mar 19, 2024

I am interested in this feature too, as sometimes I need to stop the reading manually instead of waiting for an error or the received byte = 0.

@ccollins476ad
Copy link

This is a very useful addition. Not sure whom to direct this question to, but are there any plans to merge this? The merge conflicts don't look too bad.

@kroppt
Copy link
Author

kroppt commented May 21, 2024

@ccollins476ad

This is a very useful addition. Not sure whom to direct this question to, but are there any plans to merge this? The merge conflicts don't look too bad.

Many changes have been made to the repository since this pull request has been opened. It seems that it won't ever be looked at. Plan accordingly. See the above alternatives. I'm happy to help if further clarification is needed.

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.

Add cancellation
4 participants