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

Prep for HAL 0.5.0 release #351

Merged
merged 4 commits into from
Jun 14, 2022
Merged

Conversation

9names
Copy link
Member

@9names 9names commented Jun 4, 2022

Update the changelog with all HAL changes, bump version number

@9names
Copy link
Member Author

9names commented Jun 4, 2022

Well this is fun.
If I don't bump the HAL version in the BSPs, I get a CI error from the BSPs (since the in-tree HAL is a different version).
If I do, I get a type conflict (because the PIO drivers pulls in 0.4.0's rp2040_hal::pio::UninitStateMachine).

@jannic
Copy link
Member

jannic commented Jun 4, 2022

Those circular dependencies really get annoying.

For i2c-pio I already suggested moving the example https://github.com/ithinuel/i2c-pio-rs/pull/6.
But we have the same issue with ws2812-pio. And that one's worse, because there are several boards with ws2812 LEDs, so having those examples for the board crates is really nice.

As those examples already depend on a git version of ws2812-pio, it's easy to work around by pointing to an updated git version, eg.:

-ws2812-pio = { git = "https://github.com/ithinuel/ws2812-pio-rs", rev = "fd6b6604d65a66242b52ccf7f24a95ca325991dd" }
+ws2812-pio = { git = "https://github.com/jannic/ws2812-pio-rs", rev = "b6d27e8f3715c2b4d0fc0737c366314beb1f18c7" }

(Just an example - of course @ithinuel could include the updated dependency in his own git, no need to point to my fork.)

We could also include a vendored version of ws2812-pio somewhere inside https://github.com/rp-rs/rp-hal so we could keep the versions in sync. As ws2812-pio is basically a single small source file, that wouldn't cause much overhead.
(Proof of concept: #353)

@9names
Copy link
Member Author

9names commented Jun 4, 2022

Yeah, I agree. I've started down the vendoring approach too.
The other option is to have BSPs not in this repo, so they have their own CI, can pin to a specific version of the HAL and {i2c,ws2812}PIO drivers that agree with each other and not break everything.

@9names
Copy link
Member Author

9names commented Jun 4, 2022

Instead of vendoring inside of the rp-hal repo we could also have a downstream fork of those repos under rp-rs.
That makes it easier to update them if the main repos change.

@jannic
Copy link
Member

jannic commented Jun 4, 2022

If we start adding new repos I think I'd prefer a separate repo for the BSPs, as you suggested above.

@9names
Copy link
Member Author

9names commented Jun 5, 2022

I was planning on having a discussion on how we maintain BSPs after this release was out, so if we can find a clean solution that will get us past this issue that would be my preference.
Having the PIO drivers support a range of versions (like ithinuel/ws2812-pio-rs@main...9names:rp-hal_0.5.0) will get us over the line for CI, but users will have to "cargo update" at least for a few packages when they update their hal/bsp to avoid them trying to compile against 0.4.0 while the HAL itself is 0.5.0
That's unfortunate.
We'll want a new release of the drivers themselves anyway for folks that are using the crates.io versions of the HAL and BSP

@9names
Copy link
Member Author

9names commented Jun 5, 2022

Hmm no, if I select a range of versions for ws2812 that would be locking us out of finishing the PIO fifo changes during 0.5, which I absolutely do not want.
I'll try to get that resolved first.

@ithinuel
Copy link
Member

ithinuel commented Jun 5, 2022

As a reminder I'm ok to move the ws2812b-pio-rs & i2c-pio-rs to the rp-rs org instead of maintaining a fork or having to vendor it.
I believe we should start using the crates' version rather than a git-hash.
Having each BSP in their respective repo (or 1 repo to hold them all) seem like a sensible solution to me since the circ dep in on the repo not on the crates.

@9names
Copy link
Member Author

9names commented Jun 5, 2022

Thanks, I forgot you said that it was okay.
Would you be able to add rp-rs/admins as a crate owner for each of them as well, so that we can push version bumps to crates.io if necessary?

@jannic
Copy link
Member

jannic commented Jun 12, 2022

What about this sequence of changes?

  1. Push branches refering to rp-hal version 0.5.0 to https://github.com/rp-rs/ws2812-pio-rs and https://github.com/rp-rs/i2c-pio-rs
  2. Change the examples to use those, by their git revision
  3. Release rp-hal version 0.5.0
  4. Do releases of ws2812-pio-rs and i2c-pio-rs depending on rp-hal 0.5.0
  5. Change the examples to refer to the released versions

Step 5 isn't a breaking change, as those are just dev-dependencies for the examples. And I guess most people copying the examples are getting them form github anyways, so it doesn't matter which versions the release points to.

@9names
Copy link
Member Author

9names commented Jun 13, 2022

I still want to get the FIFO changes across the line for 0.5.0.
Now that we have the pio repos under rp-rs i'm okay with linking to a git branch.
Moved my i2c-pio-rs branch into rp-rs/i2c-pio-rs, updated #352 to use that branch.
I'll rebase this on top of main once that is merged, then create branches for ws2812-pio-rs and i2c-pio-rs that depend on rp-hal 0.5.0.

@9names 9names force-pushed the prepare_for_hal_0.5.0_release branch from 6c9e948 to 2aa93cf Compare June 13, 2022 08:40
@9names 9names requested a review from jannic June 13, 2022 10:00
@jannic
Copy link
Member

jannic commented Jun 13, 2022

How shall we handle the board support packages?
As they re-export the hal, a breaking change of the hal is also a breaking change of the bsp.
Do we release them separately, or shall we bump their versions at the same time?

@jannic
Copy link
Member

jannic commented Jun 13, 2022

Do we release them separately, or shall we bump their versions at the same time?

When releasing new versions of the BSPs, we should also update their changelogs, and I noticed some of them contain broken links to non-existing tags, e.g. in https://github.com/rp-rs/rp-hal/blob/main/boards/adafruit-feather-rp2040/CHANGELOG.md

To not delay the HAL release, I'd prefer to do the BSP releases later.

@9names
Copy link
Member Author

9names commented Jun 13, 2022

I've usually left BSP release stuff until after the HAL is out. Some of them have not even had a release yet.
All the links will need to be updated if we move the BSPs, that won't be fun.

@9names 9names merged commit 1574a36 into rp-rs:main Jun 14, 2022
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.

3 participants