Closed superware closed 8 years ago
Thankyou for your comments. The SerialPortStream was designed to have buffering from the outright to provide consistent behaviour independent from the underlying serial port driver in Windows. While there's an IOCtl() that can specify this, a driver is not required to honour such buffering. I could write 5MB on one driver and it blocks for a long period of time, but eventually sending the data, or on another driver where the write would fail. Buffering abstracts that from the user. The same goes for the input path, you have to service by reading the serial port driver at regular intervals else you run into data loss or reduced performance (if hardware flow control is effectively used).Different behaviour for different serial drivers is something I want to generally avoid.
Buffering solves that main use case for which I wrote this library and that was to allow automation, where a test program may be sleeping for 30 seconds, but where there should be no loss of serial data. Performance reduction was a tradeoff for easier use and stream abstraction which didn't exist in the Microsoft implementation (they read/write using asynchronous I/O directly to the serial port).Pro's to the MS implementation here is performance, Cons are some of what I've mentioned above (and some other weird bugs especially with character handling)
Secondly, to keep this compatible with Windows XP (there is still a few), the read/write has to be on it's own single thread for CancelIO to work (note, there's an API on Vista that removes this restriction). So you have to implement some kind of reader/writer consumer pattern which is the second biggest reason for buffering and influenced the design somewhat. Inside the main thread I use overlapped I/O so that none of the read/write calls to the OS should block. You can't cancel the MS implementation, Look at trying to Close() or Dispose() while a blocking read/write is in progress to see what happens (there's a test case for that and was used to test a previous issue on GitHub). These are likely fixable on Vista and later, but at 2012 when the library started, XP was (and still is somewhat) relevant.
As for timing - as soon as the data arrives, it is received and your application is notified. Events are put on a worker thread, so that the main loop within the SerialPortStream can operate even if your event delegate takes longer than it should, so that driver buffers don't fill up resulting in buffer overflows. The MS implementation puts much harder realtime restrictions on your application if you use events.
The Read/Write methods use Events to be notified as soon as data is in a buffer, that you get the information as soon as it's there. So in reality I do not think you'll notice any performance loss.
The main reason I can think of for performance loss with my implementation will be to do with the Windows scheduler due to having multiple threads. Indeed, my own tests show on Linux using Mono that data arrives and is notified generally after 1 or 2 bytes (115200 baud) while reading in a tight loop as the Linux scheduler is much faster (10ms compared to 55ms for Windows). So if you're happy with scheduling delays, there should be no issue. That's how I found a bug in the .NET framework in System.Text.
Of course you're more than welcome to do performance testing comparing both implementations (and even on Linux and with Mono), as well as to fork and make updates.
Apart from bug fixing (and loss of a device still needs to be worked on), I really don't expect many more changes to this library.
The internal
Read
(and possiblyWrite
) buffer introduces latency which might be significant. I think this actually isn't a SerialPortStream task (even more if not optional), since it violates the Single Responsibility Principle (SRP).Imho, a dedicated (and configurable)
BufferedStream
class should take care of this, and not theSerialPortStream
itself.Further more, the example you gave:
Sounds like a bad practice at best :-) An I/O stream should operate as fast as possible, closest to the "low level" as possible, anyway this should be the default.
All in a constructive spirit, of course. Thank you for your great work and response.