Skip to content

Conversation

@MabezDev
Copy link
Member

@MabezDev MabezDev commented Sep 29, 2023

Whilst the PCR, SYSTEM and DPORT peripherals are different, we currently use them all in the same way. This PR unifies the peripheral name in the hal to SYSTEM. The idea is that they all do the same sort of thing, so we can collect them under the same name, and later down the line we can begin to expose differences under an extended API.

The benefits to this are imo quite big, the examples now are all identical, which makes things easier for esp-wifi, and paves a path towards the multichip hal.

Why not do this in the PAC? Imo the pac should be as close to the hardware as possible, and the HAL is where we should have abstractions such as this.

Must

  • The code compiles without errors or warnings.
  • All examples work.
  • cargo fmt was run.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You updated existing examples or added examples (if applicable).
  • Added examples are checked in CI

Nice to have

  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code.

Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

I love it!

Just one idea:
Wouldn't it make sense to have something like RADIO => virtual instead of RADIO => false to make it clearer what this does / means

@MabezDev
Copy link
Member Author

Wouldn't it make sense to have something like RADIO => virtual instead of RADIO => false to make it clearer what this does / means

Nice idea! I like that much better.

@MabezDev MabezDev force-pushed the unify-system branch 3 times, most recently from 7e988ce to 0898984 Compare September 29, 2023 14:37
Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Left one comment, but overall LGTM. Thanks for taking care of this, I think it's a nice improvement.

Whilst the PCR, SYSTEM and DPORT peripherals are different, we currently
use them all in the same way. This PR unifies the peripheral name in the
hal to `SYSTEM`. The idea is that they all do the same sort of thing, so
we can collect them under the same name, and later down the line we can
being to expose differences under an extended API.

The benifits to this are imo quite big, the examples now are all identical,
which makes things easier for esp-wifi, and paves a path towards the
multichip hal.

Why not do this in the PAC? Imo the pac should be as close to the
hardware as possible, and the HAL is where we should abstractions such
as this.
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.

4 participants