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

Deletes the top-level workspace and moves the examples. #828

Merged
merged 3 commits into from
Aug 17, 2024

Conversation

thejpster
Copy link
Member

RP2350 stuff is coming, and you can't build rp2040-hal for Armv8-M and you can't build rp235x-hal for Armv6-M, and so they cannot be in the same workspace. Ergo, if the rp2350-hal is coming to this repo (and it is), we cannot have a workspace any more.

I also moved the examples into their own package, because that's much neater than having to deal with memory.x being in magically the right place, or a bunch of dev-dependencies that are not relevant to most crate users.

@thejpster thejpster force-pushed the move-examples-delete-workspace branch from 826cb98 to 7a63fc7 Compare August 16, 2024 16:37
@thejpster thejpster requested review from 9names and jannic August 16, 2024 16:39
@thejpster
Copy link
Member Author

thejpster commented Aug 16, 2024

I'll rebase this

Edit: done

@thejpster thejpster force-pushed the move-examples-delete-workspace branch from fa175fb to b423208 Compare August 16, 2024 19:47
rp2040-hal/Cargo.toml Outdated Show resolved Hide resolved
pio = "0.2.0"
pio-proc = "0.2.0"
rp2040-boot2 = "0.3.0"
rp2040-hal = { path = "../rp2040-hal", version = "0.10.0", features = ["binary-info", "critical-section-impl", "rt", "defmt"] }
Copy link
Member

Choose a reason for hiding this comment

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

Although It was an eyesore, I think it was a good thing to see what features each example required.

Copy link
Member Author

Choose a reason for hiding this comment

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

Feature unification. The HAL is only built once and needs all the features set that are required by any bin. With examples it would just silently not build any examples when the required feature wasn't set, which I found frustrating (cargo build wouldn't build any examples, but cargo build --all-features would).

We could split each example into its own package, but that seems extreme.

Or I guess I could document each required feature at the top of each binary, but I've got no way of checking that that's correct.

Copy link
Member

Choose a reason for hiding this comment

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

You are right. I found it nice that from a user experience, targeting a specific examples would tell "that examples requires those features".
But I’m fine with this as well.

Would it be good in the future to mutualize the examples and have a "portability"/"bsp" module in the example crate to map to either rp2040 or rp235x ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's very unlikely that anyone wouldn't want the critical-section-impl or rt features. In particular, not using rt gives you a 'default' vector table which is broken and doesn't work. I'm also going to change all the examples so they include binary info by default, because it's just nice to have.

That leaves the 'defmt' feature. I could add a defmt feature to the examples package, and then mark the various binary crates within it that require the defmt feature in the HAL to require that feature to be set on the package.

@thejpster thejpster force-pushed the move-examples-delete-workspace branch from f7adce8 to b57d484 Compare August 17, 2024 12:45
RP2350 stuff is coming, and you can't build rp2040-hal for Armv8-M and you can't build rp235x-hal for Armv6-M, and so they cannot be in the same workspace. Ergo, if the rp2350-hal is coming to this repo (and it is), we cannot have a workspace any more.

I also moved the examples into their own package, because that's much neater than having to deal with memory.x being in magically the right place, or a bunch of dev-dependencies that are not relevant to most crate users.
Removes node.js deprecation notice.
Keys are now sorted, and all one-liners.
@thejpster thejpster force-pushed the move-examples-delete-workspace branch from b57d484 to fae6b25 Compare August 17, 2024 12:51
@thejpster thejpster merged commit 679ed27 into main Aug 17, 2024
12 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