search

stm32-rs / bxcan

bxCAN peripheral driver for STM32 chips
Apache License 2.0
31 stars 22 forks source link

Add support for defmt 0.3 #47

Closed newAM closed 2 years ago

newAM commented 2 years ago

Update defmt (and other knurling-rs tools) to 0.3, see https://ferrous-systems.com/blog/defmt-3/

korken89 commented 2 years ago

This is how I'm solving it in heapless to support both 0.2 and 0.3: https://github.com/japaric/heapless/pull/251 Might be of interest.

newAM commented 2 years ago

This is how I'm solving it in heapless to support both 0.2 and 0.3: japaric/heapless#251 Might be of interest.

That's a good idea, I updated it to use a range.

Looks ok to me, but there is a failure in build.

I cannot reproduce the same failure on Linux, and unfortunately I don't have a Windows install easily accessible right now.

Are you able to help me out with this?

jonas-schievink commented 2 years ago

please ping me once CI is fixed

newAM commented 2 years ago

Somebody on matrix pointed out the Windows build is not building for an embedded target, which is not supported by defmt: https://github.com/knurling-rs/defmt/issues/463

I fixed the failure by removing that build for Windows :sweat_smile:.

@jonas-schievink CI is fixed.

jonas-schievink commented 2 years ago

hmm, why did it work with 0.2?

newAM commented 2 years ago

hmm, why did it work with 0.2?

I did a quick scan through the code.

defmt 0.3 introduced some new global symbols for flushing:

https://github.com/knurling-rs/defmt/blob/124a6f6f3abe4e77e885d6fab001872a2da08ee8/defmt/src/lib.rs#L377-L382

These symbols get defined by the global logger:

https://github.com/knurling-rs/defmt/blob/8a6e8eebe40f943d9b0ba8725cd6da033b9c399e/macros/src/lib.rs#L24-L28

https://github.com/knurling-rs/defmt/blob/e240ba99bc0c2258bc8ba2d7e4694e568f19c758/macros/src/attributes/global_logger.rs#L41-L45

I am guessing there is something about Windows that doesn't like GNU linker script or the export_name attributes that were introduced for these new symbols.

jonas-schievink commented 2 years ago

bors r+

bors[bot] commented 2 years ago

Build succeeded:

newAM commented 2 years ago

Thanks for the review & merge!

Would it be possible to get a 0.6.1 release with this change?

jonas-schievink commented 2 years ago

Published 0.6.1

Regarding the linker error, I don't think that's a problem with defmt 0.3.0 specifically, since https://github.com/stm32-rs/bxcan/pull/49 also fails to link again defmt 0.2.3 (https://github.com/stm32-rs/bxcan/runs/4213689874?check_suite_focus=true)