stm32-rs / stm32f7xx-hal

A Rust embedded-hal HAL for all MCUs in the STM32 F7 family
Apache License 2.0
115 stars 67 forks source link

bxcan/defmt version conflict #114

Closed pdgilbert closed 3 years ago

pdgilbert commented 3 years ago

In code that uses multiple device crates I have started having cargo update problems because of the change in the dependency bxcan = "0.5" (see https://github.com/stm32-rs/bxcan/issues/22#issue-848677821). Can the dependency be more flexible, say bxcan = ">=0.4" ?

hannobraun commented 3 years ago

I'm not familiar with bxcan, or the details of how we use it (I reviewed the PR adding it, but have since forgotten all about it). Does this HAL actually work with multiple versions of bxcan? Can we be sure that, even if it works for us, this can't cause breakage for our users?

If the answer to both questions is "yes", then I'm happy to merge a PR that relaxes the version requirement. Although it shouldn't be open-ended, but restricted to versions that are known to work (so bxcan = ">=0.4, <0.6" would probably be correct).

pdgilbert commented 3 years ago

(Anyone please correct me if I say something is wrong. This is the first time I've run into this.)

The problem seems to be that defmt uses a link, so it is not possible for bxcan to specify a range of versions for defmt. A specific version of bxcan is tied to a specific version of defmt. (See https://github.com/knurling-rs/defmt/issues/426#issue-823703071 ) Thus stm32f7xx-hal specifying a specific version of bxcan propagates the defmt conflict problem to anything linked with stm32f7xx-hal.

Does this HAL actually work with multiple versions of bxcan?

Technically I think this is not about "multiple versions" but rather about allowing a range of versions so that a single version can be selected when other linked crates also use bxcan. stm32f7xx-hal CI seems to work when bxcan = ">=0.4, <0.6" is allowed. My workflow testing at https://github.com/pdgilbert/eg_stm_hal/actions/runs/712196680 and at https://github.com/pdgilbert/LoRaGPS-rust/actions/runs/712016668 is working, other than things that are broken for other reasons. Without this fix I have to removestm32f7xx-hal because it breaks all the other crates.

The change frombxcan 0.4 to 0.5 was just made fairly recently in the git version of stm32f7xx-hal. Unless you upgraded for a specific reason, to fix something that was broken, then it is unlikely that allowing the older version will cause any new problems within this HAL. I'm not sure what the new version of bxcan provides, but presumably there are fixes and new features. The range specification means users would be able to take advantage of these as long as other crates they use do not constrain them.

this can't cause breakage for our users?

I am pretty sure your last upgraded specification has already broken things for your users. It is just not being widely reported because

So, this is a fix for something that is already broken. I'll submit a PR shortly.

hannobraun commented 3 years ago

Thanks for the info, and sorry for the late reply. I'll reply on your pull request (#115).

hannobraun commented 3 years ago

Fixed in #115.