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

Fix receiving of binary data with unknown length. #28

Closed
wants to merge 1 commit into from

Conversation

xrombik
Copy link

@xrombik xrombik commented Jun 29, 2019

This commit adds the ability to receive data in binary format with undefined length. To determine the end of the data block, the character interval is used. This is the most expected behavior when receiving data, because it allows you to receive blocks of data in the format in which they were transmitted. Without this commit, ingestion occurs before the buffer is filled with the expected number of bytes, which is almost always useless. More detailed description is here:
http://unixwiz.net/techtips/termios-vmin-vtime.html

@vsergeev
Copy link
Owner

In most embedded applications involving serial ports, I've refrained from and didn't have the luxury of depending on character time to frame messages, and instead relied purely on protocol delimiters or message headers. But this is an interesting feature, and I can imagine it being useful in several contexts (actual TTYs, NMEA, etc.). I'll have to take a closer look, but ideally, I'd like to merge it while preserving as much of the existing functionality and timeout semantics of read() as possible.

@xrombik
Copy link
Author

xrombik commented May 4, 2020

You variant does not pass simple test - send data throw serial ports, even they are emulated or hardwired - its merge data blocks on unpredictable way. You took this decision from python-serial, after the authors made changes there, which led to this error. This is a bug and a regression because of this behavior in python-serial in the first releases. This is wrong behavior because it differs from the behavior we have when we use native open/read/write calls through python directly. Your wrapper allows its own interpretation of the data, which means that you must specify that this solution allows only text data in a limited range of character code values, and also requires on the user side the use of a protocol that knows how to build/disassemble the data stream into individual blocks.

@vsergeev
Copy link
Owner

vsergeev commented May 5, 2020

I'm curious, aside from an interactive terminal, do you have an example of this kind of application that uses the serial port intercharacter timeout for framing? I thought gpsd might use it, but it appears that even it opens the serial port in O_NONBLOCK, where VTIME and VMIN are ignored, and reads chunks as they arrive.

I'm not aware of pyserial's approach or history with reads, but the current read implementation in python-periphery is no different than what you would find for a stream socket or a Unix pipe, where the application is expected to frame messages by headers or delimiters in the data stream, not by timing. python-periphery is written in the context of interfacing with external peripherals via UART, SPI, I2C, etc. busses on an embedded system, where protocols with such headers and/or delimiters are very common. I'm not sure I follow your comment about the limited range of character code values -- the Serial module should allow reading and writing raw binary data, with the exception of XON/XOFF control codes if software flow control is enabled.

That being said, I still intend to merge this PR into python-periphery, as it is a better and more flexible timeout mechanism, though I will make some modifications to ensure it can be used in a backwards compatible way with the more naive timeout when VTIME and VMIN are not configured.

vsergeev added a commit that referenced this pull request May 18, 2020
vsergeev added a commit that referenced this pull request May 18, 2020
vsergeev added a commit that referenced this pull request May 27, 2020
vsergeev added a commit that referenced this pull request May 28, 2020
@vsergeev vsergeev closed this in 0f97423 May 29, 2020
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.

None yet

2 participants