de-vri-es / serial2-rs

Cross platform serial ports for Rust
Other
41 stars 10 forks source link

Unsupported baudrate on Mac #38

Closed Hacheriki closed 2 weeks ago

Hacheriki commented 2 weeks ago

First of all, thanks for this crate. I am currently using library that depends on your lib. And I have an issue that whenever I try to open port with baud rate 1000000 it panics and writes this in logs Os { code: 22, kind: InvalidInput, message: "Invalid argument" } It does not happen on baud rates lower. It also does not happen when I use another library for serial ports or when I use your library on Windows. Is there a some way to fix this issue?

de-vri-es commented 2 weeks ago

Hmm, if that happens it sounds like the OS is saying the baud rate is not supported on that device. Setting the baud rate is pretty straightforward on macOS.

Could you try setting the baud rate to 1000000 after opening the serial port, and then check exactly where it fails?

It could be on Settings::set_baud_rate() or on SerialPort::set_configuration(). I would like to know which one it is.

let mut serial_port = serial2::SerialPort::open(path, 115200).unwrap();
let mut config = serial_port.get_configuration().unwrap();
config.set_baud_rate(1000000).unwrap(); // Here?
serial_port.set_configuration(&config).unwrap(); // Or here?
Hacheriki commented 2 weeks ago

Sure, It happens here serial_port.set_configuration(&config).unwrap(); I am using this to manipulate dynamixel (and I just realized, that's your's library too:) ) And on regular Dynamixel Wizard I can open this port with given baud rate, also I can open it with rust crate "serialport" So I thought it could be something with serial2 library. Or there some differences? Also when I switch dynamixel to another baud rate, everything works perfectly fine, but I need to make it work on higher rates

de-vri-es commented 2 weeks ago

Sure, It happens here serial_port.set_configuration(&config).unwrap();

Hmm.. interesting. It seems like that's really the OS saying the configuration is invalid...

I am using this to manipulate dynamixel (and I just realized, that's your's library too:) )

Hehe, small world :D

And on regular Dynamixel Wizard I can open this port with given baud rate, also I can open it with rust crate "serialport"

Is there something like strace for macOS that you could use to monitor the syscalls being made? It would be extremely useful if I could compare the syscalls and their arguments being made to configure the port.

So I thought it could be something with serial2 library. Or there some differences? Also when I switch dynamixel to another baud rate, everything works perfectly fine, but I need to make it work on higher rates

Yeah, if the serial port supports that baud rate then serial2 needs to be able to configure it.

de-vri-es commented 2 weeks ago

Ah.. it appears that on macOS you need a non-standard IOSSIOSPEED ioctl... why on earth they didn't just accept higher values in termios with tcsetattr is a mystery to me... They have the bits for it..

Anyway, this will need to be implemented.

One difficulty is that some sources say that the speed set with this ioctl isn't reflected in the output of tcgetattr. And I don't see any documentation about an ioctl to get the speed again.

On the other hand, some test code from apple seems to indicate that you can get the speed with tcgetattr after a IOSSIOSPEED ioctl:

From https://opensource.apple.com/source/IOSerialFamily/IOSerialFamily-91/tests/IOSerialTestLib.c.auto.html

int _testIOSSIOSPEEDIoctl(int fd, speed_t speed)
{
    struct termios options;

    // The IOSSIOSPEED ioctl can be used to set arbitrary baud rates other than
    // those specified by POSIX. The driver for the underlying serial hardware
    // ultimately determines which baud rates can be used. This ioctl sets both
    // the input and output speed.

    if (ioctl(fd, IOSSIOSPEED, &speed) == -1) {
        printf("[WARN] ioctl(..., IOSSIOSPEED, %lu).\n", speed);
        goto fail;
    }

    // Check that speed is properly modified
    if (tcgetattr(fd, &options) == -1) {
        printf("[WARN] _modifyAttributes: tcgetattr failed\n");
        goto fail;
    }

    if (cfgetispeed(&options) != speed ||
        cfgetospeed(&options) != speed) {
        printf("[WARN] _modifyAttributes: cfsetspeed failed, %lu, %lu.\n",
               speed,
               cfgetispeed(&options));
        goto fail;
    }

    return 0;
fail:
    return -1;

}

/edit: Ah, but also from apple (https://developer.apple.com/library/archive/samplecode/SerialPortSample/Listings/SerialPortSample_SerialPortSample_c.html#//apple_ref/doc/uid/DTS10000454-SerialPortSample_SerialPortSample_c-DontLinkElementID_4):

    // Print the new input and output baud rates. Note that the IOSSIOSPEED ioctl interacts with the serial driver
    // directly bypassing the termios struct. This means that the following two calls will not be able to read
    // the current baud rate if the IOSSIOSPEED ioctl was used but will instead return the speed set by the last call
    // to cfsetspeed.

    printf("Input baud rate changed to %d\n", (int) cfgetispeed(&options));
    printf("Output baud rate changed to %d\n", (int) cfgetospeed(&options));

This means we will need to cache the baud rate on the SerialPort struct for OS X, which means that if anyone else adjusts the speed outside of our control we will never even know... :|

Hacheriki commented 2 weeks ago

Oh, I see… Certified Apple moment :| I found util that can show syscalls on Mac, and will try it tomorrow, maybe it will be helpful If you need any tests, you can ask anytime

de-vri-es commented 2 weeks ago

In theory I implemented it in #39.

Could you test that PR? In particular, I'd also like to know if serial_port.get_configuration()?.get_baud_rate()? reports the correct custom/high baud rate now.

If not, it will break timeout calculations in the last dynamixel2 release :p

Hacheriki commented 2 weeks ago

You're magician, now it does not crash and serial_port.get_configuration()?.get_baud_rate()? gets right baud rate value that I specified. I couldn't get real device today, so I'll test it next day. But seems fixed :)

de-vri-es commented 2 weeks ago

Yeah.. a consequence of this though: if you clone the serial port (with try_clone()) and then change the baud rate on one of them, the other one will wrongly keep reporting the old value.

I suppose we could try to figure out if tcgetattr() really doesn't report the new speed...

I pushed another branch: apple-baud-rates-trust-tcgetattr, can you also test with that one? In specific again if it reports the correct baud rate after setting it. Be sure to also test with a high baud rate of 1_000_000 again :)

If that branch works correctly, it will be much better in terms of weird bugs regarding speed reporting.

Hacheriki commented 2 weeks ago

apple-baud-rates-trust-tcgetattr works fine I tried this

let mut serial_port = serial2::SerialPort::open("/dev/cu.usbserial-110", 115200).unwrap();
let mut config = serial_port.get_configuration().unwrap();
let mut serial_port_two = serial_port.try_clone().unwrap();

config.set_baud_rate(1000000).unwrap();
serial_port.set_configuration(&config).unwrap();

let a = serial_port.get_configuration().unwrap().get_baud_rate().unwrap();
let a2 = serial_port_two.get_configuration().unwrap().get_baud_rate().unwrap();
println!("{a} {a2}")

And it output this 1000000 1000000 So it seems to report right value Also It now works with dynamixel2 lib as I force changed dependency. Thank you a lot :)

de-vri-es commented 2 weeks ago

Alright, then it's not too bad :D I hope this won't turn out to break with some device drivers though :see_no_evil:

Thanks for testing! I will go with the trust-tcgetattr version then!

de-vri-es commented 2 weeks ago

Fix released in v0.2.26 \o/