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

Updated XIAO M0 dependencies and examples #742

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

TheRolfFR
Copy link
Contributor

Summary

I updated XIAO M0 dependencies and examples with latest crates.
I tested blink and ssd1306 examples but all examples do compile so it should be good.
Feel free to use the modifications for other boards, especially usb device changes for string descriptors.

Checklist

  • CHANGELOG.md for the BSP or HAL updated
  • [pass] 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.

If Adding a new Board

  • [pass] Board CI added to crates.json
  • [pass] Board is properly following "Tier 2" conventions, unless otherwise decided to be "Tier 1"

If Adding a new cargo feature to the HAL

  • [pass] Feature is added to the test matrix for applicable boards / PACs in crates.json

@@ -118,7 +119,7 @@ fn main() -> ! {
NVIC::unmask(interrupt::SERCOM4);
}
loop {
delay.delay_ms(1000u16);
delay.delay_ns(1000000000u32);
Copy link
Contributor

@jbeaurivage jbeaurivage Jul 12, 2024

Choose a reason for hiding this comment

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

Just fyi, the DelayNs trait has delay_ms as a provided method if you prefer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got an error saying to disambiguate delay_ms and I don't know how to solve that:

error[E0034]: multiple applicable items in scope
   --> examples\sercom_interrupt.rs:122:15
    |
122 |         delay.delay_ms(1000);
    |               ^^^^^^^^ multiple `delay_ms` found
    |
note: candidate #1 is defined in the trait `cortex_m::prelude::_embedded_hal_blocking_delay_DelayMs`
   --> C:\Users\yannl.THEROLF-PC\.cargo\registry\src\index.crates.io-6f17d22bba15001f\embedded-hal-0.2.7\src\blocking\delay.rs:16:5
    |
16  |     fn delay_ms(&mut self, ms: UXX);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: candidate #2 is defined in an impl of the trait `xiao_m0::embedded_hal::delay::DelayNs` for the type `xiao_m0::atsamd_hal::delay::Delay`
help: disambiguate the method for candidate #1
    |
122 |         cortex_m::prelude::_embedded_hal_blocking_delay_DelayMs::delay_ms(&mut delay, 1000);
    |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
help: disambiguate the method for candidate #2
    |
122 |         xiao_m0::embedded_hal::delay::DelayNs::delay_ms(&mut delay, 1000);
    |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Copy link
Contributor

@jbeaurivage jbeaurivage Jul 14, 2024

Choose a reason for hiding this comment

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

You have two traits in scope that provide a delay_ms method. You can either:

  • Remove the cortex_m::prelude::_embedded_hal_blocking_delay_DelayMs trait from the scope (I suspect it comes from the prelude)
  • Disambiguate which trait you're using by using fully qualified syntax:
<Delay as DelayNs>::delay_ms(&mut delay, 1000);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback, I learned something.

I tried removing the bsp::hal::prelude::* import but they are too much imports inside this file, so I would just recode all imports or remove one with a non-lowercase empty mod. I think that is a deeper problem that should have a proper solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, personally I'm partial to deprecating the prelude entirely. It's obviously not only my decision, but I believe the prelude is causing more problems than it's worth. And often the "band-aid" fix is not obvious, and introduces weird syntax that newbies aren't accustomed to (as you have just discovered)!

@jbeaurivage
Copy link
Contributor

Thanks @TheRolfFR! Please just make sure the formatting check passes, and I can merge this.

@jbeaurivage jbeaurivage merged commit 37a97c1 into atsamd-rs:master Jul 15, 2024
89 of 107 checks passed
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.

2 participants