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

Naming and design tweaks #18

Merged
merged 8 commits into from
Oct 23, 2024
Merged

Naming and design tweaks #18

merged 8 commits into from
Oct 23, 2024

Conversation

de-vri-es
Copy link
Member

@de-vri-es de-vri-es commented Oct 12, 2024

Global overview:

  • Although I liked the idea of not exposing too much time details in the trait, having a stateful timeout requires the Transport implementation to always store the deadline. This means that you basically always need a wrapper type to implement the Transport trait.

    So instead I made the timeout non-stateful so we can implement the trait directly for serial2::SerialPort.

    This also opened the way for renaming the trait to SerialPort again.

  • I didn't want the SerialPort impls to return dynamixel2 errors, but only their own error. So I removed the Timeout error variant and instead added a static is_timeout_error() function to the SerialPort trait. Similarly I removed the InitializeError and just have the functions return SerialPort::Error.

So far, these are the high level changes. I also want to rename Bus, Device and Messenger, but I'm out of time for today :p

@omelia-iliffe: Could you look over the changes and see what you think?

@omelia-iliffe
Copy link
Contributor

Awesome. This all looks great. I see what you meant about having an Instant type now. I was worried I wouldn't be able to make this work with embedded platforms but I found solutions for both rp2040-hal and stm32g4xx_hal I am targeting.
You can see the updated rp2040 example here: https://github.com/omelia-iliffe/dynamixel2-rs/tree/no_std_example/examples/rp2040. It compiles and I will test it when I get a chance.

src/device.rs Outdated Show resolved Hide resolved
@de-vri-es de-vri-es merged commit 160a0f5 into main Oct 23, 2024
1 check passed
@de-vri-es de-vri-es deleted the bike-shedding branch October 23, 2024 12: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.

2 participants