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

add rs485 support #29

Merged
merged 9 commits into from
Apr 23, 2024
Merged

add rs485 support #29

merged 9 commits into from
Apr 23, 2024

Conversation

omelia-iliffe
Copy link
Contributor

Initial attempt with the rs485 crate and adding the struct to the settings.
I have tested with an rs485 board and it works. It will currently fail with a non rs485 device so need to add some checks to the get_from_file fn

@de-vri-es
Copy link
Owner

de-vri-es commented Mar 8, 2024

Thanks for the PR!

I think I like the approach of making it part of the Settings. Even if it doesn't map to the termios struct, for a user I think this is an intuitive way to enable RS485 mode.

I prefer to avoid the dependency on the rs485 crate and just use libc directly.

Ideally, we would also figure out how to do this on Windows. If not, we should probably put this under the unix feature until someone can contribute a Windows implementation. But I can take care of this part if you prefer.

@de-vri-es
Copy link
Owner

de-vri-es commented Mar 8, 2024

I've been reading a bit more here: https://www.kernel.org/doc/html/v5.16/driver-api/serial/serial-rs485.html

I think the only thing we need to implement now is the SER_RS485_ENABLED flag. The rest is basically flow control for the RTS/DTR flags, so maybe that should be a new FlowControl option. But I'm pretty sure you don't need that for Dynamixels, because those pins aren't even connected :p

/edit: Actually, this one would be very nice to expose too: SER_RS485_TERMINATE_BUS. And maybe SER_RS485_MODE_RS422.

Maybe SER_RS485_ENABLED and SER_RS485_MODE_RS422 should be combined into a enum Mode { Rs232, Rs422, Rs485 }. Although the Rs232 variant is misleading, because if the serial device only supports one mode, then that mode will be used if you select Rs232...

@omelia-iliffe
Copy link
Contributor Author

Although the Rs232 variant is misleading, because if the serial device only supports one mode, then that mode will be used if you select Rs232...

Its confusing as the PCIe card I am using only supports rs485 but without setting the flag to enable_rs485 it doesn't work.
The device is an umbratek mpcie rs485 card with the chip a xr17v358 chip

I've embedded the rs485 crate, as its pretty minimal, I am would end up rewriting it all anyway. Modified it to work with libc crate and the latest bitflags crate.

@de-vri-es
Copy link
Owner

Its confusing as the PCIe card I am using only supports rs485 but without setting the flag to enable_rs485 it doesn't work.
The device is an umbratek mpcie rs485 card with the chip a xr17v358 chip

yaaay :p

I've embedded the rs485 crate, as its pretty minimal, I am would end up rewriting it all anyway. Modified it to work with libc crate and the latest bitflags crate.

Sorry for the delay in review. I need to read up a bit to ensure we model this correctly. Sadly so far I can't find anything on how this is done on other platforms :(

@de-vri-es de-vri-es self-assigned this Mar 24, 2024
@de-vri-es
Copy link
Owner

I've been working on an API that I think make sense. Sorry for the radio silence.

This ioctl is another can of worms in the serial communication landscape, so I want to take some time to model it correctly.

What I'm leaning towards now is an API to specifically configure RS-485 and RS-422, where things like the RTS signal state and bus termination is only available in RS-485 mode.

I'm also limiting the exposed settings, since even the Linux kernel actually "sanitizes" the options silently. For example, if you set RTS_ON_SEND and RTS_AFTER_SEND to the same value, the kernel simply sets both of them to pre-determined value:
https://github.com/torvalds/linux/blob/f2f80ac809875855ac843f9e5e7480604b5cbff5/drivers/tty/serial/serial_core.c#L1382-L1399

Also, the MODE_RS422 flag is currently not used by any mainline driver:
https://github.com/search?q=repo%3Atorvalds%2Flinux%20RS485_MODE_RS422&type=code

But one of them interprets RX_DURING_TX as RS-422 mode:
https://github.com/torvalds/linux/blob/f2f80ac809875855ac843f9e5e7480604b5cbff5/drivers/tty/serial/8250/8250_exar.c#L506-L516

But I intend to solve most of this with warnings in the documentation rather than hacks to detect device quirks.

@de-vri-es de-vri-es mentioned this pull request Apr 7, 2024
@omelia-iliffe
Copy link
Contributor Author

No worries about the radio silence, and thanks for continuing to work on it. Its a complicated problem, but your approach sounds good. Looking forward to seeing it. :)

@de-vri-es de-vri-es force-pushed the rs485 branch 4 times, most recently from c7ce4d5 to a199b30 Compare April 9, 2024 13:59
@de-vri-es de-vri-es force-pushed the rs485 branch 2 times, most recently from 8b7dbf8 to c39b738 Compare April 9, 2024 14:31
@de-vri-es
Copy link
Owner

I've pushed some commits to adjust the API. For usage, you can check the rs485 example.

I will still go over the changes again later, when I've had some time to let it sink :)

@DanielJoyce
Copy link

This is nice, but also a lot of people simply emulate this stuff throygh the already available R232 features. Its nice to have, but maybe a comment about bitbanging it yourself over regular serial is also possible if stuff doesn't work.

@omelia-iliffe
Copy link
Contributor Author

I confirmed this works for my setup today and the ergonomics seem nice to me. I noticed a couple of minor mistakes ill fix now.

@DanielJoyce I'm not sure how you could emulate the switching of ioctls with bitbanging. Perhaps you could write the comment as you suggested with an example?

/// Please read all the warnings in the [`rs4xx`] module carefully.
#[cfg(any(doc, all(feature = "rs4xx", target_os = "linux")))]
#[cfg_attr(feature = "doc-cfg", doc(cfg(all(feature = "rs4xx", target_os = "linux"))))]
pub fn set_rs4xx_mode(&self, mode: &rs4xx::TransceiverMode) -> std::io::Result<()> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be mode: impl Into<rs4xx::TransceiverMode>?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could. Only downside is that it means copying the argument instead of passing it by reference. But I'm not too worries about that. So lets do that :)

@de-vri-es
Copy link
Owner

de-vri-es commented Apr 15, 2024

I confirmed this works for my setup today and the ergonomics seem nice to me. I noticed a couple of minor mistakes ill fix now.

👍

I'll wait for your fixes before merging.

@DanielJoyce I'm not sure how you could emulate the switching of ioctls with bitbanging. Perhaps you could write the comment as you suggested with an example?

I think they mean that you can switch the RTS/DTR signals manually in between calls to read() and write(). But I'm not sure if that is enough in all cases. RS-485 and RS-422 require a different transceiver (with different electrical properties) from RS-232. The kernel documentation is very vague in exactly what SER_RS485_ENABLED and SER_RS485_MODE_RS422 really do. But maybe they change the active transceiver to really switch between RS-232 and RS-485/RS-422 for some devices.

(Technically, RS-422 has different electrical properties from RS-485 too, but a RS-485 transceiver should be backwards compatible with RS-422.)

@DanielJoyce
Copy link

RS485 is a two wire protocol as specd. Extra wires are not supported by everyone.

What I mean by bit banging is if your rs485 port is exposing a dtr pin and Linux doing the twiddling for you isn't working, as long as Linux exposes that DTR you can twiddle it yourself

@DanielJoyce
Copy link

Like how currently the crate exposes the pins for rs232 and we can set them high / low manually.

@omelia-iliffe
Copy link
Contributor Author

whoops sorry I thought I had replied. Ready to merge @de-vri-es :)

@de-vri-es de-vri-es merged commit 5e41c45 into de-vri-es:main Apr 23, 2024
49 checks passed
@de-vri-es
Copy link
Owner

No problem! Released as v0.2.22 \o/

@omelia-iliffe omelia-iliffe deleted the rs485 branch April 23, 2024 09:29
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.

3 participants