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
boards/pba-d-01-kw2x: rely on the common adapter selection code #12151
Merged
fjmolinas
merged 1 commit into
RIOT-OS:master
from
cladmi:pr/pba-d-01-kw2x/do_not_use_OPENOCD_ADAPTER_INIT
Sep 23, 2019
Merged
boards/pba-d-01-kw2x: rely on the common adapter selection code #12151
fjmolinas
merged 1 commit into
RIOT-OS:master
from
cladmi:pr/pba-d-01-kw2x/do_not_use_OPENOCD_ADAPTER_INIT
Sep 23, 2019
Conversation
This file contains 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
Do not set 'OPENOCD_EXTRA_INIT' but rely on 'openocd/openocd-adapters/dap.inc.mk' to select the adapter. Using 'OPENOCD_EXTRA_INIT' for this was deprecated. When 'DEBUG_ADAPTER_ID' was set it was already the case but not with 'SERIAL'. The compatibility with using 'SERIAL' is maintained.
miri64
added
Area: boards
Area: Board ports
Area: build system
Area: Build system
Type: enhancement
The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
labels
Sep 10, 2019
Testing Procedure:
DEBUG_ADAPTER_ID=02000203C3194E743EE5B38C BOARD=pba-d-01-kw2x BUILD_IN_DOCKER=1 RIOT_CI_BUILD=1 make --no-print-directory -C examples/hello-world/ flash
SERIAL=02000203C3194E743EE5B38C BOARD=pba-d-01-kw2x BUILD_IN_DOCKER=1 RIOT_CI_BUILD=1 make --no-print-directory -C examples/hello-world/ flash
|
fjmolinas
added
CI: ready for build
If set, CI server will compile all applications for all available boards for the labeled PR
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
Sep 23, 2019
fjmolinas
approved these changes
Sep 23, 2019
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 was able to reproduce the testing procedure, change make sense. ACK
Thank you for the 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
CI: ready for build
If set, CI server will compile all applications for all available boards for the labeled PR
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
Type: enhancement
The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
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.
Contribution description
Do not set 'OPENOCD_EXTRA_INIT' but rely on
'openocd/openocd-adapters/dap.inc.mk' to select the adapter.
Using 'OPENOCD_EXTRA_INIT' for this was deprecated.
When 'DEBUG_ADAPTER_ID' was set it was already the case but not with 'SERIAL'.
The compatibility with using 'SERIAL' is maintained.
Review procedure
No other boards is setting
OPENOCD_EXTRA_INIT
:git grep OPENOCD_EXTRA_INIT
The configuration of the adapter does end up in almost the same place with
OPENOCD_ADAPTER_INIT
RIOT/dist/tools/openocd/openocd.sh
Lines 353 to 355 in f74381c
And is the way we use for many other boards:
git grep '$(DEBUG_ADAPTER_ID)'
Testing procedure
Note: I will set
SERIAL_TTY
from the command line to not trigger the usb listing when using random serial number.With this PR, the selection to
OPENOCD_ADAPTER_INIT
works with bothSERIAL
andDEBUG_ADAPTER_ID
:In master we had the same behavior with
DEBUG_ADAPTER_ID
And when using
SERIAL
this was handled throughOPENOCD_EXTRA_INIT
instead ofOPENOCD_ADAPTER_INIT
When multiple boards are connected, flashing is correctly handled:
No serial configuration fails (for reference)
The SERIAL handling works
And it still works with `DEBUG_ADAPTER_ID`
Issues/PRs references
Found while reviewing #11976