Simplify embassy-executor / esp-hal-embassy version bump process

[t-moe]

Every time a new embassy-executor version is released, esp-hal-embassy needs to be updated, which in turn cannot happen until embedded-test is updated.

( Also: the embedded-test esp32c6 example need to use patched versions of esp-hal-* until the new esp-hal-* versions are released. Otherwise CI fails.)

[kaspar030]

Would a range dependency for embassy-executor work? >=0.7, <=0.9? Simply extend on non-breaking embassy-executor bumps?
The subset that e-t is using of e-e's API is more stable than the semver compatibility.

[t-moe]

Would a range dependency for embassy-executor work? >=0.7, <=0.9?

We had this in an earlier version, and it caused a lot of confusion for the users. The issue is that dev-dependencies (such as embedded-test) are not unified with normal dependencies (such as embassy-executor) range-dependencies allow for duplicated dependencies in the crate graph.
So for example if your root crate depends on embassy-executor 0.8, embedded-test would bring in embassy-executor 0.7.
See https://doc.rust-lang.org/cargo/reference/resolver.html#unexpected-dependency-duplication for an example.

[kaspar030]

So for example if your root crate depends on embassy-executor 0.8, embedded-test would bring in embassy-executor 0.7.

Ah, right. :/

It doesn't seem to do that in every case. I patched e-t (edit, 0.7.0-alpha.3) to use the range locally, and with that, it compiles & works with Ariel, which is still on e-e 0.7. That doesn't produce a duplicate e-e.

So, feature flags it is? embassy-executor-09?

[kaspar030]

It doesn't seem to do that in every case.

Maybe because I have a Cargo.lock. 🤦

[t-moe]

It doesn't seem to do that in every case. I patched e-t (edit, 0.7.0-alpha.3) to use the range locally, and with that, it compiles & works with Ariel, which is still on e-e 0.7. That doesn't produce a duplicate e-e.

yes, I think the problem only arises if the root crate uses a newer version (e.g. 0.8 ) than the lower bound on e-t (0.7).
this will lead to a compiler error because no arch-* / std-* features are enabled on e-e 0.7. (unless you are using the ariel-os feature, which brings its own macros).

So, feature flags it is? embassy-executor-09?

that would be one option.

Another would be to not depend on embassy-executor in embedded-test at all and just emit #[embassy_executor::task] from the proc-macro and hope for the best... This gives ugly compiler errors though.

[kaspar030]

Maybe it is just never correct to have multiple embassy-executor versions, and upstream should add some "link=i_am_the_only_one". Maybe in that case, Cargo would not chose multiple versions but one that works.

[bugadani]

Unfortunately, cargo will not help us with this issue. Cargo will always pick the newest version it is allowed to pick. If embedded-test lets it pick a range, cargo will go for the top of the range. The links directive doesn't change that, I believe that is a link-time property.

---

Versioned features are a good option regardless of the future compatibility problem: maintaining compatibility with older embassy-executor can be useful - HALs rarely update at the same time, especially esp-hal which needs to wait for a bunch of dependencies.

However, I think the best solution is the one that distances embedded-test from embassy-executor as much as possible. Ideally, we shouldn't need to update embedded-test at all for a new executor release. Unfortunately, an upcoming breaking change in embassy-executor is that the syntax of spawning tasks will change, so the code emitted by embedded-test needs changing at that point, too.

I'm one of the rare masochists who are fairly comfortable at working with macros, so maybe there is a simpler solution, but embedded-test needs three things (or after a quick glance at the code, maybe just the last one):

• How to spawn tasks
• How to transform functions into tasks
• How to mark the main task and start the executor

These can be provided by macros by the user, or, if the appropriate feature is selected, the macros can be provided by embedded-test, too.

Long story short: if embedded-test needs a framework-specific syntax to do something, let users define that syntax in their program. I personally think this should be a rare enough/advanced enough topic to reach for macros as a solution. Hopefully this isn't just paper engineering and this is actually possible to do.

[t-moe]

However, I think the best solution is the one that distances embedded-test from embassy-executor as much as possible.

I agree.

I had something similar in mind: let the user pass in (via embededded_test::tests(...)) the attribute names to use for annotating a task/main function (e.g. ariel_os::task) and let him pass the name of a function that spawns the tasks.

These can be provided by macros by the user

With your idea ^^, how would the user pass the macros to embedded-test?

I like the idea, that the user only has to set this up once per project, and not for every embededded_test::tests(...) location.

[bugadani]

how would the user pass the macros to embedded-test?

The idea would be to either prescribe the macro's name, or inject it with the attribute (#[embedded_test::tests(implementation = macro_name)] if necessary.

the attribute names to use for annotating

That assumes the underlying framework uses attributes in the first place :)

[t-moe]

The downside about this whole approach is that there is no way to typecheck the macros. Compiler errors will be ugly and have an unhelpful span (pointing to the attribute args implementation = macro_name at best).

[kaspar030]

May I suggest going with features for now? The next embassy-executor update is pending. Using cargo features is well understood. Still requires a new embedded-test release for every embassy-executor bump, but that'd be small few lines change.

I can draft a PR!

[t-moe]

@bugadani
Would going with feature flags work for you?

[bugadani]

Sure, probably.

[t-moe]

Then, @kaspar030, if you could draft a PR that would be highly appreciated.