Support arbitrary baud rates on BSDs #843
Conversation
Susurrus
force-pushed
the
arb_baud
branch
5 times, most recently
from
133cfa8 to
08d1228
Compare
|
It looks like things are still not correct for mac platforms; it needs to be decided if u32 is a fine output type or if it should really be |
Susurrus
force-pushed
the
arb_baud
branch
2 times, most recently
from
ec16106 to
06d0c21
Compare
|
If you implement cfsetspeed(&mut t, BaudRate::B9600);
let b : BaudRate = cfgetispeed(&t).try_into().unwrap(); |
|
TryFrom is nightly-only. I'll add a FIXME to change the API in the future. |
|
I'll take that as an implicit review @asomers, but cancel this merge if I shouldn't have. bors r+ |
|
Not yet! That's was just what I could come up with while still at work. I'm going to do the full review now. bors cancel |
|
bors r- |
Canceled |
src/sys/termios.rs
Outdated
| @@ -354,6 +475,13 @@ impl From<libc::speed_t> for BaudRate { | |||
| } | |||
| } | |||
|
|
|||
| // TODO: Include `TryFrom<u32> for BaudRate` once that API stabilizes | |||
| impl From<BaudRate> for u32 { | |||
There was a problem hiding this comment.
On Linux this function will return something very different than what it returns on the BSDs. I don't think that's what we want. We should probably only implement this trait on the BSDs. On Linux, BaudRate is opaque.
There was a problem hiding this comment.
Good catch, I originally meant to only expose this on BSDs.
src/sys/termios.rs
Outdated
| /// Get output baud rate (see | ||
| /// [cfgetospeed(3p)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/cfgetospeed.html)). | ||
| /// | ||
| /// `cfgetospeed()` extracts the output baud rate from the given Termios structure. |
There was a problem hiding this comment.
Termios should be in backticks here and elsewhere.
There was a problem hiding this comment.
I don't think it should, Termios is not a struct, termios is, Termios is the name of the API.
There was a problem hiding this comment.
NVM, I agree with what you're saying.
src/sys/termios.rs
Outdated
| } | ||
|
|
||
| /// Set both the input and output baud rates (see | ||
| /// [termios(3)](http://man7.org/linux/man-pages/man3/termios.3.html)). |
There was a problem hiding this comment.
It's silly to use a Linux man page link in the documentation for non-Linux operating systems. How about https://www.freebsd.org/cgi/man.cgi?query=cfsetspeed instead?
There was a problem hiding this comment.
Indeed, corrected.
src/sys/termios.rs
Outdated
| let inner_termios = unsafe { termios.get_libc_termios_mut() }; | ||
| let res = unsafe { libc::cfsetospeed(inner_termios, baud as libc::speed_t) }; | ||
| termios.update_wrapper(); | ||
| Errno::result(res).map(drop) |
There was a problem hiding this comment.
Personally, I find .map(|_| ()) to be more intuitive than .map(drop), though it is a little longer.
.map(drop) makes it seem like there is a need to drop the type at that point when really all you're doing is trying to erase the type information and return an empty tuple. So do that explicitly instead of throwing drop in there.
|
@asomers Take a look-see now, might be more to your liking. |
|
bors r+ |
Closes #842