Skip to content

Conversation

@plaes
Copy link
Contributor

@plaes plaes commented May 28, 2023

When attempting to build a single example, the suggested feature list for at least embassy-related examples was missing embassy-time-timg0.

@jessebraham
Copy link
Member

Thanks for the PR, good catch.

I think this was probably missing because on most chips you have the option of using embassy-time-timg0 or embassy-time-systimer as the time driver. @MabezDev might have some more context to add here. I think for the sake of examples this is probably fine though, unless he has any objections.

@jessebraham
Copy link
Member

Might be nice to do this for the other HALs as well, if you don't mind?

plaes added 3 commits May 31, 2023 21:14
When attempting to build a single example, the suggested feature list
for at least embassy-related examples was missing `embassy-time-timg0`,
which caused somewhat cryptic compile errors.
@plaes plaes force-pushed the esp32-hal-example-features branch from f6c9d43 to 7e250f9 Compare May 31, 2023 19:25
@plaes
Copy link
Contributor Author

plaes commented May 31, 2023

Might be nice to do this for the other HALs as well, if you don't mind?

This is done for the xtensa-based HAL's. I'll skip RISC-V-based as I don't have toolchain set up for this at the moment.

@MabezDev
Copy link
Member

MabezDev commented Jun 1, 2023

imo this is only really worth doing on the esp32, but if we want it, we can do it for the rest we can do.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 13, 2023

IMHO this makes sense for ESP32 but for the others the user could use different timer implementations (TIMG or SYSTIMER) so for them it might be better to have something like this:

#[cfg(debug_assertions)]
compile_error!("PSRAM on ESP32 needs to be built in release mode");

Where we can check that one of the timers is enabled and provide a friendly error message explaining how to get the example to work

@jessebraham
Copy link
Member

Sorry for letting this bitrot, kind of fell of my radar.

I'm still not entirely sure how we want to handle this; I think maybe just to make our lives easier we could default to embassy-timer-timg0 by default for all examples and document that the embassy-timer-systick feature is also available for non-ESP32 chips.

Would like to hear some feedback on this from other people, though. If you're still interested in reviving this then, once we have an answer, would be happy to get this merged. However, if you no longer have the time or interest that is also fine, and you can feel free to close this; I won't make the decision for you :)

@jessebraham
Copy link
Member

I'm going to close this PR for the time being, as we still have not yet decided how to move forward on this. Thank you for opening this PR regardless, though!

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.

4 participants