rust-embedded/embedded-hal

Ship a 1.0 release

thejpster opened this issue ยท 73 comments

thejpster commented

Part of rust-embedded/wg#383

Settled blockers:

Open blockers:

  • #547
  • #553 ?
  • #555
  • Show that MCU and linux implementations work fine
  • Show that drivers can work fine with the new changes
  • Your issue here!

Feel free to nominate issues also by commenting below

jamesmunns commented

Here is a link to the video call discussion had by myself and most of the members of @rust-embedded/hal team. This includes initial decisions made, and justification/reasoning for each point. The next steps for this would be to codify this into an RFC, and get approval for it (though there is already general consensus within the embedded-hal team).

by the way, this was the "issue with the list of big topics" we were looking for during the meeting :)

eldruin commented

I believe the only unresolved issue as of the list above is #172
I would also add the dependency on rust-embedded/nb#20
Then we need to think about the dev-dependencies which are at the moment stm32f3 and futures. I guess specially these will hold us back for a while.

Disasm commented

This async story began just recently and I do not expect it to end in the coming year. Why should we block on it? We can always add this later. In the worst case, these async traits will be released in embedded-hal v2.0.0.

  • ๐Ÿ‘5
therealprof commented

I also think we should keep async separate because it's a different paradigm and might need require a different approach anyway. I can imagine taking an approach similar to async-std vs std, i.e. have the async traits in a separate crate async-embedded-hal which would allow crates to move indepedently and express more clearly what is supported where.

  • ๐Ÿ‘7
ryankurte commented

i have mixed feelings about async being separate in the long term, but imo it very much depends whether we can find an approach (as is the case for one of the demonstrated ones) that allows everything to be a default impl on top of our base nb or poll traits as we have now with the blocking default methods, so we have one source of truth for integration, simplify the job of hal implementers, and you'll always have access to both. Or if these introduce fundamentally incompatible requirements and must be managed separately. this, however, is not something we can conclusively tell afaik.

i posed here (#172 (comment)) that if we have a path towards a decision on the nb/futures swap I would be okay to hold, but so long as it's an open / unknown problem I would also prefer to keep moving and accept integrating async may be another breaking change down the line.

(i would also very much like to hear the thoughts of @japaric if ya have a moment to look at all of this)

  • ๐Ÿ‘3
hannobraun commented

I'd like to nominate #249 as a blocker for the 1.0 release.

hannobraun commented

@rust-embedded/hal Would you consider releasing 1.0-alpha.2? I'm working on a driver that I'd like to release soon, and it would be great if I had access to PinState.

  • ๐Ÿ‘1
ryankurte commented

Thanks for the reminder, shipped in #255

  • ๐Ÿ‘1
hannobraun commented

I've recently learned something that I'd like to share, in case it isn't common knowledge yet: A cargo update will pick up alpha.2, if alpha.1 is specified in Cargo.toml. I think this is problematic, as new alpha releases might introduce breaking changes. HAL crates that would like to test-drive the alpha can't do so while guaranteeing semver stability.

Maybe it would be best to yank the existing alpha releases and test the new changes in a 0.3 series instead? As far as I can see, the alternative would be breakage, inhibited adoption, or additional hoops to jump through (like hiding all alpha implementations behind a feature flag).

eldruin commented

I believe an exact version should be needed for alphas. i.e. embedded-hal = "=1.0.0-alpha.2"

  • ๐Ÿ‘3
hannobraun commented

Oh yeah, for a moment I completely forgot that you could do that. Seems to be a good way to work around the issue! (Although in general, the situation is still not ideal, as not everyone will remember to do that. Still, probably not worth yanking and re-releasing.)

dbrgn commented

Yeah, plus if you use alpha versions for production code, you should be aware that things might break at any time ๐Ÿ™‚

hannobraun commented

Yeah, plus if you use alpha versions for production code, you should be aware that things might break at any time

But that issue affects not only those running the alpha, but everyone using a HAL that supports it in addition to the 0.2.x version. Of course, you might say that HALs intended for production use shouldn't support the alpha version, but then what is the point of an alpha, if it can't be used? :-)

In any case, @eldruin's workaround is good enough, so I'm fine.

  • ๐Ÿ‘1
therealprof commented

But that issue affects not only those running the alpha, but everyone using a HAL that supports it in addition to the 0.2.x version. Of course, you might say that HALs intended for production use shouldn't support the alpha version, but then what is the point of an alpha, if it can't be used? :-)

I don't think you can support multiple versions at once. We do have a few implementations with a non-released separate branch for the alpha versions but I'm not worried about breakage there.

david-sawatzke commented

I don't think you can support multiple versions at once.

No issue there, see (and various impls of both in master):

https://github.com/lpc-rs/lpc8xx-hal/blob/6e787f8e26d908b3cea13c3c47f04695f4112837/Cargo.toml#L40

  • ๐Ÿ‘3
rumatoest commented

Hello. I think that you must address such issue as #29
There are many MCUs that do not have open-drain like pins (like Arduinos), but it is still possible to "emulate" similar behavior to communicate with devices via only single data line.
Basically it is mostly about combining two traits together OutputPin + InputPin considering that each HAL implementation will perform under the hood magic while calling function from different traits.

David-OConnor commented

Thoughts on ditching try_ before this goes live? It clutters syntax, isn't consistent with other libs, and doesn't provide much value.

  • ๐Ÿ‘3
Dirbaio commented

I think the try_ naming is a mistake.

  1. It is noisy
  2. It is inconsistent with the rest of the Rust ecosystem, therefore unidiomatic. For example, all IO in std is fallible, yet doesn't have try_.
  3. AFAICT, the only reason for try_ is avoiding naming collisions if infallible traits are ever introduced, but:
    • Infallible traits can be achieved by impls having Error = ! (or core::convert::Infallible)
    • Infallible traits are a bad idea anyway, IMO:
      • If drivers use infallible traits, it fragments the ecosystem.
      • If drivers stick to fallible traits, no usability improvement is achieved.
      • HAL crates will have to implement both the infallible and fallible traits, unless a blanket impl is done. Such blanket impl can be done in an external crate, there's no advantage in having these in embedded-hal.
      • It makes the API more complicated because now there's two ways to achieve the same thing with non-obvious (or no) tradeoffs. It makes the docs harder to read because everything is duplicated.
    • If infallible traits were still introduced, naming collisions wouldn't be that bad honestly. You either import the fallible or infallible trait, and go use the methods. No code will use both at the same time, realistically.
    • It is very unlikely that infallible traits will be introduced for most traits. The only ones I kindof see happening is GPIO.

I think the way to go is keep the decision of having only fallible traits, but name the methods foo instead of try_foo.

  • Less visual noise
  • Consistent within the embedded-hal crate
  • ... but also consistent with the rest of Rust.
  • ๐Ÿ‘13
reneherrero commented

I have to agree. Unnecessary as Rust won't let you mistake the return type.

therealprof commented

@Dirbaio Thanks for your writeup.

I almost fully agree with your assessment. However there is precedence for methods which are using try_ to differentiate from the previous infallible methods like try_fold vs fold. Fallible methods in e-h were always a mistake IMHO, but some people have (maybe had?) strong opinions about keeping the door open to go infallible without cognitive and computational overhead and having the fallible methods to include try_ is the idiomatic way to achieve that.

The reasoning from the critical meeting where this decision was made can be read here: https://hackmd.io/ck-xRXtMTmKYXdK5bEh82A?view#Final-Summary

I think the best way to achieve that would be for someone who deeply cares to create an RFC so people can properly weigh in and we can have a vote in the end.

Rahix commented

I want to make a point for keeping the try_ prefix. I came to like it quite a bit while building a HAL against the alpha-version because it allows a HAL-API design that clearly discerns the generic interfaces from the hardware-specific ones.

Generally an inherent problem with an effort like e-h is that the provided API must be a one-fits-all design. This of course leads to decisions like having every method fallible. Such things are absolutely necessary to ensure drivers written against e-h will work in every situation, but it limits the ergonomics for code that is written against a specific hardware. For example you have fallible methods for setting pins which is super strange on a microcontroller where it obviously cannot fail. No matter whether the method is called try_set_high() -> Result<> or set_high() -> Result<>, the problem is the same.

I think the solution to this is to not view the e-h API as the general HAL API users are meant to use. Instead, a HAL crate should provide its own hardware-specific API for direct users and implement the e-h traits alongside for generic drivers to interface with.

I think this is a useful Rust pattern in general which especially applies here: A type should implement its full featureset as an API consisting of inherent methods (impl Type {}) and then implement appropriate traits (impl Trait for Type {}) ontop of this. Using a trait for the primary API has its uses (e.g. io::Read) but comes with the huge drawback of locking everyone into a set API that might not fit them perfectly (see fallible vs. infallible). I think especially in the heterogeneous world of MCU peripherals, it is important that every HAL can make its own API decisions here.

As such I think it is wrong to use the e-h traits for anything else than enabling generic drivers that work across HALs. In particular using them as the main way to interact with the hardware from application code is not a design which scales in my opinion.

To put it into more concrete terms, here is a GPIO pin implementation for a HAL which leverages such a design. I have a HAL-crate locally where I explored this in more detail, if there is interest, I can publish it.

pub struct Pin<...> {}

impl<...> Pin<Output> {
    pub fn set_high(&mut self) {
        // non-fallible method for direct users
    }

    pub fn set_low(&mut self) {
        // non-fallible method for direct users
    }
}

impl<...> embedded_hal::OutputPin for Pin<Output> {
    pub fn try_set_high(&mut self) {
        // generic method for generic drivers
        self.set_high();
        Ok(())
    }

    pub fn try_set_low(&mut self) {
        // generic method for generic drivers
        self.set_low();
        Ok(())
    }
}

This allows direct code to look clean; no cluttering by try_ prefixes nor any handling of impossible errors (the never ending let _ = pin.set_high();, pin.set_high().void_unwrap();, or pin.set_high().ok();). Additionally, no magic traits need to be imported into scope first, because the API is made from inherent methods.

#[entry]
fn main() -> ! {
    // ...

    loop {
        pin.set_high();
        d.delay_ms(100);
        pin.set_low();
        d.delay_ms(100);
    }
}

But it also allows writing generic drivers which can work with any HAL, fallible or not, and such driver code will clearly show that it is able to handle errors by having the try_ prefix visible:

impl<PIN: OutputPin, DELAY: DelayMs> BlinkyDriver<PIN, DELAY> {
    pub fn blinky(&mut self) -> Result<!, SomeError> {
        loop {
            self.pin.try_set_high()?;
            self.d.try_delay_ms(100)?;
            self.pin.try_set_low()?;
            self.d.try_delay_ms(100)?;
        }
    }
}

In more complex situations, the benefit of the HAL being able to define its own API is even greater. Suppose peripherals like a SPI controller which often have custom features the HAL might want to expose. A design like outlined above places no limitations on doing this while still keeping the generic layer supported alongside (with reduced featureset) so generic drivers can be "connected" with zero effort (one of the big benefits of embedded rust in my eyes).

  • ๐Ÿ‘8
David-OConnor commented

I agree with all Rahix said re how to use EH, and how that mitigates try_ by reducing the number of places it occurs, and why fallibility doesn't seem like a good fit for hardware-infallible pins but is fine etc. I don't see how that justifies the try_ syntax over the alternative of its omission.

therealprof commented

This allows direct code to look clean; no cluttering by try_ prefixes nor any handling of impossible errors (the never ending let _ = pin.set_high();, pin.set_high().void_unwrap();, or pin.set_high().ok();). Additionally, no magic traits need to be imported into scope first, because the API is made from inherent methods.

BTW: If everything is fallible you can also chain calls rather than handling (or ignoring) the possibility of failure on each of them individually...

self.pin.try_set_high()
    .and_then (|_| self.d.try_delay_ms(100))
    .and_then (|_| self.pin.try_set_low())
    .and_then (|_| self.d.try_delay_ms(100))

I do think the ability to combine peripheral operations with the incredible Result API is a big feature, not a bug. And compared to the times of thetry! macro of the early days this is constantly becoming more and more convenient to use every other Rust release.

Dirbaio commented

@therealprof: However there is precedence for methods which are using try_ to differentiate from the previous infallible methods like try_fold vs fold.

As outlined in my previous post, try_ would be idiomatic if we had fallible and infallible traits. However, we don't have them right now, it's unlikely that we'll have them in the future, and IMO it's a bad idea to have them.

@Rahix: I came to like it quite a bit while building a HAL against the alpha-version because it allows a HAL-API design that clearly discerns the generic interfaces from the hardware-specific ones.

You're saying the try_ prefix is good because it allows distinguishing embedded-hal methods from HAL-speficic methods. I disagree this is a good thing, in fact I think it's yet another symptom of the prefix being the "wrong" choice:

In idiomatic Rust APIs, try_ is supposed to distinguish fallible from infallible. As a user, if I see a struct has try_foo and foo methods, I expect try_foo to return Result<X, Error> and foo to return X (and panic/block/etc on failure).

However, HAL methods are likely to be fallible too! For example, it's likely that a HAL Uart will end up with a HAL-specific read() method returning Result and the e-h try_read() method returning Result too. This is inconsistent and unidiomatic. As a user, if I see read and try_read I'd expect read to be infallible.

Maybe the HAL-specific method should be named try_read() too for consistency, but then the "advantage" of being able to distinguish HAL vs e-h methods by the try_ prefix is gone.

As an aside, if we were to keep the try_ prefix, the traits themselves should be named TryFoo too, right? See From + from vs TryFrom + try_from. This is yet another reason the current naming is unidiomatic.

  • ๐Ÿ‘2
Dirbaio commented

This has been discussed in today's Rust Embedded meeting.

           Dirbaio:  maybe the real question here is "do we want infallible versions of the traits or not"
           Dirbaio:  if yes, then try_ makes sense (but traits should be renamed to TryFoo too)
           Dirbaio:  if not, then try_ should be removed
             rahix:  I didn't get around to reply yet, but I do see the point Dirbaio made. I would be okay with removing the prefix as well, my point was related but not at its core a reason to keep try_.
           Dirbaio:  so maybe to unblock the issue, the discussion of "do we want infallible versions of the traits or not" should be had now, not posponed? 
            Lumpio:  IMHO the traits are most useful for building generic drivers, and I don't see infallible traits being a thing there if we want fallibleness ever. Which I think is a good thing. HALs can add infallible methods in their concrete implementations if they want, which can even have the same names because of how traits work.
         adamgreig:  having the HAL provide infallible methods for end-users and implement e-h to provide infallible seems like a really nice way to keep the HALs nice for direct end users to use while keeping e-h suitably generic for drivers
           Dirbaio:  IMO postponing the discussion of infallible traits, but adding the try_ naming "just in case" is not the way to go
           Dirbaio:  if the answer is "we don't want infallible traits" then we're stuck with bad naming forever for no reason
         adamgreig:  what's the argument for infallible types in e-h?
            Lumpio:  And hopefully in the Future Rust will be expanded so that you can just ignore Result<T, !>, and use the fallible methods in app code without any extra boilerplate or warnings.
             rahix:  From a technical standpoint I think infallible traits don't get us much at this point, the performance will be the same.
       therealprof:  re adamgreig "having the HAL provide infallible...": Not sure how you would do that. That'd bring you back right to the "no panic" issue.
            Lumpio:  I think adamgreig might've means "implement e-h to provide fallible"
            Lumpio:  meant*
         adamgreig:  sorry yes, second one should be fallible
         adamgreig:  same point Lumpio is making
           Dirbaio:  I haven't seen any convincing argument for having infallible traits in e-h to be honest
           Dirbaio:  usually it's "ease of use"
         adamgreig:  ease of use" seemed appealing until rahix pointed out we don't need it in e-h
             rahix:  yeah, that was exactly my point
           Dirbaio:  but it can be achieved by having HALs add infallible / friendlier methods, it can be achieved by wrapper traits in external crates: impl InfallibleOutputPin for OutputPin<Error=!>
           Dirbaio:  and the disadvantages are many (see my comment)
       therealprof:  I think the main proponent of the infallibe traits should be lurking here on the channel. \U0001f609 
           Dirbaio:  who is it?
       therealprof:  Not going to out anybody here if they don't want to speak up.
           Dirbaio:  great! no infallible traits then \U0001f61c
       therealprof:  Yeah, I guess that's what it's going to be.
             rahix:  would it make sense to have a vote for this or is it uncritical enough to just make a decision right now and here?
            Lumpio:  (IRC seems to have died to switching to Matrix for a while)
       therealprof:  Well, one of the team members is not present due to timezone issues.
 firefrommoonlight:  I took rahix's advice and use infallible gpio methods in HAL. I bring this up, since that's an option when using HALs directly, ie you aren't dependent on the EH trait. For generic drivers, you are, and I don't have an opinion.
         adamgreig:  perhaps the best option is PR making the name change and the hal team can vote on it? it's their call in the end, and at least the pr is hopefully fairly quick to put together
         adamgreig:  sed s/try_//
 firefrommoonlight:  I'm treating EH traits as a bolt-on to HAL - not the HAL's primary API
       therealprof:  Let's do that.
         adamgreig:  re firefrommoonlight "I'm treating EH traits as a bolt-on to HAL - not the HAL's primary API": yea, this shift in perspective has definitely brought me around to not needing infallible traits in e-h, personally
AlyoshaVasilieva commented

Can I nominate #201? E-H CountDown timers can't really be used generically right now, because the choice of Time type varies heavily:

Right now a generic driver that needs to use a timer in some way has to ask a user for both a timer and as many different time values as it may need. (Or if it only needs one value, maybe ask a user to preconfigure a Periodic timer)

  • ๐Ÿ‘1
eldruin commented

@AlyoshaVasilieva #201 certainly highlights a problem but at the moment there are no concrete PR proposals to fix it. I think this will require more time to iron out and would not include it in 1.0.
In any case, please do not feel discouraged. We would welcome proposals on how to fix it.

Dirbaio commented

Perhaps CountDown should be removed before 1.0, and added later when fixed then? The current trait is useless in generic drivers, and fixing it is a breaking change.

There are other traits with completely unconstrained type Time, which I think suffer from the same problem.

  • CountDown
  • WDT
  • Capture
  • PWM

Are there any generic drivers using them?

  • ๐Ÿ‘1
eldruin commented

We should not forget that embedded-hal is useful not only for generic drivers but also as a common abstraction for MCU HALs. Thanks to embedded-hal it should be easier to change between MCUs.
If you find no HALs using a particular trait, removing it initially and readding them later on would also be possible.

David-OConnor commented

In that regard, it provides a standard interface.

I've chosen set_freq(freq: f32) in my code bases, where freq is in hertz. This is an arbitrary choice over period: f32, where period is in seconds. Note that this interface doesn't make sense for non-FPU MCUs, except perhaps in initial setup as a once-off.

David-OConnor commented

I agree with Dirbao's proposal. Timers are too varied in features and use to map well to a most-common-features-only trait. I make liberal use of them, and have never found a use for a blocking, non-configurable API. I don't think the common abstraction use case is a justification on its own - the implied question is, as "Are there any generic drivers using them", "Are there any code bases that use these HAL-provided APIs?"

As an extension to Dirbao and eldruin's most recent posts: embedded-hal I2C, SPI, UART etc APIs are general; they provide a good example for how to set up an API for those peripherals. Countdown doesn't.

  • ๐Ÿ‘1
hannobraun commented

Are there any generic drivers using them?

Stepper is using timer::CountDown, but with a where clause that requires conversion from embedded_time::duration::Nanoseconds to CountDown::Time to be possible. I've extended LPC8xx HAL to support this, but I'm not aware of any other HALs that do so.

By itself, timer::CountDown is not very useful.

  • ๐Ÿ‘€1
AlyoshaVasilieva commented

https://github.com/dbrgn/espresso is an incomplete driver for interfacing with ESP devices running the ESP-AT firmware. ESP-AT firmware is designed to let you add an ESP microcontroller to a project and give your already-in-use microcontroller ability to use WiFi (plus other things). espresso expects timers to have Time: From<u32> and interpret it as milliseconds, which will fail for most HALs, whether due to it not being supported or because it interpreted 1000u32 as 1000Hz (1ms) or 1000ns (0.001ms).

It's built on an old version of atat; the current version uses CountDown timers in the same way.

I also have a never-finished driver for interfacing with devices running ESP-AT firmware; the idea was to be able to add WiFi/maybe Bluetooth to any E-H UART-capable MCU by adding an ESP32. While working on it I found a bug in ESP-AT firmware where it would crash and stop responding forever (until RESET pin was pulled low). I added a 10s timeout using a Timer with core::time::Duration and then found out that:

  1. Only the HAL I was using had defined Time like that
  2. None of the other HALs had agreed on a common Time unit, and STM32 HALs couldn't handle >1s times due to using Hertz instead. An STM32 was the next board I was going to try it with.

So I stopped working on it and moved on to other things. (I planned to come back and keep working once ESP-AT bug was fixed, but didn't; pretty sure it would still need a timer)

I didn't need any advanced features of a timer, just to be able to say 'start timing ten seconds' and 'has that time passed?'

David-OConnor commented

u32 milliseconds isn't much better than STM32fxx's Hertz: In this case, it's limited to no higher than 1Khz, which is a non-starter for many uses. On FPU-MCUs, thoughts on f32 seconds or Hz? One caveat is some people prefer a struct or enum wrapper, ie to avoid errors such as timer.set_freq(temperature).

Sh3Rm4n commented

I think time / duration / frequency related interfaces should be postponed and not be included in the 1.0 release as it is very obvious from the discussion above that the current abstractions are not useful enough on its own and the community has to further explore what the best solution is.

There are some solutions, like using f32 / f64 based timing values or generic approaches, like embedded-time or other proposals I've missed.

But my feeling is, that as of now, we do not have enough experience with those approaches to consider including them into embedded-hal.

Dirbaio commented

I prefer no floats, because:

  • Not all MCUs have FPUs
  • Even if you do have an FPU, you may want to not enable it to save power (I use the nrf52840 like this, for example).

I prefer "time units" (seconds/milliseconds/microseconds/ticks/whatever) to "frequency units" (hertz), because;

  • With Hz you can't delay for 2s, or for 0.75s (unless you use floats or fractions)
  • (almost?) all MCUs timers specify the "time to delay for" in time units (number of ticks) instead of frequency units. Converting from frequency to time requires expensive reciprocals (1/x), vs converting between differnt time units just requires multiplying by constants.

Either way, I guess our options are the following:

  1. Delay 1.0 until "how to handle times" is resolved.
  2. Release 1.0 as-is, knowing that we will have to do breaking changes to fix this later.
  3. Remove all traits that use time before releasing 1.0, add them back later when resolved.

Traits that use time that would ahve to be renamed

  • delay: DelayMs, DelayUs
  • timer: CountDown, Cancel
  • WDT
  • Capture
  • PWM

I think option 3 is best.

  • ๐Ÿ‘7
adamgreig commented

Seems like we could keep DelayMs and DelayUs given they specify precisely their integer units?

  • ๐Ÿ‘2
Dirbaio commented

True, they don't suffer from the problem. If we leave them in then they'll probably end up being inconsistent with how time handling works in the rest of the traits, which would be a shame....

David-OConnor commented

I agree that delay_ms(ms: u32) and delay_us(us: u32) are nice APIs for short blocking delays, eg cortex-m systick. delay_ฮผs(ฮผs: u32) I'm less sure about.

I prefer "time units" (seconds/milliseconds/microseconds/ticks/whatever) to "frequency units" (hertz), because;

* With Hz you can't delay for 2s, or for 0.75s (unless you use floats or fractions)

* (almost?) all MCUs timers specify the "time to delay for" in time units (number of ticks) instead of frequency units. Converting from frequency to time requires expensive reciprocals (1/x), vs converting between differnt time units just requires multiplying by constants.

I buy that, but still find it arbitrary if using floats. I've now added set_period(time: f32) to my timer APIs, so you can do either. (time is in seconds.)

eldruin commented

Should we rather have the units in the trait names everywhere?

  • DelayMs, DelayUs, DelayNs
  • CountDownMs, CountDownUs, CountDownNs
burrbull commented

I buy that, but still find it arbitrary if using floats. I've now added set_period(time: f32) to my timer APIs, so you can do either. (time is in seconds.)

16777216.0 + 1.0

burrbull commented

Yet one pre-release maybe?
cc @eldruin

eldruin commented

@burrbull Seems appropriate, yes. -> #327

Dirbaio commented

I would like to nominate #264 as blocking for 1.0.

  • ๐Ÿ‘3
adamgreig commented

We had a brief chat about 1.0 in the meeting today and two interesting points were raised:

  1. Should we remove the nb traits entirely for 1.0? It's not clear that they're especially useful or widely used, many users report confusion with them, and ultimately the async traits should fully replace nb. Plus, we could always add them back, but we can't ever delete them later if they're included in 1.0.
  2. Could we keep embedded-hal-async a separate crate forever, with embedded-hal just containing the common types/errors and the blocking traits? It would mean we could remove the ::blocking sub-modules which simplifies the crate, but we couldn't easily merge async traits back in the future.

Both worth resolving before we release 1.0.

  • ๐Ÿ‘7
burrbull commented
  1. we could always add them back

Could we? If we plan to replace them with asyncs.

Move nb traits in other crate like async traits now?

rumatoest commented

I guess there is no need to depend on nb because of async. For compatiblity nb should be in separate trait that probably will be deprecated one day or even right after it will be released :)

thejpster commented

How would you do non-blocking calls without the traits where the methods return nb? It's entirely valid to spin in a reactor loop without using async/await syntax, and without using Futures.

JanBerktold commented

Hi! What is the missing work to drive this to completion? How can folks help?

  • ๐Ÿ‘€7
eldruin commented

@JanBerktold There is a list of open blocker issues/PRs in the top post.
Once those are settled, we can release 1.0.
Feel free to participate in any of them.

tgross35 commented

What exactly is needed by "Show that drivers can work fine with the new changes"? I think this may be difficult short of actually porting the drivers, but is it worth reaching out to driver/PAC authors of major drivers to ask them to review?

Same goes for "Show that MCU and linux implementations work fine", that's a bit abstract.

It seems like a release must be right around the corner since the only blocking thing with an open issue is docs. Where is help needed to push this over the edge?

  • ๐Ÿ‘2
eldruin commented

Those mean porting a variety of drivers and MCU and linux HAL implementations to the -rc.1 version and reporting if there are any problems or not.
That is where I see help needed the most, since it is based on community work.

  • ๐Ÿ‘1
pbert519 commented

What exactly means a variety of drivers and HAL implementations?
Is it a number? A percentage? A list of important crates? A feeling?
The goal is not measurable unless there is some kind of definition.

Finomnis commented

I would in general be interested in starting a migration for imxrt-hal. The important open question to me is: How stable is the embedded-hal 1.0 API already? How much can we expect it to change? Is it intended to stay as-is and only change if we discover problems? Or are there still open design decisions?

Dirbaio commented

1.0-rc1 should be identical to the final 1.0, unless some problem is discovered.

tgross35 commented

Is there a target release date? I think crates are probably more likely to start a port in seriousness (and discover bugs) if there is a solid plan to release before the end of the year, barring anything fatal. After all, some crates have had open issues for switching to 1.0 for three years atsamd-rs/atsamd#332

  • ๐Ÿ‘4
tgross35 commented

esp-hal already supports this rc esp-rs/esp-hal#926

tgross35 commented

Just searching around https://grep.app/search?q=embedded-hal%5Cw%2A%20%3D.%2A1%5C.0.%2A%28rc%7Calpha%29&regexp=true, it seems like it is actually pretty well used already.

I excluded anything that is still on an alpha or hasn't been updated in a year, which adds 5 or so more crates.

That all seems like pretty good coverage the biggest missing ones are atsamd-hal and the stm32 crates (I opened an issue here stm32-rs/stm32f1xx-hal#473 and poked here atsamd-rs/atsamd#332). But embassy covers the STM32 hardware so I wouldn't think this would be too different.

It would be good to have a list of specific crates that are need to be tried with the new interface before making it official.

  • โค๏ธ8
rursprung commented

thanks a lot for your list, @tgross35 !

your regex is missing some dependencies which are written in a different format. e.g. stm32f4xx-hal supports embedded-hal v1.0.0-alpha.11 (and there's an issue for the RC upgrade: stm32-rs/stm32f4xx-hal#686), so a major stm32 HAL is already supporting it, which should be a good sign ๐Ÿ™‚

  • ๐Ÿ‘2
adamgreig commented

After some discussion last week, we now plan to release the final 1.0 version on Dec 28th, assuming no major issues come up before then. Please keep testing the RC in the meantime!

  • ๐ŸŽ‰13
  • โค๏ธ5
madsmtm commented

Hey, I've opened a few issues pertaining to the API surface we're stabilizing: #523, #524, #526, #527 and to a lesser extent #525.

I'll volunteer to do the work for each of these, if you think the changes are desired?

Also, this is only the v1.0 release of the embedded-hal crate, right? E.g. the release of v1.0 of embedded-hal-async should probably wait until the async trait functionality reaches stable Rust - at which point its functionality will probably just be included in embedded-hal itself.

Speaking of async, we should probably decide at least a rough plan for how that integration will look like, given that async fn in traits will be stable in 1.75.

adamgreig commented

Thank you for the review and issues!

Not coincidentally, December 28th (the planned e-h 1.0 release) is the day Rust 1.75 is released, and thus the day async traits are in stable Rust, so I think (?) the plan is to release 1.0 of e-h-async at the same time. However, I don't know if there's still a plan to merge it into embedded-hal, though I haven't heard it discussed in some time.

  • ๐Ÿ‘1
Finomnis commented

I wouldn't be against it :) Although I don't think the plan to do so would interfere with an eh 1.0 release.

Dirbaio commented

this was discussed when we did the split initially, the decision was to leave them split after 1.0.

  • ๐Ÿ‘2
Dirbaio commented

๐Ÿ“ฃ we just released v1.0.0-rc.3 with a small GPIO trait change (require &mut self in all methods, previously InputPin and StatefulOutputPin required only &self).

We'd appreciate it if you could do another round of testing. Change looks small but InputPin is a quite foundational trait, and the change might have unintended consequences in type inference / method resolution, we'd love to hear if it's causing issues for you.

eldruin commented

I have updated the list of pending issues here. @Dirbaio could you have a look?

Florob commented

I'd like to nominate #455 for the 1.0 release. I.e. specify whether I2C operations can return early like the SPI ones. If so: do we need a flush()?

  • ๐Ÿ‘1
Dirbaio commented

thanks @eldruin @Florob for bringing these up, I've sent a few PRs and believe they're all cleared now.

Dirbaio commented

No release today, it's going to be delayed a bit due to being blocked on #547

  • ๐Ÿ‘2
  • โค๏ธ1
eldruin commented

embedded-hal and embedded-hal-async 1.0.0 have now been released.
Thank you so much to everyone that was worked and helped through all this time to make this a reality.
๐ŸŽ‰

  • โค๏ธ8
  • ๐Ÿš€20
dbrgn commented

Thanks a lot to all people involved for pushing this forward week after week over the last 6 years! It's been a while ๐Ÿ™‚

Embedded Rust has a bright future ahead! ๐Ÿฆ€

  • โค๏ธ13
hansfbaier commented

Awesome!!!