Skip to content

Add forwards-compatible v0.6.5 #315

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

Merged
merged 3 commits into from
Jan 18, 2021
Merged

Add forwards-compatible v0.6.5 #315

merged 3 commits into from
Jan 18, 2021

Conversation

adamgreig
Copy link
Member

This PR creates a new v0.6.5 on the v0.6.x branch, which depends on v0.7 and re-exports many of its types.

Notable changes:

  • The links field is removed from Cargo.toml to permit linking to v0.7 series
  • The #[no_mangle] CORE_PERIPHERALS static is removed to permit linking to v0.7 series
  • The TAKEN static is removed: sadly we can't access the 0.7 static because it's not pub, so instead this version calls take() and steal() from v0.7 to ensure only one version of PERIPHERALS (from either 0.6 or 0.7) can exist at once.
  • As in v0.5.10, we transmute a unit tuple into PERIPHERALS as we can no longer construct many of its types.
  • I've removed the old Travis config and copied the GHA CI/clippy runs back into this branch so hopefully they run (and pass!) here.

Types that are not exported from 0.7:

  • ITM, MPU, NVIC, SCB all have breaking changes, though some smaller types within those modules are re-exported.
  • in register, we keep basepri, basepri_max, lr, pc, apsr only for the targets which 0.7 no longer exports them for.

I've tested that using this version builds on several projects that currently use 0.6.4, including using RTIC, various STM32 PACs, etc, and including changing those projects to use c-m 0.7 at the top level but continue to depend on other crates (PACs, RTIC, etc) that use 0.6.4. They all build. Nevertheless I'd definitely appreciate more testing on this. We could even do a prerelease on crates.io?

@adamgreig adamgreig requested a review from a team as a code owner January 13, 2021 02:01
@rust-highfive
Copy link

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against v0.6.x. Please double check that you specified the right target!

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Jan 13, 2021
Copy link
Member

@thalesfragoso thalesfragoso left a comment

Choose a reason for hiding this comment

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

Thanks Adam! Everything seems to be in order, should we merge this and do a pre-release ?

@korken89
Copy link
Contributor

We are planning to have RTIC 0.6 be based on cortex-m 0.7, and something hits me.
Is it possible to support both cortex-m 0.6 and 0.7 in an external framework?

I gave it a look, and it seems that one needs to detect which version is being compiled to select the proper names in the library. E.g. in RTIC we had issues with InterruptNumber vs Nr.
Is there a recommended way to support multiple cortex-m versions?

@adamgreig
Copy link
Member Author

cortex-m 0.7 implements InterruptNumber for Nr, so if you just use InterruptNumber and someone passes you Nr, I think it should work OK. Once this PR is released as cortex-m 0.6.5, it should be fine for you to use cortex-m in RTIC and projects still using 0.6.x should be able to interoperate. If you have time I'd be grateful if you could try it out with this 0.6.5 (hopefully we'll do a pre-release soon) and check my expectation is correct...

@adamgreig
Copy link
Member Author

I've updated this PR to be a prerelease version, so I think we could merge this and I'll publish it as 0.6.5-alpha, then if it seems OK we can do a new PR making it 0.6.5 and releasing it.

@korken89
Copy link
Contributor

Cool, I can surely give it a try with RTIC!

Copy link
Member

@thalesfragoso thalesfragoso left a comment

Choose a reason for hiding this comment

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

Thanks again!

bors r+

bors bot added a commit that referenced this pull request Jan 18, 2021
315: Add forwards-compatible v0.6.5 r=thalesfragoso a=adamgreig

This PR creates a new v0.6.5 on the v0.6.x branch, which depends on v0.7 and re-exports many of its types.

Notable changes:

* The `links` field is removed from Cargo.toml to permit linking to v0.7 series
* The `#[no_mangle] CORE_PERIPHERALS` static is removed to permit linking to v0.7 series
* The `TAKEN` static is removed: sadly we can't access the 0.7 static because it's not pub, so instead this version calls `take()` and `steal()` from v0.7 to ensure only one version of `PERIPHERALS` (from either 0.6 or 0.7) can exist at once.
* As in v0.5.10, we transmute a unit tuple into `PERIPHERALS` as we can no longer construct many of its types.
* I've removed the old Travis config and copied the GHA CI/clippy runs back into this branch so hopefully they run (and pass!) here.

Types that are *not* exported from 0.7:

* ITM, MPU, NVIC, SCB all have breaking changes, though some smaller types within those modules are re-exported.
* in `register`, we keep `basepri`, `basepri_max`, `lr`, `pc`, `apsr` only for the targets which 0.7 no longer exports them for.

I've tested that using this version builds on several projects that currently use 0.6.4, including using RTIC, various STM32 PACs, etc, and including changing those projects to use c-m 0.7 at the top level but continue to depend on other crates (PACs, RTIC, etc) that use 0.6.4. They all build. Nevertheless I'd definitely appreciate more testing on this. We could even do a prerelease on crates.io?

Co-authored-by: Adam Greig <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 18, 2021

Timed out.

@adamgreig
Copy link
Member Author

Right, I backported the GHA scripts but didn't update bors to require those instead of Travis (which was removed). Hopefully bors will like this...

bors retry

@bors
Copy link
Contributor

bors bot commented Jan 18, 2021

Build succeeded:

@bors bors bot merged commit b51662f into v0.6.x Jan 18, 2021
@bors bors bot deleted the v0.6.5 branch January 18, 2021 11:33
@adamgreig
Copy link
Member Author

Ok, this is published as 0.6.5-alpha, and I've managed to test it.

Annoyingly, crates (such as the stm32f4 PAC) that support "cortex-m >=0.5.8, <0.7" would normally use 0.6.4 and would soon use 0.6.5, but when my top-level crate specifies 0.6.5-alpha which uses 0.7.0 which has links=cortex-m, Cargo settles on 0.5.8 for stm32f4's dependency, which doesn't have links=cortex-m but does have CORE_PERIPHERALS, so the build fails. Cargo won't pick 0.6.x because they all (except 0.6.5-alpha which it won't pick) have links=cortex-m so Cargo skips them since 0.7.0 already links=cortex-m. For the same reason it won't pick 0.5.10 which depends on ^0.6.0, so it settles for 0.5.8 which it can't tell will fail later (because it pre-dates specifying links=cortex-m).

As far as I can tell, once 0.6.5 is released properly, this should all Just Work (Cargo will choose 0.6.5 for stm32f4 when resolving ">=0.5.8,<0.7") but so far I've only been able to test it by patching my dependencies to also specify 0.6.5-alpha directly.

Any other testing appreciated, I'll open PRs soon to do 0.6.5 actual release and to back-port the CHANGELOG changes to master.

adamgreig pushed a commit that referenced this pull request Jan 12, 2022
315: Use volatile read for ICSR register r=adamgreig a=lulf

This prevents the compiler from optimizing the read.

Edit: I also added a change to include the 9th bit in the IRQ. I can leave that out if it would break anything. 

#314

Co-authored-by: Ulf Lilleengen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants