-
Notifications
You must be signed in to change notification settings - Fork 159
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
Extract family data to JSON file #47
Conversation
Let's stick to JSON as the source of truth and remove the makeuf2families script. Could you replace the Let's also remove the family IDs from the readme (and remove the script) and instead just link the JSON document from the readme. I don't think there's much value to the users of keeping that list (unlike list of actual bootloader implementations). |
Roger that. Changes applied. |
utils/uf2conv.py
Outdated
@@ -238,6 +211,17 @@ def write_file(name, buf): | |||
print("Wrote %d bytes to %s" % (len(buf), name)) | |||
|
|||
|
|||
def load_families(): | |||
with open("uf2families.json") as f: |
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.
where should we look for this file? I think this checks the current folder, while I think it would be wiser to look in the same place where the script is placed?
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 current JSON file is in that location. I can add code that will retrieve it from a fixed URL if you like, but right now the JSON file is in the utils
directory with this script.
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.
I think that if you run:
cd ~/myrepo
python uf2/utils/uf2conv.py foo.bin
then it will look for the families in ~/myrepo
but it should look in ~/myrepo/uf2/utils
. People often include uf2 as submodule just for that script.
No need to download.
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.
Understood. I have made a change that I think addresses this concern.
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.
Changed it to your version.
utils/uf2conv.py
Outdated
@@ -212,7 +212,7 @@ def write_file(name, buf): | |||
|
|||
|
|||
def load_families(): | |||
with open("uf2families.json") as f: | |||
with open(os.path.join(sys.path[0], "uf2families.json")) as f: |
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.
I think you need os.path.dirname(os.path.abspath(__file__))
not sys.path[0]
see https://stackoverflow.com/questions/3430372/how-do-i-get-the-full-path-of-the-current-files-directory
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.
My search-fu found this one first 😆 https:https://www.tutorialspoint.com/How-to-open-a-file-in-the-same-directory-as-a-Python-script
And I did test it out:
➜ uf2 git:(bcr-family-fixes) python3 utils/uf2conv.py --family aaa
Family ID needs to be a number or one of: ATMEGA32, SAML21, NRF52, ESP32, STM32L1, STM32L0, STM32WL, LPC55, STM32G0, GD32F350, STM32L5, STM32G4, MIMXRT10XX, STM32F7, SAMD51, STM32F4, FX2, STM32F2, STM32F1, STM32F0, SAMD21, STM32F3, STM32F407, STM32H7, STM32WB, ESP8266, KL32L2, STM32F407VG, NRF52840, ESP32S2, ESP32S3, ESP32C3, RP2040, STM32L4
➜ uf2 git:(bcr-family-fixes) ✗ cd utils
➜ utils git:(bcr-family-fixes) ✗ python3 uf2conv.py --family aaa
Family ID needs to be a number or one of: ATMEGA32, SAML21, NRF52, ESP32, STM32L1, STM32L0, STM32WL, LPC55, STM32G0, GD32F350, STM32L5, STM32G4, MIMXRT10XX, STM32F7, SAMD51, STM32F4, FX2, STM32F2, STM32F1, STM32F0, SAMD21, STM32F3, STM32F407, STM32H7, STM32WB, ESP8266, KL32L2, STM32F407VG, NRF52840, ESP32S2, ESP32S3, ESP32C3, RP2040, STM32L4
➜ utils git:(bcr-family-fixes) ✗
I like your answer better, so let me change it.
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.
Sorry about that! I run python interactively and sys.path[0] there is empty, but it does work the way you describe when run from a script! You can leave as is or change it to "my" version (which is more explicit)
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.
Changed it to your version.
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.
Thank you very much for bearing with me!
Collaborating is caring. I appreciate the consideration. |
This patch creates a new JSON file to address my concern in #46 regarding having a single "source of truth" for family info.
To start with, I extracted the known IDs, short names and descriptions from
README.md
anduf2conv.py
using an included Python scriptmakeuf2families.py
. If we choose to continue down this path, this script will no longer be needed and can be removed.I have updated the
uf2conv.py
script to use the JSON data.Long term, when adding new IDs, updating the JSON file directly should be the preferred method. When it is updated, the README can be regenerated using the included
updatereadme.py
script.One remaining work item is that the
!!!
entries in the JSON file need to be resolved, and then theREADME.md
can be updated.