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.

Sign up for GitHub

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

Jump to bottom

esp-config cannot be used with esp-hal-embassy #2846

Open
bugadani opened this issue Dec 19, 2024 · 4 comments · May be fixed by #2848
Open

esp-config cannot be used with esp-hal-embassy #2846

bugadani opened this issue Dec 19, 2024 · 4 comments · May be fixed by #2848
Labels
bug Something isn't working package:esp-config
Milestone
1.0.0-beta.0

Comments

@bugadani
Copy link
Contributor

bugadani commented Dec 19, 2024 •
edited
Loading

esp-hal's build script errors if users set esp-hal-embassy config options. This is, because esp-hal doesn't know about the other crate, and esp-hal is a prefix of esp-hal-embassy, therefore the crate thinks that ESP_HAL_EMBASSY_ config values belong to it, but rejects them as unknown.
@bugadani bugadani added bug Something isn't working status:needs-attention This should be prioritized package:esp-config labels Dec 19, 2024
@MabezDev MabezDev removed the status:needs-attention This should be prioritized label Dec 19, 2024
@MabezDev MabezDev added this to the 1.0.0-beta.0 milestone Dec 19, 2024
@MabezDev
Copy link
Member

Yeah we should fix this ASAP, added to the milestone.

@bugadani
Copy link
Contributor Author

Any ideas how? :D I guess we can list the conflicting prefixes for esp-hal, but that solution only works as long as we don't create another esp-hal-* crate. Or even worse, someone else creates one - looking at you, community crates.

@bjoernQ
Copy link
Contributor

bjoernQ commented Dec 19, 2024

only thing I can think of (and is breaking) is to have a special separator between the crate name and the config key.

Something like ESP_HAL_EMBASSY_CFG_config - then esp-hal would look for ESP_HAL_CFG_xxx only

and even a crate named esp-hal-cfg would look for ESP_HAL_CFG_CFG_xxx

But it's not ideal

@bugadani
Copy link
Contributor Author

bugadani commented Dec 19, 2024 •
edited
Loading

We can go with two underscores as a separator, I guess. ESP_HAL_CFG__FOO may still not be visually obvious, but it's probably less magical?

@bugadani bugadani linked a pull request Dec 19, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working package:esp-config
Projects
esp-rs
Status: Todo
Milestone
1.0.0-beta.0
Development

Successfully merging a pull request may close this issue.

Use _CONFIG_ to separate config prefix and key bugadani/esp-hal
3 participants
@bugadani @bjoernQ @MabezDev