NRF52 Board Code Deduplication #1201
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I have created a common NRF52Board when I tried to add in built-in temperature. When merged, we can remove duplicated code. Copy and paste code is not a way to go. |
fschrempf
force-pushed
the
nrf52-board-deduplication
branch
from
c2e37eb to
4ad9903
Compare
Contributor
Author
|
Force-pushed some clang-format fixes in the changed code and rebased to current dev. |
fschrempf
force-pushed
the
nrf52-board-deduplication
branch
from
4ad9903 to
ebfbad4
Compare
Signed-off-by: Frieder Schrempf <[email protected]>
The reboot() method is the same for all NRF52 boards. Use a shared implementation. Signed-off-by: Frieder Schrempf <[email protected]>
Use a common begin() method that can be called from derived classes to contain the shared initialization code. Signed-off-by: Frieder Schrempf <[email protected]>
Some NRF52 boards are able to use the internal power-efficient DC/DC regulator. Add a new class that can be inherited by board classes to enable this feature and reduce the power consumption. Signed-off-by: Frieder Schrempf <[email protected]>
The startOTAUpdate() is the same for all NRF52 boards. Use a common implementation for all boards that currently have a specific implementation. The following boards currently have an empty startOTAUpdate() for whatever reasons and therefore are not inheriting NRF52BoardOTA to keep the same state: Nano G2 Ultra, Seeed SenseCAP T1000-E, Wio WM1110. Signed-off-by: Frieder Schrempf <[email protected]>
fschrempf
force-pushed
the
nrf52-board-deduplication
branch
from
ebfbad4 to
aeab413
Compare
The function in the derived class is virtual per definition. Mark it to make this clearer to the reader. Signed-off-by: Frieder Schrempf <[email protected]>
The RAK4631/RAK4630 module are able to use the DC/DC converter. Enable it to reduce power consumption. Signed-off-by: Frieder Schrempf <[email protected]>
fschrempf
force-pushed
the
nrf52-board-deduplication
branch
from
2040a24 to
8eb229b
Compare
Contributor
Author
|
Now rebased to current dev. @ripplebiz @recrof @fdlamotte May I kindly ask if this could be considered for reviewing and merging? Keeping this up to date with |
Collaborator
would love to, but only @liamcottle @ripplebiz and @fdlamotte have merge rights. |
Contributor
Author
|
@fdlamotte Thanks a lot! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was a bit disappointed to see the loads of duplicated code and seemingly nobody being bothered by the increasing amount of copy and paste when adding new hardware variants.
So I tried to "be the change I want to see" and started to deduplicate the NRF52 board code. Except for the last commit, these changes are intended to be purely refactoring and not change the functional behavior.
The last commit enables the DC/DC converter for the RAK4631 hardware. The documentation says that the DC/DC converter can be used and it was tested to decrease the power consumption significantly.