-
Notifications
You must be signed in to change notification settings - Fork 416
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
Add mikoto board #230
Add mikoto board #230
Conversation
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 for your PR. Could you confirm the VID/PID as review comment.
src/boards/mikoto/board.h
Outdated
#define USB_DESC_VID 0x7A79 | ||
#define USB_DESC_UF2_PID 0x0520 | ||
#define USB_DESC_CDC_ONLY_PID 0x0520 |
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.
could you confirm this is the VID/PID assigned specifically for this board.
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.
could you confirm this is the VID/PID assigned specifically for this board.
7A79 is "zy" (zhiayang), I haven't seen this duplicated anywhere, but do you have any suggestions to be sure? Thank you.
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.
you cannot randomly pick an vid/pid. These ids are uniquely assigned by vendor for each product and this repo only accept one with valid vid/pid. You should try to apply for an vid/pid from other donor.
hmm, is there an easy way for me to get one? thank you. |
you could apply for one https://pid.codes/ |
I've got a PID now: 1209 / 7a01. pidcodes/pidcodes.github.com#701 |
Thanks @zhiayang , I changed that. |
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.
everything look good, only a minor change request to correct the board id format
src/boards/mikoto/board.h
Outdated
|
||
#define UF2_PRODUCT_NAME "Mikoto nRF52840" | ||
#define UF2_VOLUME_LABEL "MIKOTO-BOOT" | ||
#define UF2_BOARD_ID "nRF52840-mikoto" |
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 am not picky but board id must follow uf2 spec https://github.com/microsoft/uf2#files-exposed-by-bootloaders . You need to add the board revision as well
The Board-ID field is machine-readable and consists of a number of dash-separated tokens. The first token is the CPU type, second is the board type, and third is the board revision. More tokens can be also added.
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, I changed that.
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.
look good, it is ready to merge, though for board binary to be part of the release, the board need to be added to the ci as well https://github.com/adafruit/Adafruit_nRF52_Bootloader/blob/master/.github/workflows/githubci.yml#L27
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.
perfect now
From https://github.com/zhiayang/mikoto