rubberduck203 / switch-hal

Rust no_std embedded-hal driver and traits for Input/Output switches (leds, transistors, buttons, switches)
Apache License 2.0
27 stars 4 forks source link

Consolidate `into` functions #9

Closed rubberduck203 closed 4 years ago

rubberduck203 commented 4 years ago

@dzarda what do you think of this?

dzarda commented 4 years ago

This is the right thing to do for sure, but I've run into a small limitation.

One is allowed to write

let sw = 5.into_active_low_switch();

which hardly seems ideal. Of course subsequent method calls fail when attempted on sw, but I feel like creating the instance should be forbidden in the first place.

Or do we need to somehow constrain IoPin in Switch itself?

rubberduck203 commented 4 years ago

which hardly seems ideal

Indeed.

Or do we need to somehow constrain IoPin in Switch itself?

Regardless of whether or not it solves this issue, yes. I attempted it before, but ran into “implicit dyn is deprecated” errors and later, “duplicate implementation” issues. I think your PR helped me understand the prior, so I can take another stab at it.

I think I’ll merge this and track it in another issue. Thank you for the review!

rubberduck203 commented 4 years ago

Or do we need to somehow constrain IoPin in Switch itself?

Yes. Yes we do. This compiles just fine.

let switch = Switch::<_, ActiveHigh>::new(5);

But when then later fail when it the compiler fails to find a trait impl for Switch::<i32, ActiveHigh>.