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

Expand cluster implementation + fix _strip to work on clusters #251

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

cosmikwolf
Copy link
Contributor

Implemented _expand_cluster, and added an example.

It takes a string or an array of strings as arguments.

Added an example in /res/expand_cluster and updated the readme

closes: #246

@cosmikwolf
Copy link
Contributor Author

cosmikwolf commented Oct 27, 2024

ah.. wait on this one. I just realized I need to add the addressOffset from the cluster to the expanded registers

update - fixed it in 40c94d4

src/patch/peripheral.rs Outdated Show resolved Hide resolved
@cosmikwolf
Copy link
Contributor Author

I have made some updates to address your concerns about multiple element clusters, and clusters with nested clusters and register arrays.

I don't really have any good examples of SVD files with properly formatted clusters and arrays. I am doing this work because the TI MSPM0 SVD clusters are a cluster****. If you have any SVD files that you know are well formatted that have these types of structures I can analyze and add to my test case, please let me know.

src/patch/peripheral.rs Outdated Show resolved Hide resolved
@cosmikwolf cosmikwolf marked this pull request as draft November 3, 2024 20:51
@cosmikwolf
Copy link
Contributor Author

I changed the way it names clusters with multiple elements to match the SVD spec https://open-cmsis-pack.github.io/svd-spec/main/elem_special.html#elem_dimArrayIndex

Since no delimiter is specified in the spec, I gave options for the end user to set the pre-index delimiter, and post-index delimiter if they wish. Also I gave an option to force a single element cluster to have an index. Details are in the readme changes.

@cosmikwolf cosmikwolf marked this pull request as ready for review November 4, 2024 03:23
src/patch/yaml_ext.rs Outdated Show resolved Hide resolved
@cosmikwolf
Copy link
Contributor Author

ok, latest pull request has print statements cleaned up, and I also refactored expand_cluster into two functions for readability

@cosmikwolf
Copy link
Contributor Author

I also added code yesterday that closes #249

@cosmikwolf cosmikwolf changed the title Expand cluster Expand cluster + fix _strip to work on clusters Nov 4, 2024
@cosmikwolf cosmikwolf changed the title Expand cluster + fix _strip to work on clusters Expand cluster implementation + fix _strip to work on clusters Nov 4, 2024
@burrbull
Copy link
Member

burrbull commented Nov 5, 2024

LGTM. Please squash commits.

@cosmikwolf
Copy link
Contributor Author

Commits squashed

@burrbull burrbull added this pull request to the merge queue Nov 7, 2024
@cosmikwolf
Copy link
Contributor Author

Also, I would be interested in joining the embedded rust working group if you guys are looking for new members

Merged via the queue into rust-embedded:master with commit 128fc7a Nov 7, 2024
12 checks passed
@burrbull
Copy link
Member

burrbull commented Nov 7, 2024

There are several groups. What exact one? This crate is under @rust-embedded/tools

@burrbull
Copy link
Member

burrbull commented Nov 7, 2024

Anyway you should create a proposal first. See rust-embedded/wg#801 for example. And add this for vote on next meeting: rust-embedded/wg#804
All teams are here: https://github.com/orgs/rust-embedded/teams

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.

Ability to uncluster
2 participants