adafruit / Adafruit_nRF52_Bootloader

USB-enabled bootloaders for the nRF52 BLE SoC chips
MIT License
445 stars 401 forks source link

Enable board-specific UF2 family IDs #152

Closed henrygab closed 4 years ago

henrygab commented 4 years ago

NOTE: REQUIRES TESTING (will edit if changes)

Fix #130.

For boards that do not define both USB VID and PID, no change in behavior. Otherwise...

henrygab commented 4 years ago

Thanks @henrygab, welcome back, hopefully you have resolved your real life issues.

Thanks. 😃

Indeed the PR is simpler that I thought. [...] I think we can just be bold and define CFG_UF2_BOARD_APP_ID as always existed. The generic FAMILY ID already ensure the backward compatible, and every board with USB required VID/PID defined.

Well, I tried to presume VID/PID are defined with commit aa8cd83. However, it appears that the feather_nrf52832 board does not define these. So, I guess I should revert to the prior version, which allows this to not be defined. 😆

Also, I cannot test this except on feather nrf52840 express, so I will defer to you for testing.

P.S. -- EPS32s2 looks interesting.

hathach commented 4 years ago

Well, I tried to presume VID/PID are defined with commit aa8cd83. However, it appears that the feather_nrf52832 board does not define these. So, I guess I should revert to the prior version, which allows this to not be defined.

832 doesn't have USB, this code won't be included to the CC progress. Please skip those ifndef.

PS: I see the CI force you to revert the change, For now you can keep the ifndef in the uf2cfg.h, and remove those in the ghostfat.c . I will clean up the include of uf2 from 832 or move the define to other place later on.

henrygab commented 4 years ago

OK, updated to remove the #ifdef from ghostfat.c, and left them in for uf2cfg.h due to build errors for '832. Ready for testing?

henrygab commented 4 years ago

@hathach Given all comments addressed, I've updated the title to remove [WIP] tag. However, I'm leaving the PR as draft because testing is still required, and it will stop accidental merge.

hathach commented 4 years ago

@hathach Given all comments addressed, I've updated the title to remove [WIP] tag. However, I'm leaving the PR as draft because testing is still required, and it will stop accidental merge.

Thanks @henrygab, I will do actual hardware testing next week