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

sys/termios: add cross-platform API for arbitrary baud rates #1632

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tmplt
Copy link

@tmplt tmplt commented Jan 13, 2022

This commit adds

pub struct ArbitraryBaudRate(pub u32);
impl TryFrom<ArbitraryBaudRate> for BaudRate { ... }

such that arbitrary baud rates can be specified on systems that
otherwise do not support it (e.g. Linux) via abstraction.

The one problem I see with this implementation is the duplication of the
BaudRate enum. Removing the duplication would require a new macro, which
may be too complex for this feature to warrant implementation.

@tmplt tmplt force-pushed the feat/termios-linux-arbitrary branch from 4685d61 to e8b9064 Compare January 13, 2022 12:50
This commit adds

    pub struct ArbitraryBaudRate(pub u32);
    impl TryFrom<ArbitraryBaudRate> for BaudRate { ... }

such that arbitrary baud rates can be specified on systems that
otherwise do not support it (e.g. Linux) via abstraction.

The one problem I see with this implementation is the duplication of the
BaudRate enum. Removing the duplication would require a new macro, which
may be too complex for this feature to warrant implementation.
@tmplt tmplt force-pushed the feat/termios-linux-arbitrary branch from e8b9064 to 1543868 Compare January 13, 2022 12:58
@tmplt tmplt marked this pull request as ready for review January 13, 2022 13:11
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Why add the ArbitraryBaudRate structure? I think implementing TryFrom<u32> directly on BaudRate should be fine.

@tmplt
Copy link
Author

tmplt commented Jan 22, 2022 via email

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.

None yet

2 participants