Tighten the type constraints

[rubberduck203]

This compiles just fine.

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

It will later fail when you try to call one of the {InputSwitch,OutputSwitch} functions.

error[E0599]: no method named `is_active` found for type `switch_hal::Switch<{integer}, switch_hal::ActiveHigh>` in the current scope
  --> tests/input.rs:18:20
   |
18 |             switch.is_active().unwrap();
   |                    ^^^^^^^^^ method not found in `switch_hal::Switch<{integer}, switch_hal::ActiveHigh>`
   |
   = note: the method `is_active` exists but the following trait bounds were not satisfied:
           `switch_hal::Switch<{integer}, switch_hal::ActiveHigh> : switch_hal::InputSwitch`

However, it probably shouldn't even let the first one compile.

This was first brought up in #9 and I believe requires an architectural decision record. Because InputPin and OutputPin are traits, it's not possible to tag them with marker trait to constrain Switch to just those two types. This results in conflicting trait implementations.

The alternative is to split Switch into two different structs, and suffer the code duplication. This option would also require two new names, as InputSwitch and OutputSwitch are already taken by the traits.

[dzarda]

What's the point of having the Switch -> {InputSwitch, OutputSwitch} hierarchy anyway? What duplication is avoided?

Seems to me that things could be simpler with complete input/output separation:

InputSwitch<T, ActiveLevel>
OutputSwitch<T, ActiveLevel>

[rubberduck203]

Sorry it's taken me so long to respond. I've been away for a moment.

This is the code I'm trying to avoid duplicating. https://github.com/rubberduck203/switch-hal/blob/785caad34a228f3d141cb79421409f638dd6cfa2/src/lib.rs#L91-L173

In order to "automatically" implement the InputSwitch and OutputSwitch traits for people, we need a concrete struct that holds an InputPin or OutputPin.

https://github.com/rubberduck203/switch-hal/blob/785caad34a228f3d141cb79421409f638dd6cfa2/src/lib.rs#L98-L101

This code is identical whether we're looking at an Input or Output and so are the new and into_pin methods.

https://github.com/rubberduck203/switch-hal/blob/785caad34a228f3d141cb79421409f638dd6cfa2/src/lib.rs#L148-L153

https://github.com/rubberduck203/switch-hal/blob/785caad34a228f3d141cb79421409f638dd6cfa2/src/lib.rs#L170-L172

If we do something like you suggest, we will need individual structs with identical implementations of new and into_pin for not just v2::InputPin and v2::OutputPin, but also v1::InputPin and v1::OutputPin (which I've yet to add).

The current design is discussed in this adr, but I didn't realize at the time that things like 5.into_active_high_switch() would compile only to fail down the chain trying to find an implementation.

There are a couple of options as I see it.

1. Create independent structs for each pin trait and suffer the duplication (perhaps use an internal macro to DRY it out).
2. Forget the blanket implementation and have each HAL provide an impl for their already existing structs.
3. Do nothing and allow things like Switch::<_, ActiveLow>::new(5) to continue compiling.

Each has their own pros and cons. I was hoping to write up an adr this weekend, but it will probably be next weekend before I get around to a proper analysis.

[dzarda]

What you're saying is all true. A few points:

• The embedded-hal group decided to remove the v1/v2 versioning so this complexity issue is no longer there
• I tried to do some constraining magic a few weeks ago, but didn't get anywhere. Someone more experienced would be of help
• From the options you listed I don't think 2. is a way to go. I strongly believe this crate should not incur any impl complexity onto the consumer. Otherwise this won't be much help.

[rubberduck203]

Revisiting this over coffee this morning and I think either we need to get the type constraints right (IoPin where or something) or use an internal macro to remove the duplication. The other options don’t seem acceptable.

[rubberduck203]

Note to self: It’s been 4 years. Are there any new rustic features that enable this?