Const generic peripheral instances #24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before this commit, all peripheral instances shared the same instance type, Instance. This commit introduces a const generic N to indicate the peripheral's instance number. The update allows users to design APIs that are generic over all N, or specific to an instance number. It's a refactor of a previous approach that used typenums. Instance APIs are now behind an impl block, instead of a dedicated module. It simplifies the auto-generated code, and makes it easier for users to design APIs to the instance types. It's also backwards compatible with the read_reg!, write_reg!, and modify_reg! macros. Unfortunately, it breaks reset_reg!, since the macro assumed a path instead of an impl block. A child commit updates the macro for the new implementation. A "sole instance" is a peripheral that has only one instance. We use 0 as the magic const generic. This isn't a detail users will need to remember; the usage guide (incoming child commit) shows how users should design APIs for a sole instance. Commit builds with all chip features. Also build the 1062 with 'nosync' and 'rtic' features. Tests fail to compile; that's fixed in child commits.
If a user pairs LPUART1 with LPUART8::release, the error now manifests at compile time instead of runtime. Remove the runtime assert that checked these operations. Remove the test, since it no longer compiles. The usage guide will demonstrate the compile failure. cargo test --features=imxrt1062 --tests --lib Documentation tests still do not work; need to fix reset_reg!.
It's API compatible with the previous approach. All tests pass: cargo test --features=imxrt1062
We override the reset_reg! macro, since we need customizations to support our const generic implementations. Our override is API compatible for users.
The documentation build includes all modules from all chips. This results in duplicate definitions, since multiple modules will associate an impl block with the same type. This commit introduces a newtype that's emitted only for documentation builds. A documentation note attached to each newtype hints at how the symbol is defined when building for a target chip.
There's no harm in defining the Instance type in a 'nosync' build, since there's no way to realize an instance. The array of interrupts is tied to the instance type, even if 'nosync' is enabled. As of this commit, the test for associated interrupts pass. Additionally, always implement Valid for instances, even if 'nosync' is enabled.
The same trait can be implemented on all unique Instance<N> types.
Describes how to use the RAL API, and design higher-level APIs using the RAL. Implemented as a separate markdown file so it could be studied within the repository. It also makes it easier to maintain within the library documentation (we don't need to inline it within the Python script or anything). Documentation examples are tested as if they were written inline.
|
Well to look through an 18kloc changeset with any sort of fine comb would be hard, but the general changes look good, it makes sense to include the const id in the type (though might effect compile times from crates that depend on this one given more generics are possibly needed). I like the sealed trait additions as well, those make sense. |
18 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PR updates the RAL API with uniquely-numbered
Instancetypes. Before this PR, all LPUARTx instances shared the same Instance type. As of this PR, all LPUARTx instances are unique over x, wherexis some constant. To make that a bit more concrete, here's what the types now look like:The approach replaces #4, and uses const generics instead of typenums. See that PR description, and this branch's commit messages, for design choices. The usage docs helps users understand how to use these peripheral instance types.