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

Provide defmt feature in BSPs, that enables atsamd-hal/defmt #810

Open
ianrrees opened this issue Jan 10, 2025 · 4 comments
Open

Provide defmt feature in BSPs, that enables atsamd-hal/defmt #810

ianrrees opened this issue Jan 10, 2025 · 4 comments
Labels
board support Related to support for a particular board enhancement New feature or request

Comments

@ianrrees
Copy link
Contributor

ianrrees commented Jan 10, 2025

In a firmware that depends on a BSP, it's a bit awkward to use defmt for types that originate in the HAL. One needs to add the same version of the HAL as the BSP uses to Cargo.toml, to enable the defmt feature.

To resolve this, we chould have all the BSPs (at least, that use a new-enough HAL) support a defmt feature themselves.

@ianrrees ianrrees added enhancement New feature or request good first issue Good for newcomers board support Related to support for a particular board labels Jan 10, 2025
@sajattack
Copy link
Member

I think part of the difficulty here is that defmt requires an addition to the linker script?

@ianrrees
Copy link
Contributor Author

ianrrees commented Jan 10, 2025

I'm not sure I follow - yes using defmt in a firmware requires modifying the linker script, but even with that in place it's not straightforward to use defmt in a firmware that depends on a BSP directly.

For example, if my firmware's Cargo.toml has something like this:

[dependencies]
metro_m4 = { version = "0.16.1", features = ["usb"] }

and in my firmware, I want to use something like:

use metro_m4 as bsp;
use bsp::hal;

#[derive(Error, defmt::Format, Debug)]
enum Error {
    #[error("UART error:")]
    UartError(hal::sercom::uart::UartError),
   // ...more variants...
}

Currently, the only way AFAICT to do this, is to modify Cargo.toml to add a dependency on the HAL, using the same version as the BSP:

[dependencies.atsamd-hal]
version = "0.20.2"
default-features = false
features = ["defmt"]

What I'm proposing is that it would be nice to be able to just do:

[dependencies]
metro_m4 = { version = "0.16.1", features = ["usb", "defmt"] }

@ianrrees ianrrees changed the title Provide defmt feature in BSPs to enable atsamd-hal/defmt Provide defmt feature in BSPs, that enables atsamd-hal/defmt Jan 10, 2025
@sajattack
Copy link
Member

Oh, yeah, I know, I'm just saying adding the dep to the default linker script might be slightly annoying and I don't think it can be feature gated. But it's probably not an issue. Just something to consider.

@ianrrees
Copy link
Contributor Author

ianrrees commented Jan 10, 2025

Ah, you're a step ahead of me: if the feature is enabled, but the linker script doesn't have the defmt piece, then the build fails. This means that the --all-features builds as are done by CI would require us to modify the default linker script if we added this defmt feature to BSPs...

So yes, probably deserves a bit more thought, I'll remove the "good first issue" tag!

@ianrrees ianrrees removed the good first issue Good for newcomers label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
board support Related to support for a particular board enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants