Roadmap for traits removed on 1.0.0 release

[eldruin]

All traits with unconstrained associated types have been removed as we prepare the 1.0.0 release. We are aware that this will be problematic in the short term but agreed it is the best way forward for embedded-hal (see the reasoning below). This includes these traits, which are available on the 0.2.x versions:

• adc::OneShot
• adc::Channel
• capture::Capture
• digital::IoPin
• pwm::Pwm
• pwm::PwmPin
• qei::Qei
• serial::Read -> embedded-io
• serial::Write -> embedded-io
• timer::Cancel
• timer::CountDown
• timer::Periodic
• watchdog::Disable
• watchdog::Enable
• watchdog::Watchdog

Why

Traits defined in embedded-hal pursue creating an interface for interoperability between generic code (be it generic user code, generic application code, generic device drivers, etc.). When a trait has an unconstrained associated type, it is not possible to write generic code around it. Each side (implementer and user) need to specify which type the associated type will be. If the types match, the both parts can work together, however, this is not truly generic code.

For example, if somebody creates a device driver that receives a CountDown struct, it needs to specify what its Time type should be. If they choose a type coming from fugit, somebody else cannot use this driver if the HAL implementation for the MCU they are using only provides CountDown with Time types defined in embedded-time. It is also not possible for the user to implement CountDown for Time types defined by fugit in a straight-forward way due to the orphan rule. In summary, it is not possible for anybody to start a countdown for a certain duration in a generic way, without it being tied to a particular time implementation and thus forcing everybody to use that one.

In the case of the digital::IoPin trait, some problems about it usage have been surfaced (#340) that have moved us to remove it for the 1.0.0 release as well.

We want to avoid the need for a semver-incompatible embedded-hal release (i.e. 2.0) for as long as possible. As such, the best compromise is to remove the traits before the release and add at least some of them back later.

The way forward

As stated above, we have removed these traits ahead of the 1.0.0 release but plan to add at least some of them back in a future semver-compatible release like 1.1.x, once we find suitable bounds for the associated types. Some problems on these traits have been highlighted as well (see open issues), so removing them also gives us the chance to rethink them.

How to get them back

The most pressing issue is finding suitable bounds for time/duration types which make these traits usable in pretty much any situation. See: #211, #122, #59, #24.

Please have a look at the issues for each of the removed traits/modules for further details.

What to do in the mean time

HALs that implement these traits, have at least the following options:

1. Continue offering implementations using the traits from the embedded-hal 0.2.x versions which should reduce the ecosystem fragmentation and keep any dependent code working.
2. Copy the removed traits into the HALs and keep everything as-is.
3. Offer only inherent methods on the structs.
4. Remove the implementations.

Generic code can do their version of these options as well for now.

Issues specific to the removed traits/modules

• 361

• 358

• 362

• 359

• 360

• 377

• 397

[chrysn]

To both verify my understanding and hopefully augment the discussion, here are some alternatives we'd have to fully removing them:

• Leave things as they are. That leaves users of time-based traits with two options:
  • Require that the underlying clocks provide arbitrary conversions they demand -- say, that they impl From<core::time::Duration> or impl From<fugit::durtion::MillisDurationU32>. (That makes them convoluted to write, but has the nice aspect of allowing build time checks whether some conversion works).
  • Ask that they not only be provisioned with a, say, timer instance, but also with matching instances of the durations they need. A setup call might thus become fn new<T: Timer>(timer: T, five_milliseconds: T::Duration, twenty_milliseconds: T::Duration)
• Find something to demand of the parameter:
  • It be a concrete type. Rather unlikely, given that zero-overhead abstractions on timers will only tolerate uXX equivalent time values at their clock rate, and no single type can be all these.
  • It be a type but parametrized over something. An example might be the fugit durations that have u32 numerators and denominators, and also allow variable-width inner units. This might work on the long run, but has open questions on stability (it'd be pub-using a non-stabilized crate), ergonomics (rather than one parameter the types now have TIMER_NUM, TIMER_DENOM, and it somewhat only shifts the problem because the third parameter would be TIMER_T which would typically be u16, u32 or u64 (or u24?), taking us back to an (albeit simpler) version of the original problem.
  • It could be a custom trait. Becoming stable would mean that no non-provided trait methods could be added, but if one of the trait's prerequisites is fallible conversion from core::duration::Time (or any other agreed-on minimum input), conversions could be added later with bespoke implementations provided by whoever can do than the (possibly inefficient) default implementation.
  • It could be a custom trait that is initially sealed: This would be a middle ground over the last two options, narrowing down what timers do to a few initially provided implementations this crate provides (say, embedded-{,hal}-time and fugit), with an option to eventually unseal it in a minor release when we have a clearer vision of what the requirements are.

Granted, pruning the hard cases is the safest of these, but I'd like to ensure we have thoroughly ruled out the alternatives.

[burrbull]

If we decide to use embedded-time somehow, first we need release 0.13 with bugfixes https://github.com/FluenTech/embedded-time/pull/114

[Sh3Rm4n]

here are some alternatives we'd have to fully removing them:

These are no alternatives to remove them, but rather proposals to add them back later. It is pretty clear from the current situation, that the best design is not clear yet. And to no unnecessarily delay a v1.0.0 release, it is better to just remove them for now. But temporary of course, until all agree on a solution.

[chrysn]

I do think that leaving them unconstrained is an option even for now.

Comparing the two developments under the assumption that we'll find some trait TAT later that we can constrain the associated type to:

a) We remove them now.

1. The trait can not be used.
2. We decide on some TAT.
3. HALs start implementing the trait with the provided TAT.
4. Users start using the trait without further constraints, using the properties of TAT.

b) We leave them unconstarined now.

1. The trait can be implemented for everything it has been so far.
2. Users constrain the type to whatever they require (say, From<core::time::Duration>), and can not use implementations that do not provide that.
3. Traits emerge that are frequently requested and frequently implemented, we group them as TAT and recommend that the associated type implement TAT (and that users will often get away with demanding that TAT is implemented for it)
4. HALs start implementing TAT (if they don't already do; many might already implement all its dependencies and just have to impl TAT for MyTime {}).
5. Some users stick with only demanding that T::At: SomethingTATDependsOn (which all recent HALs provide), new users usually go with T::At = TAT. Some users that made requirements exceeding TAT stick with that (staying limited in the HALs they can use), others find useful conversions and constructors in TAT and pivot there.

Eventually, we still get a very similar ecosystem. The upside of option a is that it doesn't need the additional where T::At: TAT on all trait users that option b entails, but option b is a lot less disruptive.

I'm not saying that option b is the ultimately better, but I do prefer it over option a.

[Sh3Rm4n]

What you are describing in b) is already the situation we have, but it's not really that clear what's the best Time type.

Traits emerge that are frequently requested and frequently implemented, we group them as TAT and recommend that the associated type implement TAT

If we, assuming "we" is the e-h wg, is recommending the best associated type, but never constrains the type itself, because that is not possible without it being a breaking change, what should be prevented to a) not delay unnecessarily v1.0 or b) not introduce a breaking change shortly after v1.0, then it stays a recommendation but not a guarantee.

And anything which is not a guarantee for the driver implementation is rather useless. What should a Time be, and how should I use it, when I want to wait 3 seconds? As a HAL implementor and a HAL user, your described scenario might be ok. But e-h should strive to be useful for HAL implementors, users of these and first and foremost driver implementors.

[Ben-PH]

What about keeping these traits around, but marking them as deprecated with a link to an issue documenting the context of its deprecation and planned re-introduction? If it's deprecated in v1.0.0, should that still allow it's removal in, say, v1.1.0, without breaking semver?

[Dirbaio]

No, removing public API is always a breaking change, regardless of whether it was marked as deprecated or not.

[Ben-PH]

I had a look at the semver documentation, something I really should have done first, sorry.

I initially read your response wrong (read it as something like "removal by means of deprecation" rather than "removal of an api-function, whether or not it's ever been marked as deprecated". That's on me), and thought "that couldn't be right", and spent a stupid amount of time going down the rabbit hole, writing a response, before realising my mistake in interpretation...

...but I learned a few things along the way :)

Though this is still a v0 crate in its current state, so the normal rules won't apply until 1.0.0, the semver spec is still a valuable guide in the meantime, IMHO. In the FAQ, it makes the recommendation:

When you deprecate part of your public API, you should do two things: (1) update your documentation to let users know about the change, (2) issue a new minor release with the deprecation in place.

(1) is satisfied in CHANGELOG.md, but (2) would need a 0.3.0 release, if the pre-release exemption didn't exist.

How about a 0.3.0 release with the items deprecated, and a note that it's a pre-release deprecation for v1.0.0? Obviously not needed, even if the semver rules are being strictly followed. It does, however, keep with the spirit and intent of the specification.

One idea that comes to mind, which I hope isn't as stupid as a fear it might be: Do a deprecation release (0.3.0) now(-ish), and then immediately before releasing 1.0.0, release a 0.4.0 which is essentially the same as 1.0.0, but without completely removing the deprecated items (where they don't conflict too heavily). I'm happy to put in the time to make the 0.3.0 happen, and if the 0.4.0 idea isn't so stupid, I'm happy to help with that as well.

[eldruin]

I do not think publishing a 0.3.0 or 0.4.0 version would help anybody since 0.X versions are traditionally understood as breaking releases and thus users would not see the warnings automatically, which would be the only practical benefit. Publishing version 1.0.0 is just as breaking so we should stick with that.