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

Remove useless call to tcflush on open #40

Merged
merged 1 commit into from
Mar 22, 2022
Merged

Conversation

jannic
Copy link
Contributor

@jannic jannic commented Mar 15, 2022

Closes: #37

This is a draft PR for now, as I didn't have time to test it on real hardware.

@ijustlovemath
Copy link

I wouldn't call this useless; this is pretty standard for getting the device into a known clean state. Otherwise, you may be dropped into the middle of a protocol, which can be hard to handle.

@jannic
Copy link
Contributor Author

jannic commented Mar 18, 2022

I'm not saying that a flush is useless in general. But at that location it doesn't help, and it is sometimes harmful.

It doesn't help as it is called before the serial port is configured. So there is still a window where data received with mismatched settings would cause garbage in the input buffer, which is not flushed but delivered to the application.

And it may be harmful because it is done after DTR is asserted, so it may throw away actual data. (That's the issue reported in #37)

As only the application can know the right time to flush buffers, I don't think it should be done automatically during open.
This is why I added a reference to TTYPort::clear to the doc comment: It's the applications duty to throw away garbage, if necessary.

@jannic jannic marked this pull request as ready for review March 18, 2022 21:55
Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

As only the application can know the right time to flush buffers, I don't think it should be done automatically during open.

Yeah, I agree with this. I did check the (not longer maintained) serial library and they are not calling tcflush() in open, so at least that's consistent.

Thanks for this!

@jessebraham jessebraham merged commit 3d6c1b2 into serialport:main Mar 22, 2022
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.

tcflush on open may cause data loss
3 participants