stm32-rs / stm32g4xx-hal

Peripheral access API for STM32G4 series microcontrollers
Apache License 2.0
67 stars 48 forks source link

Move to embedded-hal 1.0 #118

Open jbtheou opened 10 months ago

jbtheou commented 10 months ago

Now that 1.0 is released, is there a plan already to move the HAL to this version?

no111u3 commented 10 months ago

If you provide some implementation for it we are ready to use it.

usbalbin commented 10 months ago

How do we want that to look like?

no111u3 commented 10 months ago

I'm not sure about remove 0.2 causing feature incomplete of 1.0, but we can split trait implementation (internal/external) and use feature based flags for it (follows f4 & other hals). 0.2 support will be enable by default as in other hals now.

techmccat commented 10 months ago

I've been working on porting the SPI driver over to hal 1.0 with some optimizations using the FIFO and packed frames (and GPIO, which was mostly just copy-pasting code).

The code has been tested with a driver I'm working on and seems to work fine. Right now it's just missing support for 16 bit frames, which should be a lot simpler than packed 8 bit.

pdgilbert commented 10 months ago

FWIW, my suggestion is to start a separate branch that is just using embedded-hal-1.0.0. That is what is being done in stm32h7xx-hal. The dual support in stm32f4xx-hal is pretty neat but the window for which it is valuable may be short, and it seems like a lot of work. You might re-consider if dual support is still interesting once an eh 1.0.0 version is working.

There seems to be a start made in branch hal-1 of @techmccat 's fork. It may be more than just changing to

[dependencies.embedded-hal]
version = "1.0.0"
package = "embedded-hal"

but that would be a beginning.

techmccat commented 10 months ago

I've gotten around to implementing all the new traits while keeping the old ones and fixing some examples, although i'm not quite satisfied with the delay, it could be much more high-res but the module needs a bit of rework (maybe something like the f4 hal with compile-time frequencies?).

Next comes updating the prelude and fixing the examples

pdgilbert commented 10 months ago

When you are ready, could you remove or change the 0.2.4 dependency to embedded-hal-old and make

[dependencies.embedded-hal]
version = "1.0.0"
package = "embedded-hal"

Then I will add your fork of stm32g4xx-hal to the active list I am testing at https://github.com/pdgilbert/rust-integration-testing/actions . At the moment I only have stm32f4xx-hal and stm32h7xx-hal active.

FYI, at the moment, with stm32f4xx-hal I have 33 examples compiling of the 67 that were compiling with eh 0.2.4, and with stm32h7xx-hal there are 23 of 67. The difference is stm32f4xx-hal magic support for both eh-1 and older. The magic might work on more of my examples, but many had shared-bus which fails now (see https://github.com/stm32-rs/stm32f4xx-hal/issues/722 ). I am trying to convert to embedded-hal-bus but I cannot get that working with rtic (see https://github.com/rtic-rs/rtic/issues/886).

techmccat commented 10 months ago

Done, examples all compile on g474 I had to change delay provider because cortex-m doesn't yet support DelayNs on SysTick

pdgilbert commented 9 months ago

I have forked @techmccat 's hal-1 branch to add the single line

pub use  stm32 as pac;

in lib.rs. This small change removes the need for a very large number of conditional statements in the setup functions for my examples. Somewhere I saw a general embedded-hal recommendation that this module be called pac rather than a manufacture/hardware specific name like stm32, but I can no longer find that recommendation. In any case, of the 11 stm32xxxx_hal's I was testing prior to moving to embedded-hal_1.0.0, stm32g4xx_hal was one of only 2 that still required using stm32 rather than pac. So, really this change should be put in the main branch too.

Regarding testing my examples with this fork, I have made very good progress. So much so that I am wondering if anything special was done to accomodate using embedded-hal_0.4.2 device crates?

There seem to be two many things I need to take care about. One is if the sensor device crate needs to consume a delay and does not yet accept DelayNs then I need to be careful to specify the delay type so that it is recognized as DelayUs. I have had better luck doing this with stm32g4xx_hal than with stm32h7xx_hal branch eh-v1.0. The second is when using embedded-io Read and Write. When I do this I can get both stm32g4xx_hal and stm32h7xx_hal versions of some examples compiling, but stm32f4xx_hal compiling fails because of https://github.com/stm32-rs/stm32f4xx-hal/issues/721.

Of 69 examples I had working with many hals prior to the change to embedded-hal_1.0.0, I now have 49 compiling with the fork of stm32g4xx_hal. With stm32f4xx_hal 50 are compiling and 28 are compiling with stm32h7xx_hal branch eh-v1.0 (see discussion https://github.com/stm32-rs/stm32h7xx-hal/issues/474). Details are at https://github.com/pdgilbert/rust-integration-testing/actions .

The way I choose to write some of the examples can affect these numbers because of the above issues. In general I lean toward trying to do things the embedded-hal_1.0.0 way but I am still learning what that means. Also, rtic's need for types rather than traits in Share and Local complicates a wholesale move to traits.