BSerialPort's WaitForInput() and timeout handling are broken.

[bipolar]

While trying to make serial_mouse work with pc_serial with real hardware (latest nightlies on 32 and 64 bits)...

It seems to me that BSerialPort.WaitForInput() is broken, due to it relying on the following code:

	int err = ioctl(ffd, TCWAITEVENT, &size, sizeof size);

That TCWAITEVENT is hard to find on ​Haiku's sources. Evidently generic/tty does not supports it, and thus, pc_serial also doesn't .

TCWAITEVENT doesn't even appears on google's searchs, at least not in any capacity ouside some BeOS/Haiku related results. Is that ioctl op even a thing?

Regarding timeouts, reads always block, at least when opening pc_serial devices (I can't test with usb_serial, at least until I get some supported adapter, sorry!).

Not really sure if that's BSerialPort's fault, or pc_serial... but I've noticed that software that relies on select() (PySerial's serialposix.py, for example), seem to work better than what BSerialPort is doing (both opening pc_serial ports).

Some tests I'm using for this:

• https://dev.haiku-os.org/attachment/ticket/17842/test_mouse_haiku.cpp
• https://dev.haiku-os.org/attachment/ticket/17861/test_serial.py

I guess I could try to write a C++ version of that test_serial.py. Might be useful to use with strace... but mostly to test alternate versions of BSerialPort/pc_serial.

Regarding the WaitForInput() thing... PySerial relies on fcntl.ioctl(self.fd, TIOCINQ, TIOCM_zero_str) to see if there is data wating to be read, and that seems to work in my tests. Maybe there's a way to rely on that for WaitForInput()?

Also, PySerial relies on select() and plain read(), and it seems to properly respect timeouts (at least it never hanged indefinitely for me, not with the hrev56351 changes, that's it).

Do changing BSerialPort to rely on select() seems like reasonable idea? Should be tty/pc_serial timeout handling be fixed instead?

I'm willing to work on this, but I need the input of people with more knowledge, and less rusty, than me :-D.

Thanks in advance.

[bipolar]

I would also like to point out that ​ClearInput() and ClearOutput() can fail silently.

I guess they can't return a status_t for BeOS compatibility, but maybe they should at least log a warning to syslog? (would have helped find #17861 sooner!

(thanks to waddlesplash for suggesting me to use strace).

Similarly, many of BSerialPort methods call _DriverControl(), which returns an error code, without checking said value. Maybe _DriverControl() is self should log something to syslog?

Opinions/suggestions?

[bipolar]

It seems TCWAITEVENT is a BeOS-only ioctrl. I guess BSerialPort was written on BeOS (way before Haiku's pc_serial/tty), and it worked there. It's, AFAICT, unsupported on Haiku.

Another issue (not affecting BSerialPort, but affecting usage of tty/pc_serial):

termios.tcdrain() is also broken on Haiku (termios.c tries to be compatible with BeOS using an TCSBRK ioctrl with a value of 1 to signify: "wait til all output is written", but the tty and serial drivers do not follow that convention, and just issue a TTYSETBREAK).

I'll keep trying to learn about this, hoping to finally be able to propose some fixes.

[waddlesplash]

TTYSETBREAK is the internal flag equivalent to TCSBRK, which is handled in the generic TTY layer, so that should work just fine. If it doesn't, something else is going wrong.

The BeOS-specific ioctls are indeed marked TODO.

[bipolar]

Thanks for the feedback. Most of this is still pretty confusing to me, so I take my comments with an appropriate amount of salt.

At least on Linux, tcdrain() waits until all the bytes in the output buffer are actually written.

That doesn't seems to be the case on Haiku in my tests, and instead of relying on tcdrain() blocking, I have to call sleep() with a number of milliseconds proportional to the baudrate I'm using, to make sure the data is properly sent.

Double checking due to your comment, it does indeed looks like Haiku's tty module takes that TCSBRK and TTYSETBREAK to usb_serial (a no-op there) and to pc_serial drivers (where it seems to set/unset a register, but not block till the output buffer is emptied).

Maybe I'm wrong in expecting that tcdrain() should behave like on Linux, but in my basic understanding after reading ​this comment, plus what I could find about TCSBRK online, made me think that BeOS did (and thus Haiku should do?) something like this:

When TCSBRK with a value of 0... send breaks down the line for 0.25 seconds, then return (no need for the set/unset variable), and when TCSBRK was issued with a 1... behave like TIOCDRAIN on ​FreeBSD.

​this Linux man page reads (emphasis mine):

Equivalent to tcsendbreak(fd, arg).

If the terminal is using asynchronous serial data transmission, and arg is zero, then send a break (a stream of zero bits) for between 0.25 and 0.5 seconds. If the terminal is not using asynchronous serial data transmission, then either a break is sent, or the function returns without doing anything. When arg is nonzero, nobody knows what will happen.

(SVr4, UnixWare, Solaris, and Linux treat tcsendbreak(fd,arg) with nonzero arg like tcdrain(fd). SunOS treats arg as a multiplier, and sends a stream of bits arg times as long as done for zero arg. DG/UX and AIX treat arg (when nonzero) as a time interval measured in milliseconds. HP-UX ignores arg.)

So, my understanding is that Haiku's implementation of tcdrain() (according to the mentioned comment) and the implementation of TTYSENDBREAK on pc_serial maybe are not 100% aligned.

Writing this just for reference, or to let others spots where I'm getting confused.

In any case, I guess I should test some more (even under BeOS), write some test programs I can share, and hopefully understand enough as to, at least, propose some changes.

Pointers, suggestions and corrections, always welcomed! Thanks!

[waddlesplash]

Fixed in hrev56986

[bipolar]

Just a note: Now that WaitForInput() got implemented, I've noticed that BSerialPort::Read() always blocks for pc_serial, even after calling BSerialPort::SetBlocking(false), when using the test case from #8789 (comment 11).

The combination of that, plus the previously non working WaitForInput() might explain some of the issues with what I assumed was timeout handling here, and on #17842?