Skip to content
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

make: allow to override RESET and RESET_FLAGS for all boards and tools #11649

Merged
merged 10 commits into from
Jun 7, 2019

Conversation

smlng
Copy link
Member

@smlng smlng commented Jun 6, 2019

Contribution description

This PR adapts Makefiles to allow for overriding the RESET command (or tool) and required RESET_FLAGS (if any present). For some boards like sam0 based once this is already possible, this PR adds this for all remaining.

This is required to allow for custom or special reset commands, for instance when running hardware-in-the-loop tests in a well known (special) environment which might allow to reset a board by controlling the power via a side-channel.

Testing procedure

try to run make reset for you favourite board that should still work. You may also try to run RESET="" RESET_FLAGS="" make reset which than should fail to reset the board.

Issues/PRs references

@smlng smlng added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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 labels Jun 6, 2019
@smlng smlng requested review from cladmi and MrKevinWeiss June 6, 2019 14:34
@MrKevinWeiss
Copy link
Contributor

heh piling up those commits 😆

@smlng
Copy link
Member Author

smlng commented Jun 6, 2019

heh piling up those commits 😆

that's (really) just so we can identify and revert if a certain board or tool fails 😬

@smlng smlng added this to the Release 2019.07 milestone Jun 7, 2019
@smlng smlng added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation and removed Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Jun 7, 2019
@cladmi
Copy link
Contributor

cladmi commented Jun 7, 2019

Would be good to verify all the values are still the same with info-debug-variable-RESET and info-debug-variable-RESET_FLAGS for the different configurations and boards.

@cladmi
Copy link
Contributor

cladmi commented Jun 7, 2019

If you want to prevent the pattern to be re-appearing in the future, you can add a detection for it in https://github.com/cladmi/RIOT/blob/master/dist/tools/buildsystem_sanity_check/check.sh

I checked the output right now.

#! /bin/sh
BOARDS=$(find boards/* -maxdepth 0 -type d \! -name "common" -exec basename {} \;)

list_all_variables() {
    for BOARD in ${BOARDS}
    do
        export BOARD
        echo ${BOARD}
        make --no-print-directory -C examples/hello-world/ info-debug-variable-RESET info-debug-variable-RESET_FLAGS
    done
}

list_all_variables | tee $1_output_simple
PROGRAMMER=jlink list_all_variables | tee $1_output_jlink

And the output of RESET and RESET_FLAGS is the same also with PROGRAMMER=jlink.

The uniflash 3 looks good even if not re-tested locally.

Copy link
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

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

Agreed and tested that it did not change the output of RESET and RESET_FLAGS in the default and jlink case.

@cladmi cladmi merged commit b9c15c3 into RIOT-OS:master Jun 7, 2019
@cladmi
Copy link
Contributor

cladmi commented Jun 7, 2019

It is also a task I would like to have with FLASHER and FFLAGS in general, but the review will be a bit trickier I think.

@smlng
Copy link
Member Author

smlng commented Jun 7, 2019

thanks @cladmi for review and merge!!

@smlng smlng deleted the pr/make/reset branch June 7, 2019 10:14
@MrKevinWeiss MrKevinWeiss added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer 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