stm32-rs / stm32l0xx-hal

A hardware abstraction layer (HAL) for the STM32L0 series microcontrollers written in Rust
BSD Zero Clause License
96 stars 60 forks source link

Enable SPI2 on subset of stm32l0x1 devices #221

Closed jcard0na closed 1 year ago

jcard0na commented 1 year ago

There are members of the stm32l0x1 subfamily that thave two SPIs. In particular the STM32l051 and the STM32l071.

Compile tested as:

cargo build --release --example spi2 --features mcu-STM32L051C8Tx

(builds)

cargo build --release --example spi2  --features mcu-STM32L031C6Tx

(fails to build, as this subfamily only has 1 SPI)

but not run on actual hardware.

jcard0na commented 1 year ago

Thanks for the quick review. I think I addressed all the comments except that I don't know how to review changes to cargo doc. Can you help with that?

jamwaffles commented 1 year ago

Thanks for the quick review.

Any time!

It looks like there are some remaining NaiveDate errors to sort out :(. Are you able to access the Github Actions logs on this PR? Either way, you should be able to run the commands in .github\workflows\ci.yaml to test for issues on your dev machine.

I don't know how to review changes to cargo doc

I'm not sure what you mean by this I'm afraid. I see warnings when I run cargo doc --features 'rt mcu-STM32L011D3Px' but nothing that stops a build, unless I try cargo doc --release --examples --features 'rt mcu-STM32L011D3Px' which vomits over a conflicting panic dependency. Do you mean these or is there something else coming up?

jcard0na commented 1 year ago

It looks like there are some remaining NaiveDate errors to sort out :(. Are you able to access the Github Actions logs on this PR? Either way, you should be able to run the commands in .github\workflows\ci.yaml to test for issues on your dev machine.

Duh. I did replace all the deprecated functions... but I introduced a new call to and_hms(). Fixed now.

I don't know how to review changes to cargo doc

I'm not sure what you mean by this I'm afraid. I see warnings when I run cargo doc --features 'rt mcu-STM32L011D3Px' but nothing that stops a build, unless I try cargo doc --release --examples --features 'rt mcu-STM32L011D3Px' which vomits over a conflicting panic dependency. Do you mean these or is there something else coming up?

I was trying to address your original cargo doc comment:

I'd want to diff the cargo doc output to check for any regressions in other parts if you do that though.

jcard0na commented 1 year ago

I do get an error when running:

cargo doc --release --examples --features 'rt mcu-STM32L011D3Px'

but the same error shows up before my changes:

warning: build failed, waiting for other jobs to finish...
error: duplicate lang item in crate `panic_semihosting`: `panic_impl`.
  |
  = note: the lang item is first defined in crate `panic_halt` (which `timer_interrupt_rtic` depends on)
  = note: first definition in `panic_halt` loaded from /opt/javier/dev/stm32l0xx-hal/target/thumbv6m-none-eabi/release/deps/libpanic_halt-93ea9eb1b94cbf3a.rmeta
  = note: second definition in `panic_semihosting` loaded from /opt/javier/dev/stm32l0xx-hal/target/thumbv6m-none-eabi/release/deps/libpanic_semihosting-aeee2f7c58ae038e.rmeta

I can complete the build if I comment out all extern crate panic_halt declarations.

jamwaffles commented 1 year ago

Right, that makes more sense now. It should work if you remove the --examples arg - those don't generate any docs.

If you wanted to fix the root cause, it would be great to have every example use panic_halt and remove panic_semihosting completely. Would also be a good opportunity to replace extern crate panic_halt with use panic_halt as _; which is a bit more modern. I'm happy to hack on this at a later date if you just want to land the SPI2 stuff for now :)

jcard0na commented 1 year ago

I did try

to have every example use panic_halt and remove panic_semihosting completely.

but that still failed with the error

error: duplicate lang item in crate `panic_semihosting`: `panic_impl`.

Here is the diff in case you want to check it out panic_halt.diff.txt

Given that I don't really know where this conflicting dependency comes from, I think I'll accept your offer to land the SPI2 fix now. Thanks!