Skip to content

boards/flash.sh: rename HEXFILE to BINFILE#10471

Merged
jcarrano merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/flash_sh/use_binfile
Nov 27, 2018
Merged

boards/flash.sh: rename HEXFILE to BINFILE#10471
jcarrano merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/flash_sh/use_binfile

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Nov 26, 2018 •
edited
Loading

Contribution description

Flasher are doing 'loadbin' so use BINFILE name.

In Makefile.include it is already setting HEXFILE to BINFILE

RIOT/boards/cc2538dk/Makefile.include

Line 34 in ce8815f

HEXFILE = $(BINFILE)

RIOT/boards/common/remote/Makefile.include

Line 28 in ce8815f

HEXFILE = $(BINFILE)

Testing procedure

Try flashing for cc2538dk or boards depending on remote/common:

You should get the same output for master and this PR with:

PROGRAMMER=jlink BOARD=cc2538dk make --no-print-directory -C examples/hello-world/ flash-only FLASHER='bash -x $(RIOTBOARD)/$(BOARD)/dist/flash.sh'

WARNING: by testing this, I noticed boards depending on remote/common cannot flash using PROGRAMMER==jlink since

4bcb353#diff-5143693651d6b10f67f159d2f8f11d47 EDIT issue was even older it was already broken by 8d07c01#diff-9f122f733b5127db6637053c7f2bbd4f

I should do a PR to fix this before this one…

PR in #10472

Issues/PRs references

Split from these PRs that try to declare BINFILE and FLASHFILE variables

@cladmi
boards/flash.sh: rename HEXFILE to BINFILE
52b0b18 
Flasher are doing 'loadbin' so use BINFILE name.
@cladmi cladmi added State: waiting for other PR State: The PR requires another PR to be merged first Area: tools Area: Supplementary tools Area: boards Area: Board ports labels Nov 26, 2018
This was referenced Nov 26, 2018
Merged
Merged
@cladmi cladmi requested a review from jcarrano November 26, 2018 16:13
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 27, 2018

I just noticed that instead of updating these scripts, the boards could just be updated to use the common dist/tools/jlink/jlink.sh. This would remove code duplication and solve everything at once when updating jlink.sh.

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 27, 2018

But as I do not have the boards and even JlinkExe (when installing from https://www.segger.com/downloads/jlink/#J-LinkSoftwareAndDocumentationPack it was called jlink), it would be hard to configure the required values for me.

@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Nov 27, 2018
jcarrano
jcarrano approved these changes Nov 27, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

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

Tested (no JLinkExe, but it should not matter.)

@jcarrano
Copy link
Copy Markdown
Contributor

jcarrano commented Nov 27, 2018

As a side comment, that command line argument handling in flash.sh is pretty flimsy.

@jcarrano jcarrano merged commit bac4327 into RIOT-OS:master Nov 27, 2018
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 27, 2018

It is used as an internal script only, so I think it is not needed to not do an extensive testing on arguments.

@cladmi cladmi deleted the pr/flash_sh/use_binfile branch November 27, 2018 17:07
@aabadie aabadie added this to the Release 2019.01 milestone Dec 2, 2018
@miri64 miri64 added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jcarrano jcarrano jcarrano approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: tools Area: Supplementary tools 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

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

4 participants

@cladmi @jcarrano @miri64 @aabadie