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

[Fixes #72] Deal with dependencies to fix things, mostly doctests #73

Merged
merged 14 commits into from
Jan 3, 2024

Conversation

michalfita
Copy link
Collaborator

Purpose

Since Rust 1.60 it's possible to enable dependencies' features only for included dependencies.

That's going to work for rt and critical-section and some other features in future.

Scope

  • Workflow
  • Dependencies handling
  • Rename of internal features (prefix by __; idea stolen from another PAC/HAL project)

Since Rust 1.60 it's possible to enable dependencies' features only for included dependencies.

That's going to work for `rt` and `critical-section` and some other features in future.
@michalfita michalfita added bug Something isn't working enhancement New feature or request build system Related to build system or CI support dependencies Pull requests that update a dependency file labels Dec 23, 2023
@michalfita michalfita added this to the HAL Release 0.5.0 milestone Dec 23, 2023
```
let pac = hal::pac::Peripherals::take().unwrap();
```
instead of
```
let pac = hal::pac::Peripherals::take().unwrap();
```
@michalfita
Copy link
Collaborator Author

@martinmortsell This PR makes the whole CI 🟢 with steal() replaced back to take() in doctests.

In result we're more aligned with pattern svd2rust now counts on and which is in motion in other PACs and HAL crates.

This is braking change! There are no *-rt chip selection features any more, only rt feature to use.

A little scope creep: In the process I updated some dependencies and ended up with need to fix up how USB is used in the code, both doctests and examples. If you don't approve it, I can move them into another PR, and clean this one up from this change.

@martinmortsell
Copy link
Contributor

I was gonna revert the steal/take switch myself. Thanks.

I'll look through the rest in the beginning of January.

@michalfita michalfita marked this pull request as ready for review December 23, 2023 18:04
@michalfita
Copy link
Collaborator Author

I was gonna revert the steal/take switch myself. Thanks.

Just revert wouldn't work. I spend quite some time to get things straight and actually build and pass all checks.

I'll look through the rest in the beginning of January.

I may not have time in January to carry the burden of the next release.

Copy link
Contributor

@martinmortsell martinmortsell left a comment

Choose a reason for hiding this comment

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

Looks mostly fine to me, could benefit from squashing some commits, aside from the one or two minor changes I suggested.

CHANGELOG.md Outdated
- Update `cortex-m-rt` version in examples to `0.7.3`.
- Switched from AFE0_AD6 to AFE0_AD8 for the atsamv71_xult board ADC example code.
- The `rt` separated to don't act as part of chip selection feature anymore.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to

Suggested change
- The `rt` separated to don't act as part of chip selection feature anymore.
- The `rt` separated to not act as part of chip selection feature anymore.

Cargo.toml Outdated
Comment on lines 13 to 49
# In the end it doesn't work as I supposed based on `Cargo.toml` from other
# projects. First, it works only at workspace level; second, it's not optional
# in the sense of probing for existence. We shall consider documenting use of
# https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#paths-overrides
# atsame70j19b = { path = "../atsamx7x-pac/pac/atsame70j19b" }
# atsame70j20b = { path = "../atsamx7x-pac/pac/atsame70j20b" }
# atsame70j21b = { path = "../atsamx7x-pac/pac/atsame70j21b" }
# atsame70n19b = { path = "../atsamx7x-pac/pac/atsame70n19b" }
# atsame70n20b = { path = "../atsamx7x-pac/pac/atsame70n20b" }
# atsame70n21b = { path = "../atsamx7x-pac/pac/atsame70n21b" }
# atsame70q19b = { path = "../atsamx7x-pac/pac/atsame70q19b" }
# atsame70q20b = { path = "../atsamx7x-pac/pac/atsame70q20b" }
# atsame70q21b = { path = "../atsamx7x-pac/pac/atsame70q21b" }
# atsams70j19b = { path = "../atsamx7x-pac/pac/atsams70j19b" }
# atsams70j20b = { path = "../atsamx7x-pac/pac/atsams70j20b" }
# atsams70j21b = { path = "../atsamx7x-pac/pac/atsams70j21b" }
# atsams70n19b = { path = "../atsamx7x-pac/pac/atsams70n19b" }
# atsams70n20b = { path = "../atsamx7x-pac/pac/atsams70n20b" }
# atsams70n21b = { path = "../atsamx7x-pac/pac/atsams70n21b" }
# atsams70q19b = { path = "../atsamx7x-pac/pac/atsams70q19b" }
# atsams70q20b = { path = "../atsamx7x-pac/pac/atsams70q20b" }
# atsams70q21b = { path = "../atsamx7x-pac/pac/atsams70q21b" }
# atsamv70j19b = { path = "../atsamx7x-pac/pac/atsamv70j19b" }
# atsamv70j20b = { path = "../atsamx7x-pac/pac/atsamv70j20b" }
# atsamv70n19b = { path = "../atsamx7x-pac/pac/atsamv70n19b" }
# atsamv70n20b = { path = "../atsamx7x-pac/pac/atsamv70n20b" }
# atsamv70q19b = { path = "../atsamx7x-pac/pac/atsamv70q19b" }
# atsamv70q20b = { path = "../atsamx7x-pac/pac/atsamv70q20b" }
# atsamv71j19b = { path = "../atsamx7x-pac/pac/atsamv71j19b" }
# atsamv71j20b = { path = "../atsamx7x-pac/pac/atsamv71j20b" }
# atsamv71j21b = { path = "../atsamx7x-pac/pac/atsamv71j21b" }
# atsamv71n19b = { path = "../atsamx7x-pac/pac/atsamv71n19b" }
# atsamv71n20b = { path = "../atsamx7x-pac/pac/atsamv71n20b" }
# atsamv71n21b = { path = "../atsamx7x-pac/pac/atsamv71n21b" }
# atsamv71q19b = { path = "../atsamx7x-pac/pac/atsamv71q19b" }
# atsamv71q20b = { path = "../atsamx7x-pac/pac/atsamv71q20b" }
# atsamv71q21b = { path = "../atsamx7x-pac/pac/atsamv71q21b" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Unclear to me what the purpose of these lines are, but if you don't have a plan for fixing it, these lines should be removed rather than commented out.

can = ["mcan-core"]

device-selected = []
__v71 = ["can"]
Copy link
Contributor

Choose a reason for hiding this comment

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

underscores for internal features, nice.

@michalfita michalfita merged commit d1664b1 into development Jan 3, 2024
88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build system Related to build system or CI support dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants