Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grand_central_m4: Bump usb-device version #753

Merged
merged 10 commits into from
Aug 31, 2024

Conversation

supersimple33
Copy link
Contributor

Summary

I am bumping the version of usb-device for the grand central.

Checklist

  • CHANGELOG.md for the BSP or HAL updated
  • All new or modified code is well documented, especially public items
  • No new warnings or clippy suggestions have been introduced - CI will deny clippy warnings by default! You may #[allow] certain lints where reasonable, but ideally justify those with a short comment.

@supersimple33 supersimple33 changed the title Patch 1 Bump usb-device version Aug 29, 2024
@supersimple33 supersimple33 marked this pull request as draft August 29, 2024 21:00
@ianrrees
Copy link
Contributor

Thanks @supersimple33 ! I think this might be a more involved change, because the Grand Central BSP is using HAL v 0.15 which I think uses usb-device 0.2 .

I'd suggest you clone atsamd-rs locally, cd in to boards/grand_central_m4 and run cargo build --examples --all-features. Once you're in a position where that works, change the HAL dependency to 0.17 and usb-device to 0.3.1 (there may be a couple other updates I'm forgetting). You can then work through the errors building one example at a time via cargo build --example=blinky_basic for example - metro_m4 would be a good reference point, as it uses the current HAL.

Per #752 the neopixel example probably won't actually work - I guess the right thing to do is get it to compile but otherwise leave it in place.

I'm happy to help with the update if you've got hardware to test with.

@ianrrees
Copy link
Contributor

Also, don't mind the CI failures on nightly - looks like we need to update docstrings - or itsybitsy.

@ianrrees ianrrees changed the title Bump usb-device version grand_central_m4: Bump usb-device version Aug 30, 2024
@supersimple33
Copy link
Contributor Author

I'm getting the following error now @ianrrees

  = note: rust-lld: error: undefined symbol: _critical_section_1_0_acquire
          >>> referenced by lib.rs:193 (/Users/addisonhanrattie/.cargo/registry/src/index.crates.io-6f17d22bba15001f/critical-section-1.1.3/src/lib.rs:193)
          >>>               /Users/addisonhanrattie/Documents/atsamd/boards/grand_central_m4/target/thumbv7em-none-eabihf/debug/examples/blinky_basic-66cbd02f4a7e08ab.blinky_basic.dd7903c5ecf9b979-cgu.0.rcgu.o:(critical_section::with::h392fdb763c500007)
          
          rust-lld: error: undefined symbol: _critical_section_1_0_release
          >>> referenced by lib.rs:210 (/Users/addisonhanrattie/.cargo/registry/src/index.crates.io-6f17d22bba15001f/critical-section-1.1.3/src/lib.rs:210)
          >>>               /Users/addisonhanrattie/Documents/atsamd/boards/grand_central_m4/target/thumbv7em-none-eabihf/debug/examples/blinky_basic-66cbd02f4a7e08ab.blinky_basic.dd7903c5ecf9b979-cgu.0.rcgu.o:(core::ptr::drop_in_place$LT$critical_section..with..Guard$GT$::hebcbe303f9685640)

when compiling.

I ran cargo tree on the grand_central_m4 and the metro_m4 directories and I noticed that in metro_m4 the cortex-m crate has an extra dependency of critical_section not in the grand_central_m4 equivalent which fails to compile. Any advice on how to fix this?

@jbeaurivage
Copy link
Contributor

@supersimple33: you have a missing critical section implementation. Just add thr critical-section-single-core feature to the cortex-m dependency and you should be good to go!

@ianrrees
Copy link
Contributor

I recently started a wiki page with notes on updating firmwares to newer HALs - have added the critical section thing there.

https://github.com/atsamd-rs/atsamd/wiki/HAL-Update-Guide

Copy link
Contributor

@jbeaurivage jbeaurivage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff overall! There are a few minor but important things that should be addresses before we can merge this.

boards/grand_central_m4/Cargo.toml Outdated Show resolved Hide resolved
pac/atsamd51p/Cargo.toml Outdated Show resolved Hide resolved
boards/grand_central_m4/CHANGELOG.md Outdated Show resolved Hide resolved
@supersimple33 supersimple33 marked this pull request as ready for review August 31, 2024 16:46
@jbeaurivage jbeaurivage merged commit 44b2a76 into atsamd-rs:master Aug 31, 2024
108 checks passed
@supersimple33
Copy link
Contributor Author

@jbeaurivage @ianrrees thanks for the guidance on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants