-
Notifications
You must be signed in to change notification settings - Fork 3k
Check in algo generation code #3996
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
Conversation
CC: @0xc0170 |
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.
Looks good. Do we have a plan to get this regression tested some how?
tools/flash_algo/extract_targets.py
Outdated
@@ -0,0 +1,80 @@ | |||
#!/usr/bin/env python |
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.
Can we name this file something that better matches what it does? like extract_flash_algos.py
or just extract.py
(it's in the flash_algo
dir after all)
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.
maybe extract_all.py
?
@@ -0,0 +1,359 @@ | |||
#!/usr/bin/env python |
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.
This name is redundant. Should this be __init__.py
instead?
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.
Discussed in person. Summary - long term this file should be moved into arm-pack-manager or another common repo. Right now the master copy lives in the FlashAlgo repository and this is a copy of it.
) | ||
|
||
ro_rw_zi = _find_sections(self.elf, sections_to_find) | ||
ro_rw_zi = _algo_fill_zi_if_missing(ro_rw_zi) |
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.
immediately re-assigning a variable can be confusing. maybe
ro_rw_zi = _algo_fill_zi_if_missing(_find_sections(self.elf, sections_to_find))
This?
f40dde3
to
606f038
Compare
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
I tested this PR. I rebuilt all devices, and run:
The last two failed with (same error):
As I understand, this script accepts the name that comes from the pack. I thought it would translate it targets.json target name to the cmsis pack name (as targets contain
|
Check in scripts which are able to generate flash algos for supported targets. To initially download all packs the following command should be run: "python extract.py --rebuild_all" After that all supported targets can be rebuilt by running: "python extract.py" Finally, to rebuild an individual target you can used its pack name: "python extract.py --target STM32F302R8"
606f038
to
70f3252
Compare
The name of the pack target should be used with The multiple flash algos for the same target are for different regions. The flash_start and flash_size will be different for each of these. Typically one algo will be flash and the others will be regions for configurable settings. To simplify this I updated the script to only create an algo for the flash region if it can determine this. I also added the option |
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Thanks for the explanation. I pulled the latest update, works for me LGTM |
I fixed the template plus implementations - to protect with /morph test |
Thanks for the update @0xc0170, the changes look good to me. |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Check in scripts which are able to generate flash algos for supported
targets.
To initially download all packs the following command should be run:
After that all supported targets can be rebuilt by running:
Finally, to rebuild an individual target you can used its pack name: