Makefile.include: set default PROGRAMMER value when flashing on IoT-LAB #12317

aabadie · 2019-09-27T15:19:04Z

Contribution description

This will avoid to load all edbg programmer management logic from a board Makefile.include and especially will avoid to add edbg to the list of FLASHDEPS: it won't be unnecessarily built when flashing a samr21-xpro/arduino-zero board on IoT-LAB.

The idea is to set a default value to the PROGRAMMER variable when IOTLAB_NODE is set (the arbitrary iotlab value is used) before the Makefile.include from the board is included. This way all edbg logic is skipped (including the addition to FLASHDEPS).

Also note that in master, issuing a flash command for samr21-xpro or arduino-zero from an IoT-LAB SSH Frontend is broken because edbg cannot be built on the fly (due to missing libudev libraries).

@cladmi, what's opinion on this fix ?

Testing procedure

Start an experiment on IoT-LAB with a samr21-xpro or arduino-zero board:

$ iotlab-experiment submit -n "" -d 20 -l saclay,samr21,1
$ iotlab-experiment wait

2 cases:

Locally, ensure that you don't have edbg already built/available (in dist/tools) and flash the board from your machine:
```
$ make BOARD=samr21-xpro IOTLAB_NODE=auto-ssh -C examples/hello-world
```
=> on master, edbg is rebuilt before flashing on IoT-LAB. With this PR, only iotlab-node --update is called

On the SSH frontend:

$ ssh <login>@saclay.iot-lab.info
$ cd RIOT
$ make BOARD=samr21-xpro IOTLAB_NODE=auto-ssh -C examples/hello-world

=> on master, the last command fails. With this PR, the board is flashed correctly.

Issues/PRs references

Related to #9694

cladmi · 2019-09-27T17:12:12Z

Makefile.include

@@ -106,6 +106,11 @@ OS := $(shell uname)
 # set python path, e.g. for tests
 PYTHONPATH := $(RIOTBASE)/dist/pythonlibs/:$(PYTHONPATH)

+# set default PROGRAMMER value if flashing a board on IoT-LAB
+ifneq (,$(IOTLAB_NODE))
+  PROGRAMMER = iotlab


Keep it with ?= to allow easily overwriting it from the environment in case it is required.

Done and comment added.

cladmi · 2019-09-27T17:13:08Z

It is possible to define an unknown PROGRAMMER without errors since your cleanup in b3b0cd9 and a4463be

Unfortunately, right now this must indeed be done at the beginning and processing the iotlab file later. Maybe add a comment about it.

I could currently see some issue when building the nordic_softdevice that does specific handling depending on the PROGRAMMER but it would not flash on IoT-LAB anyway as it modifies the flash command. There may also be other boards that link differently, but it does not mean the default behavior would work anyway. If defined with ?= it could always be overwritten from command line.

aabadie · 2019-09-27T18:09:19Z

right now this must indeed be done at the beginning and processing the iotlab file later. Maybe add a comment about it.

I'll add a comment about that. Do you think the actual position is ok ?

If defined with ?= it could always be overwritten from command line.

That would be cleaner indeed. I'll change for that.

fjmolinas · 2019-09-30T09:28:59Z

@cladmi are you ok with the changes now?

cladmi · 2019-09-30T14:04:22Z

I'll add a comment about that. Do you think the actual position is ok ?

Comment is good.

I did not thought about the location. As your comment is saying, it does not need to be that early. Before docker is even maybe problematic or implying there is a reason for be there.
Somehow just after the PREFIX definition could be good. It would be before all the board parsing. And no need to be after the INCLUDES or Makefile.features as it would be confusing.

This will avoid to load edbg logic and especially adds it to the list of FLASHDEPS: it won't be unnecessarily built when flashing a samr21-xpro/arduino-zero board on IoT-LAB

cladmi · 2019-09-30T14:17:41Z

👍

aabadie · 2019-09-30T14:17:46Z

Somehow just after the PREFIX definition could be good. It would be before all the board parsing. And no need to be after the INCLUDES or Makefile.features as it would be confusing.

I moved the change right after PREFIX is exported. Directly amended.

aabadie · 2019-09-30T15:11:54Z

@fjmolinas, I think we are good here. Any chance you could try and ACK ?

fjmolinas

Changes look good and make sense, I can see the issue and that this fixes it:

master: make BOARD=samr21-xpro IOTLAB_NODE=auto-ssh -C examples/hello-world flash

make BOARD=samr21-xpro IOTLAB_NODE=auto-ssh -C examples/hello-world flash
make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Building application "hello-world" for "samr21-xpro" with MCU "samd21".

"make" -C /home/francisco/workspace/RIOT/boards/samr21-xpro
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/samd21
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/samd21/periph
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/newlib_syscalls_default
"make" -C /home/francisco/workspace/RIOT/sys/pm_layered
"make" -C /home/francisco/workspace/RIOT/sys/stdio_uart
   text	   data	    bss	    dec	    hex	filename
   8484	    120	   2560	  11164	   2b9c	/home/francisco/workspace/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf
[INFO] edbg binary not found - building it from source now
CC= CFLAGS= make -C /home/francisco/workspace/RIOT/dist/tools/edbg
rm -Rf /home/francisco/workspace/RIOT/dist/tools/edbg/bin
mkdir -p /home/francisco/workspace/RIOT/dist/tools/edbg/bin
/home/francisco/workspace/RIOT/dist/tools/git/git-cache clone "https://github.com/ataradov/edbg" "4f5d490bfffc7fd10855e513e6e88be5a9a3f789" "/home/francisco/workspace/RIOT/dist/tools/edbg/bin"
Cloning into '/home/francisco/workspace/RIOT/dist/tools/edbg/bin'

In iotlab:

make: Entering directory '/senslab/users/molina/RIOT/examples/hello-world'
[INFO] edbg binary not found - building it from source now
CC= CFLAGS= make -C /senslab/users/molina/RIOT/dist/tools/edbg
if [ 4f5d490bfffc7fd10855e513e6e88be5a9a3f789 != 4f5d490bfffc7fd10855e513e6e88be5a9a3f789 ] ; then \
	git -C /senslab/users/molina/RIOT/dist/tools/edbg/bin clean -xdff ; \
	git -C /senslab/users/molina/RIOT/dist/tools/edbg/bin fetch "https://github.com/ataradov/edbg" "4f5d490bfffc7fd10855e513e6e88be5a9a3f789" ; \
	git -C /senslab/users/molina/RIOT/dist/tools/edbg/bin checkout -f 4f5d490bfffc7fd10855e513e6e88be5a9a3f789 ; \
	touch /senslab/users/molina/RIOT/dist/tools/edbg/bin/.git-downloaded ; \
fi
env -i PATH="/opt/gcc-arm-none-eabi-7-2018-q2-update/bin:/opt/gcc-arm-none-eabi-4_9-2015q1/bin:/opt/mspgcc-3.2.3/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/usr/lib/jvm/java-8-oracle/bin:/usr/lib/jvm/java-8-oracle/db/bin:/usr/lib/jvm/java-8-oracle/jre/bin" TERM="xterm-256color" "make" -C "/senslab/users/molina/RIOT/dist/tools/edbg/bin"
make: Entering directory '/senslab/users/molina/RIOT/dist/tools/edbg/bin'
gcc -W -Wall -Wextra -O2 -std=gnu11 dap.c edbg.c target.c target_atmel_cm0p.c target_atmel_cm3.c target_atmel_cm4.c target_atmel_cm7.c target_atmel_cm4v2.c target_mchp_cm23.c  dbg_lin.c -ludev -o edbg
dbg_lin.c:42:21: fatal error: libudev.h: No such file or directory
 #include <libudev.h>
                     ^
compilation terminated.
Makefile:46: recipe for target 'edbg' failed
make: *** [edbg] Error 1
make: Leaving directory '/senslab/users/molina/RIOT/dist/tools/edbg/bin'
Makefile:13: recipe for target 'all' failed
make[1]: *** [all] Error 2
/senslab/users/molina/RIOT/makefiles/tools/targets.inc.mk:16: recipe for target '/senslab/users/molina/RIOT/dist/tools/edbg/edbg' failed
make: *** [/senslab/users/molina/RIOT/dist/tools/edbg/edbg] Error 2
make: Leaving directory '/senslab/users/molina/RIOT/examples/hello-world'

pr: make BOARD=samr21-xpro IOTLAB_NODE=auto-ssh -C examples/hello-world flash

make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Building application "hello-world" for "samr21-xpro" with MCU "samd21".

"make" -C /home/francisco/workspace/RIOT/boards/samr21-xpro
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/samd21
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/samd21/periph
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/newlib_syscalls_default
"make" -C /home/francisco/workspace/RIOT/sys/pm_layered
"make" -C /home/francisco/workspace/RIOT/sys/stdio_uart
   text	   data	    bss	    dec	    hex	filename
   8488	    120	   2560	  11168	   2ba0	/home/francisco/workspace/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf
iotlab-node --jmespath='keys(@)[0]' --format='int'  --list saclay,samr21,1 --update /home/francisco/workspace/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf | grep 0
0
make: Leaving directory '/home/francisco/workspace/RIOT/examples/hello-world'

In iotlab fronted:

make BOARD=samr21-xpro IOTLAB_NODE=auto-ssh -C examples/hello-world flash-only
make: Entering directory '/senslab/users/molina/RIOT/examples/hello-world'
iotlab-node --jmespath='keys(@)[0]' --format='int'  --list saclay,samr21,1 --update /senslab/users/molina/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf | grep 0
0
make: Leaving directory '/senslab/users/molina/RIOT/examples/hello-world'

ACK

aabadie · 2019-10-01T07:49:11Z

Thanks for reviewing @cladmi and @fjmolinas !

aabadie added Area: build system Area: Build system Area: boards Area: Board ports labels Sep 27, 2019

aabadie changed the title ~~Makefile.include: set default PROGRAMMER value when flahsing on IoT-LAB~~ Makefile.include: set default PROGRAMMER value when flashing on IoT-LAB Sep 27, 2019

aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 27, 2019

aabadie force-pushed the pr/make/iotlab_node_flash branch from d296051 to c81c1cc Compare September 27, 2019 15:32

cladmi reviewed Sep 27, 2019

View reviewed changes

aabadie force-pushed the pr/make/iotlab_node_flash branch from c81c1cc to 556be74 Compare September 27, 2019 18:11

Makefile.include: set default PROGRAMMER value when flahsing on IoT-LAB

06123ef

This will avoid to load edbg logic and especially adds it to the list of FLASHDEPS: it won't be unnecessarily built when flashing a samr21-xpro/arduino-zero board on IoT-LAB

aabadie force-pushed the pr/make/iotlab_node_flash branch from 556be74 to 06123ef Compare September 30, 2019 14:16

fjmolinas approved these changes Oct 1, 2019

View reviewed changes

fjmolinas merged commit c215deb into RIOT-OS:master Oct 1, 2019

aabadie deleted the pr/make/iotlab_node_flash branch October 1, 2019 07:48

aabadie mentioned this pull request Oct 8, 2019

make/testbed-support: fix error with FLASHFILE not defined #12394

Merged

aabadie mentioned this pull request Oct 18, 2019

make/testbed-support: fix error with FLASHFILE not defined [backport 2019.10] #12493

Merged

kb2ma added this to the Release 2019.10 milestone Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Makefile.include: set default PROGRAMMER value when flashing on IoT-LAB #12317

Makefile.include: set default PROGRAMMER value when flashing on IoT-LAB #12317

aabadie commented Sep 27, 2019

cladmi Sep 27, 2019

aabadie Sep 27, 2019

cladmi commented Sep 27, 2019

aabadie commented Sep 27, 2019

fjmolinas commented Sep 30, 2019

cladmi commented Sep 30, 2019

cladmi commented Sep 30, 2019

aabadie commented Sep 30, 2019

aabadie commented Sep 30, 2019

fjmolinas left a comment

aabadie commented Oct 1, 2019

Makefile.include: set default PROGRAMMER value when flashing on IoT-LAB #12317

Makefile.include: set default PROGRAMMER value when flashing on IoT-LAB #12317

Conversation

aabadie commented Sep 27, 2019

Contribution description

Testing procedure

Issues/PRs references

cladmi Sep 27, 2019

Choose a reason for hiding this comment

aabadie Sep 27, 2019

Choose a reason for hiding this comment

cladmi commented Sep 27, 2019

aabadie commented Sep 27, 2019

fjmolinas commented Sep 30, 2019

cladmi commented Sep 30, 2019

cladmi commented Sep 30, 2019

aabadie commented Sep 30, 2019

aabadie commented Sep 30, 2019

fjmolinas left a comment

Choose a reason for hiding this comment

aabadie commented Oct 1, 2019