-
-
Notifications
You must be signed in to change notification settings - Fork 397
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
Teach target-gen #2543
Teach target-gen #2543
Conversation
28da96b
to
2908074
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks good from my perspective :). -110000 on the diff is always nice :D
Yeah getting rid of 2MB of raw YAML is always a win. But let's not be hasty, there might be unforseen side effects here. |
I agree, I only tested esp devices 🙈 |
I did a spot-review on most of the changes and they seemed fine, but this PR breaks most of the website targets so that part isn't ideal :) I need a green-light from Noah and probably a website patch to deal with this, first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'refresh' is a nice addition to target-gen, and gives us the ability to make future optimizations.
Thanks for the effort to reduce duplicates in the definitions. Clearly a valuable optimization!
Haha I didn't expect #2540 to significantly impact this idea, but it does. Looks like merging the targets like this is actually not viable, at least not if we want to keep variant |
... to:
psel
as hex (makes RP2040 a bit more readable)The idea came while experimenting with STM detection, where the there are myriads of very similar variants of the same chip. probe-rs only cares about memory layouts, core configuration and flash algos currently, so these targets can be merged.
The original target names are preserved under
aliases
and this information can also be used in the future if a "de-deduplicate" command will become necessary. This is not kept for algos because I don't think they are meaningful if separate.In total, this PR removes about 40% (in binary size) of our
targets/
folder. The website needs to be adjusted to expand targets based on the aliasses, I think.