Skip to content

boards: model nrf52 boards#17299

Merged
leandrolanzieri merged 11 commits intoRIOT-OS:masterfrom
fjmolinas:pr_nrf52_boards_kconfig
Dec 8, 2021
Merged

boards: model nrf52 boards#17299
leandrolanzieri merged 11 commits intoRIOT-OS:masterfrom
fjmolinas:pr_nrf52_boards_kconfig

Conversation

@fjmolinas
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas commented Nov 30, 2021

Contribution description

Follow up to #16837 this models the remaining BOARDs
I had to do multiple changes to model this one, so commits are better checked one by one.

Testing procedure

  • Gren CI
  • Check that driver modeling still makes sense
  • Check that default bootloader config makes sense

Issues/PRs references

part of #16875

@github-actions github-actions bot added Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: SAUL Area: Sensor/Actuator Uber Layer Area: sys Area: System Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Nov 30, 2021
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 1, 2021
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Dec 1, 2021
@fjmolinas fjmolinas force-pushed the pr_nrf52_boards_kconfig branch from cdfd7ef to 919fefb Compare December 6, 2021 21:56
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Dec 7, 2021

I had quite some trouble with this one, the common nrf52 dependencies are a complete mess: some select the common boards/nrf* something dependencies, but not the features, or the dependencies but not the common module, or the features and not the common module. Anyway, seems like its finally right, can I squash @leandrolanzieri to clean this up a bit?

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Dec 7, 2021

I had quite some trouble with this one, the common nrf52 dependencies are a complete mess: some select the common boards/nrf* something dependencies, but not the features, or the dependencies but not the common module, or the features and not the common module. Anyway, seems like its finally right, can I squash @leandrolanzieri to clean this up a bit?

Sure, go ahead. It's gonna be easier to review

@fjmolinas fjmolinas force-pushed the pr_nrf52_boards_kconfig branch 2 times, most recently from 0e916a7 to 2792097 Compare December 7, 2021 11:11
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Dec 7, 2021

Squashed

leandrolanzieri
leandrolanzieri reviewed Dec 7, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me already. Just some minor nits. Let's see what the CI thinks

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Dec 7, 2021

@leandrolanzieri can I squash if green and remove the .murdock change?

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Dec 7, 2021

@leandrolanzieri can I squash if green and remove the .murdock change?

Go ahead

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Dec 7, 2021

Damn, recent changes broke this PR again... :( will adapt.

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Dec 7, 2021

Damn, recent changes broke this PR again... :( will adapt.

Changes upstream

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Dec 8, 2021

Also 10a5d96 seems unrelated

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Dec 8, 2021

@fjmolinas you still need to remove 03bb3a6 and pick one board for testing.

There are already nrf52 boards listed.

@fjmolinas fjmolinas force-pushed the pr_nrf52_boards_kconfig branch from 03bb3a6 to a699857 Compare December 8, 2021 09:40
@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Dec 8, 2021
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 8, 2021
Merged
@leandrolanzieri leandrolanzieri added this to the Release 2022.01 milestone Dec 8, 2021
leandrolanzieri
leandrolanzieri reviewed Dec 8, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I just spotted this now. Also there is a typo in the message of f54fdf3

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Dec 8, 2021

Feel free to squash directly

@fjmolinas fjmolinas force-pushed the pr_nrf52_boards_kconfig branch from a699857 to 766bfea Compare December 8, 2021 12:25
@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Dec 8, 2021
leandrolanzieri
leandrolanzieri approved these changes Dec 8, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, this was tested on the CI over all nrf52 boards.

@leandrolanzieri leandrolanzieri merged commit 15165bf into RIOT-OS:master Dec 8, 2021
@fjmolinas fjmolinas deleted the pr_nrf52_boards_kconfig branch December 8, 2021 13:58
@aabadie aabadie mentioned this pull request May 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@smlng smlng Awaiting requested review from smlng

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: SAUL Area: Sensor/Actuator Uber Layer Area: sys Area: System 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 Platform: ARM Platform: This PR/issue effects ARM-based platforms Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines

Projects

None yet

Milestone

Release 2022.01

Development

Successfully merging this pull request may close these issues.

2 participants

@fjmolinas @leandrolanzieri