Skip to content

boards/feather-nrf52840*: rename to adafruit-feather-nrf52840-* #21349

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

Merged
merged 2 commits into from
Apr 4, 2025

Conversation

mguetschow
Copy link
Contributor

Contribution description

While we are in spring cleaning mood, and with #21242 in, let's make use of the board alias feature to start renaming boards in a backward-compatible way according to RDM0003.

Testing procedure

CI should automatically start using the new board names for all tests except tests/build_system/board_alias which automatically tests the new board aliases. You can test the working aliases locally by using the old BOARD names feather-nrf52840 and feather-nrf52840-sense.

@github-actions github-actions bot added Area: doc Area: Documentation Area: build system Area: Build system Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration labels Apr 3, 2025
@mguetschow mguetschow requested a review from crasbe April 3, 2025 12:22
@crasbe crasbe added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 3, 2025
@mguetschow mguetschow force-pushed the feather-nrf52840-alias branch from 857b8d2 to ae71300 Compare April 3, 2025 12:31
@crasbe
Copy link
Contributor

crasbe commented Apr 3, 2025

The boards/common/adafruit-nrf52-bootloader/doc.md has to be adapted as well and there are some references to the old name in the doc.mds of the Feather nRF52840 Express and Sense.

Also the tests/sys/fido2_ctap_hid/Makefile, tests/drivers/lsm6dsxx/Makefile, tests/sys/xtimer_now32_overflow/Makefile and
tests/sys/fido2_ctap/Makefile still have the old board format.

Edit: I found those with grep -Rnw . -e "feather-nrf52840".

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Apr 3, 2025
@mguetschow
Copy link
Contributor Author

Thanks for double-checking, should be fixed now.

@crasbe
Copy link
Contributor

crasbe commented Apr 3, 2025

You could use the opportunity to adapt the headers according to #21335, but that's no must.

Personally I think the warning introduced in #21242 should've been red because now you don't really notice it, but that's too late now I guess 😅

I'm not sure if it would be better to name the fixup PRs something like tests/*: rename to adafruit-feather-nrf52840-*, since it's another directory? idk

Otherwise everything looks good to me, I can physically test it when I'm home with the nRF52840DK pretending to be a Feather.

@mguetschow
Copy link
Contributor Author

You could use the opportunity to adapt the headers according to #21335, but that's no must.

I'd rather not, to be honest, to keep the changes small for this commit. Still waiting for @KSKNico to provide the next subtree PR :)

Personally I think the warning introduced in #21242 should've been red because now you don't really notice it, but that's too late now I guess 😅

It's never too late: #21352

I'm not sure if it would be better to name the fixup PRs something like tests/*: rename to adafruit-feather-nrf52840-*, since it's another directory? idk

I've now squashed them together anyways.

@riot-ci
Copy link

riot-ci commented Apr 3, 2025

Murdock results

✔️ PASSED

e92ac97 examples/**/nimble_gatt: increase answer buffer for longer BOARD names

Success Failures Total Runtime
10281 0 10282 10m:25s

Artifacts

@crasbe crasbe enabled auto-merge April 3, 2025 15:15
@crasbe crasbe added this pull request to the merge queue Apr 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 3, 2025
@crasbe
Copy link
Contributor

crasbe commented Apr 3, 2025

#define STR_ANSWER_BUFFER_SIZE 100

This is the cause of the build failure. Apparently with adafruit-feather-nrf52840-express, the string is longer than 100 letters (101 to be exact 😅 ).

@mguetschow
Copy link
Contributor Author

100 ways to break your builds :P

@github-actions github-actions bot added the Area: examples Area: Example Applications label Apr 4, 2025
@mguetschow
Copy link
Contributor Author

Updated to 120 bytes and confirmed to still work with feather-nrf52840-sense and my smartphone :)

@mguetschow mguetschow force-pushed the feather-nrf52840-alias branch from 90e49d3 to efef39f Compare April 4, 2025 08:55
@mguetschow
Copy link
Contributor Author

...and was so free to squash already

@crasbe
Copy link
Contributor

crasbe commented Apr 4, 2025

I'm still not super happy that everything is one commit, but I don't think it's necessary to split it up again.

@mguetschow mguetschow force-pushed the feather-nrf52840-alias branch from efef39f to 49abc17 Compare April 4, 2025 09:13
@mguetschow mguetschow force-pushed the feather-nrf52840-alias branch from 49abc17 to e92ac97 Compare April 4, 2025 09:13
@crasbe crasbe enabled auto-merge April 4, 2025 09:52
@crasbe crasbe added this pull request to the merge queue Apr 4, 2025
Merged via the queue into RIOT-OS:master with commit 801d19b Apr 4, 2025
25 checks passed
@mguetschow mguetschow deleted the feather-nrf52840-alias branch April 7, 2025 07:18
@mguetschow
Copy link
Contributor Author

Thanks for your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: doc Area: Documentation Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants