boards: set samd21-arduino-bootloader as DEFAULT_MODULE by fjmolinas · Pull Request #13664 · RIOT-OS/RIOT

fjmolinas · 2020-03-20T10:12:27Z

Contribution description

This PR proposes using DEFAULT_MODULE for the samd-arduino-bootloader. Since in #12304 it was stated that this MODULE was desired as the DEFAULT it will not be disabled automatically if another stdio_% is requested.
This could be done by moving the inclusion of dependencies to stdio.in.mk

ifneq (,$(filter $(filter-out stdio_cdc_acm,$(STDIO_MODULES)),$(USEMODULE)))
  # stdio_cdc_acm cannot be used when another STDIO is loaded
  DISABLE_MODULE += stdio_cdc_acm
  # boards_common_samd21-arduino-bootloader depends on stdio_cdc_acm so disable
  # boards_common_samd21-arduino-bootloader if stdio_cdc_acm is disabled
  DISABLE_MODULE += boards_common_samd21-arduino-bootloader
endif

ifneq (,$(filter boards_common_samd21-arduino-bootloader,$(USEMODULE)))
  # setup the samd21 arduino bootloader related dependencies
  include $(RIOTBOARD)/common/samd21-arduino-bootloader/Makefile.dep
endif

But this would change the current behavior.

It selects stdio_cdc_acm as a USEMODULE and not a DEFAULT_MODULE to make more explicit that id using arduino-bootloader disabling stdio_cdc_acm is simply not possible. So is forcing usage of two stdio_% module you would simply get an error.

Testing procedure

flashing should still work as before by default

BOARD=arduino-mkrfox1200 make -C tests/xtimer_usleep flash

stty -F /dev/ttyACM0 raw ispeed 1200 ospeed 1200 cs8 -cstopb ignpar eol 255 eof 255
sleep 1
"make" -C /home/francisco/workspace/RIOT/boards/arduino-mkrfox1200
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/boards/common/arduino-mkr
"make" -C /home/francisco/workspace/RIOT/cpu/samd21
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"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/samd21/periph
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common/periph
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/div
"make" -C /home/francisco/workspace/RIOT/sys/auto_init/usb
"make" -C /home/francisco/workspace/RIOT/sys/event
"make" -C /home/francisco/workspace/RIOT/sys/isrpipe
"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/test_utils/interactive_sync
"make" -C /home/francisco/workspace/RIOT/sys/tsrb
"make" -C /home/francisco/workspace/RIOT/sys/usb/usbus
"make" -C /home/francisco/workspace/RIOT/sys/usb_board_reset
"make" -C /home/francisco/workspace/RIOT/sys/usb/usbus/cdc/acm
"make" -C /home/francisco/workspace/RIOT/sys/xtimer
   text	   data	    bss	    dec	    hex	filename
  17548	    132	   5908	  23588	   5c24	/home/francisco/workspace/RIOT/tests/xtimer_usleep/bin/arduino-mkrfox1200/tests_xtimer_usleep.elf
/home/francisco/workspace/RIOT/dist/tools/bossa-1.9/bossac -p /dev/ttyACM0 -o 0x2000 -e -i -w -v -b -R /home/francisco/workspace/RIOT/tests/xtimer_usleep/bin/arduino-mkrfox1200/tests_xtimer_usleep.bin
Device       : ATSAMD21x18
Version      : v2.0 [Arduino:XYZ] Mar 10 2017 12:20:17
Address      : 0x0
Pages        : 4096
Page Size    : 64 bytes
Total Size   : 256KB
Planes       : 1
Lock Regions : 16
Locked       : none
Security     : false
BOD          : true
BOR          : true
Erase flash

Done in 0.842 seconds
Write 17680 bytes to flash (277 pages)
[==============================] 100% (277/277 pages)
Done in 0.113 seconds
Verify 17680 bytes of flash
[==============================] 100% (277/277 pages)
Verify successful
Done in 0.144 seconds
Set boot flash true
make: Leaving directory '/home/francisco/workspace/RIOT/tests/xtimer_usleep'

to use a different stdio_%, arduino bootloader needs to be disabled

USEMODULE+=stdio_rtt BOARD=arduino-mkrfox1200 make -C tests/xtimer_usleep FAILS

make: Entering directory '/home/francisco/workspace/RIOT/tests/xtimer_usleep'
Required modules were disabled using DISABLE_MODULE: stdio_cdc_acm stdio_rtt
/home/francisco/workspace/RIOT/tests/xtimer_usleep/../../Makefile.include:844: *** You can let the build continue on expected errors by setting CONTINUE_ON_EXPECTED_ERRORS=1 to the command line.  Stop.
make: Leaving directory '/home/francisco/workspace/RIOT/tests/xtimer_usleep'

DISABLE_MODULE=boards_common_samd21-arduino-bootloader USEMODULE+=stdio_rtt BOARD=arduino-mkrfox1200 make -C tests/xtimer_usleepOK

BOARD=arduino-mkrfox1200 make -C tests/xtimer_usleep
Building application "tests_xtimer_usleep" for "arduino-mkrfox1200" with MCU "samd21".

"make" -C /home/francisco/workspace/RIOT/boards/arduino-mkrfox1200
"make" -C /home/francisco/workspace/RIOT/boards/common/arduino-mkr
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/samd21
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common

If disabled none of the dependencies appear:

DISABLE_MODULE=boards_common_samd21-arduino-bootloader BOARD=arduino-mkrfox1200 make -C tests/xtimer_usleep info-modules --no-print-directory

auto_init
auto_init_xtimer
board
boards_common_arduino-mkr
core
core_init
core_msg
core_panic
cortexm_common
cortexm_common_periph
cpu
div
isrpipe
newlib
newlib_nano
newlib_syscalls_default
periph
periph_common
periph_gpio
periph_init
periph_init_common
periph_init_gpio
periph_init_init
periph_init_pm
periph_init_timer
periph_init_uart
periph_pm
periph_timer
periph_uart
pm_layered
sam0_common_periph
stdin
stdio_uart
stdio_uart_rx
sys
test_utils_interactive_sync
tsrb
xtimer

Issues/PRs references

Documentation on DEFAULT_MODULE use #13651
Extends on #12304
See #13650 (review) and #13650 (comment).

fjmolinas · 2020-03-20T11:54:49Z

Note the usage matches the current definition of DISABLE_MODULE:

export DISABLE_MODULE        # Used in the application's Makefile to suppress DEFAULT_MODULEs.

aabadie

Nice cleanup which makes sense. I'll test on feather-m0 later today.

boards/common/arduino-mkr/Makefile.dep

aabadie · 2020-03-24T13:35:18Z

Unfortunately, this doesn't build:

Details

make BOARD=feather-m0 -C examples/hello-world/ clean all --no-print-directory 
Building application "hello-world" for "feather-m0" with MCU "samd21".

"make" -C /work/riot/RIOT-review/boards/feather-m0
"make" -C /work/riot/RIOT-review/core
"make" -C /work/riot/RIOT-review/cpu/samd21
"make" -C /work/riot/RIOT-review/cpu/cortexm_common
"make" -C /work/riot/RIOT-review/cpu/cortexm_common/periph
"make" -C /work/riot/RIOT-review/cpu/sam0_common
"make" -C /work/riot/RIOT-review/cpu/sam0_common/periph
"make" -C /work/riot/RIOT-review/cpu/samd21/periph
"make" -C /work/riot/RIOT-review/drivers
"make" -C /work/riot/RIOT-review/drivers/periph_common
"make" -C /work/riot/RIOT-review/sys
"make" -C /work/riot/RIOT-review/sys/auto_init
"make" -C /work/riot/RIOT-review/sys/auto_init/usb
"make" -C /work/riot/RIOT-review/sys/event
"make" -C /work/riot/RIOT-review/sys/isrpipe
"make" -C /work/riot/RIOT-review/sys/newlib_syscalls_default
"make" -C /work/riot/RIOT-review/sys/pm_layered
"make" -C /work/riot/RIOT-review/sys/tsrb
"make" -C /work/riot/RIOT-review/sys/usb/usbus
"make" -C /work/riot/RIOT-review/sys/usb/usbus/cdc/acm
"make" -C /work/riot/RIOT-review/sys/usb_board_reset
arm-none-eabi-gcc: error: /work/riot/RIOT-review/examples/hello-world/bin/feather-m0/boards_common_samd21-arduino-bootloader.a: No such file or directory
make: *** [/work/riot/RIOT-review/examples/hello-world/../../Makefile.include:538: /work/riot/RIOT-review/examples/hello-world/bin/feather-m0/hello-world.elf] Error 1

The common samd21 arduino bootloader is not built.

fjmolinas · 2020-03-24T14:12:26Z

@aabadie added a fixup that should solve the issue.

aabadie · 2020-03-24T14:35:10Z

@fjmolinas, your last commit fixes the issue and looks good. Please squash!

fjmolinas · 2020-03-24T16:42:47Z

I pushed a change disabling the arduino bootloader for riotboot and minimal.

bootloaders/riotboot/Makefile

boards/common/sodaq/Makefile

aabadie

All good, ACK!

Let's now wait for Murdock.

aabadie · 2020-03-25T09:36:27Z

Ok, this needs a rebase now...

fjmolinas · 2020-03-25T10:10:56Z

Rebased!

aabadie · 2020-03-25T12:30:52Z

Murdock is still complaining. Any idea how to solve this @fjmolinas ?

fjmolinas · 2020-03-31T11:05:04Z

I pushed a fix that I think should fix the issue.

aabadie · 2020-03-31T11:40:43Z

I pushed a fix that I think should fix the issue.

Thanks but it still doesn't seems to work with Murdock.

fjmolinas · 2020-03-31T11:54:45Z

Thanks but it still doesn't seems to work with Murdock.

But a different failure!

fjmolinas · 2020-03-31T12:17:11Z

But a different failure!

I have the impression this is an issue with info-boards-supported. Boards that should not be present are, and boards that should arent. But only when bootloader_arduino is involved...

fjmolinas · 2020-04-01T11:59:35Z

@aabadie I changed the approach and removed the use of DEFAULT_MODULE. It uses regular dependencies to sort this out. Please take a second look.

Changed approach.

samd21-arduino-bootloader and its dependencies will only be included if no other stdio_% (other than stdio_cdc_acm) is included.

fjmolinas · 2020-04-01T14:20:05Z

@aabadie murdock is failing on unrelated issues. But otherwise its now OK. Let me know if you Ok with the new approach.

aabadie · 2020-04-01T14:37:30Z

Let me know if you Ok with the new approach.

I'm OK with the new approach, it looks cleaner.

aabadie

Changes make sense. I tested this PR on feather-m0 and it works as expected:

with the arduino bootloader

$ make BOARD=feather-m0 -C examples/hello-world/ flash term
make: Entering directory '/work/riot/RIOT-review/examples/hello-world'
Building application "hello-world" for "feather-m0" with MCU "samd21".

"make" -C /work/riot/RIOT-review/boards/feather-m0
"make" -C /work/riot/RIOT-review/boards/common/samd21-arduino-bootloader
"make" -C /work/riot/RIOT-review/core
"make" -C /work/riot/RIOT-review/cpu/samd21
"make" -C /work/riot/RIOT-review/cpu/cortexm_common
"make" -C /work/riot/RIOT-review/cpu/cortexm_common/periph
"make" -C /work/riot/RIOT-review/cpu/sam0_common
"make" -C /work/riot/RIOT-review/cpu/sam0_common/periph
"make" -C /work/riot/RIOT-review/cpu/samd21/periph
"make" -C /work/riot/RIOT-review/drivers
"make" -C /work/riot/RIOT-review/drivers/periph_common
"make" -C /work/riot/RIOT-review/sys
"make" -C /work/riot/RIOT-review/sys/auto_init
"make" -C /work/riot/RIOT-review/sys/auto_init/usb
"make" -C /work/riot/RIOT-review/sys/event
"make" -C /work/riot/RIOT-review/sys/isrpipe
"make" -C /work/riot/RIOT-review/sys/newlib_syscalls_default
"make" -C /work/riot/RIOT-review/sys/pm_layered
"make" -C /work/riot/RIOT-review/sys/tsrb
"make" -C /work/riot/RIOT-review/sys/usb/usbus
"make" -C /work/riot/RIOT-review/sys/usb/usbus/cdc/acm
"make" -C /work/riot/RIOT-review/sys/usb_board_reset
   text	   data	    bss	    dec	    hex	filename
  15316	    132	   5868	  21316	   5344	/work/riot/RIOT-review/examples/hello-world/bin/feather-m0/hello-world.elf
stty -F /dev/ttyACM0 raw ispeed 1200 ospeed 1200 cs8 -cstopb ignpar eol 255 eof 255
sleep 1
/work/riot/RIOT-review/dist/tools/bossa-1.9/bossac -p /dev/ttyACM0 -o 0x2000 -e -i -w -v -b -R /work/riot/RIOT-review/examples/hello-world/bin/feather-m0/hello-world.bin
Device       : ATSAMD21x18
Version      : v2.0 [Arduino:XYZ] Mar  5 2016 17:46:52
Address      : 0x0
Pages        : 4096
Page Size    : 64 bytes
Total Size   : 256KB
Planes       : 1
Lock Regions : 16
Locked       : none
Security     : false
BOD          : true
BOR          : true
Erase flash

Done in 0.809 seconds
Write 15448 bytes to flash (242 pages)
[==============================] 100% (242/242 pages)
Done in 0.089 seconds
Verify 15448 bytes of flash
[==============================] 100% (242/242 pages)
Verify successful
Done in 0.149 seconds
Set boot flash true
sleep 2
/work/riot/RIOT-review/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" 
2020-04-01 21:38:34,445 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2020-04-01 21:38:35,449 # 387b-review_samd21_bootloader)
2020-04-01 21:38:35,450 # Hello World!
2020-04-01 21:38:35,452 # You are running RIOT on a(n) feather-m0 board.
2020-04-01 21:38:35,453 # This board features a(n) samd21 MCU.
2020-04-01 21:38:36,353 # Exiting Pyterm

without the arduino bootloader

$ USEMODULE=stdio_null make BOARD=feather-m0 -C examples/hello-world/ flash term
make: Entering directory '/work/riot/RIOT-review/examples/hello-world'
Building application "hello-world" for "feather-m0" with MCU "samd21".

"make" -C /work/riot/RIOT-review/boards/feather-m0
"make" -C /work/riot/RIOT-review/core
"make" -C /work/riot/RIOT-review/cpu/samd21
"make" -C /work/riot/RIOT-review/cpu/cortexm_common
"make" -C /work/riot/RIOT-review/cpu/cortexm_common/periph
"make" -C /work/riot/RIOT-review/cpu/sam0_common
"make" -C /work/riot/RIOT-review/cpu/sam0_common/periph
"make" -C /work/riot/RIOT-review/cpu/samd21/periph
"make" -C /work/riot/RIOT-review/drivers
"make" -C /work/riot/RIOT-review/drivers/periph_common
"make" -C /work/riot/RIOT-review/sys
"make" -C /work/riot/RIOT-review/sys/auto_init
"make" -C /work/riot/RIOT-review/sys/newlib_syscalls_default
"make" -C /work/riot/RIOT-review/sys/pm_layered
"make" -C /work/riot/RIOT-review/sys/stdio_null
   text	   data	    bss	    dec	    hex	filename
   7756	    112	   2544	  10412	   28ac	/work/riot/RIOT-review/examples/hello-world/bin/feather-m0/hello-world.elf
stty -F /dev/ttyACM0 raw ispeed 1200 ospeed 1200 cs8 -cstopb ignpar eol 255 eof 255
sleep 1
/work/riot/RIOT-review/dist/tools/bossa-1.9/bossac -p /dev/ttyACM0 -o 0x2000 -e -i -w -v -b -R /work/riot/RIOT-review/examples/hello-world/bin/feather-m0/hello-world.bin
Device       : ATSAMD21x18
Version      : v2.0 [Arduino:XYZ] Mar  5 2016 17:46:52
Address      : 0x0
Pages        : 4096
Page Size    : 64 bytes
Total Size   : 256KB
Planes       : 1
Lock Regions : 16
Locked       : none
Security     : false
BOD          : true
BOR          : true
Erase flash

Done in 0.809 seconds
Write 7868 bytes to flash (123 pages)
[==============================] 100% (123/123 pages)
Done in 0.045 seconds
Verify 7868 bytes of flash
[==============================] 100% (123/123 pages)
Verify successful
Done in 0.092 seconds
Set boot flash true
sleep 2
/work/riot/RIOT-review/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" 
2020-04-01 21:39:02,399 # Connect to serial port /dev/ttyACM0
2020-04-01 21:39:02,399 # could not open port /dev/ttyACM0: [Errno 2] No such file or directory: '/dev/ttyACM0'

In this case, the board cannot be reflashed automatically anymore. So one has to trigger the bootloader mode manually by double tapping the reset button.

ACK and go!

fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Mar 20, 2020

fjmolinas requested a review from aabadie March 20, 2020 10:12

fjmolinas mentioned this pull request Mar 20, 2020

Makefile.dep: DEFAULT_MODULE += stdio_rtt will always load dependencies - even if other stdio is selected #13460

Closed

aabadie self-assigned this Mar 24, 2020

aabadie requested changes Mar 24, 2020

View reviewed changes

boards/common/arduino-mkr/Makefile.dep Outdated Show resolved Hide resolved

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 30fc716 to 0b6907f Compare March 24, 2020 13:12

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 3a3571c to 2c00298 Compare March 24, 2020 14:39

aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 24, 2020

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 2c00298 to 8fa2017 Compare March 24, 2020 16:39

fjmolinas requested review from MichelRottleuthner, leandrolanzieri and smlng as code owners March 24, 2020 16:39

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 8fa2017 to 4567395 Compare March 24, 2020 16:40

aabadie reviewed Mar 24, 2020

View reviewed changes

bootloaders/riotboot/Makefile Outdated Show resolved Hide resolved

aabadie reviewed Mar 25, 2020

View reviewed changes

boards/common/sodaq/Makefile Show resolved Hide resolved

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 4567395 to b9acff4 Compare March 25, 2020 08:59

aabadie previously approved these changes Mar 25, 2020

View reviewed changes

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 25, 2020

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from b9acff4 to fc232c3 Compare March 25, 2020 10:10

aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 25, 2020

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from fc232c3 to 4761470 Compare March 31, 2020 11:04

fjmolinas mentioned this pull request Mar 31, 2020

boards/adafruit-clue: add initial support #13276

Merged

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 4761470 to 32467db Compare April 1, 2020 11:58

boards: conditionally include samd21-arduino-bootloader

1387b73

samd21-arduino-bootloader and its dependencies will only be included if no other stdio_% (other than stdio_cdc_acm) is included.

fjmolinas force-pushed the pr_samd21_bootloader_default_module branch from 32467db to 1387b73 Compare April 1, 2020 13:10

aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 1, 2020

aabadie approved these changes Apr 1, 2020

View reviewed changes

aabadie merged commit cd0b765 into RIOT-OS:master Apr 1, 2020

fjmolinas deleted the pr_samd21_bootloader_default_module branch April 1, 2020 20:01

leandrolanzieri added this to the Release 2020.04 milestone Apr 2, 2020

aabadie mentioned this pull request Apr 2, 2020

boards/samd21-bootloader*: small cleanup #13794

Merged

Conversation

fjmolinas commented Mar 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

fjmolinas commented Mar 20, 2020

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aabadie commented Mar 24, 2020

Uh oh!

fjmolinas commented Mar 24, 2020

Uh oh!

aabadie commented Mar 24, 2020

Uh oh!

fjmolinas commented Mar 24, 2020

Uh oh!

Uh oh!

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

aabadie commented Mar 25, 2020

Uh oh!

fjmolinas commented Mar 25, 2020

Uh oh!

aabadie commented Mar 25, 2020

Uh oh!

fjmolinas commented Mar 31, 2020

Uh oh!

aabadie commented Mar 31, 2020

Uh oh!

fjmolinas commented Mar 31, 2020

Uh oh!

fjmolinas commented Mar 31, 2020

Uh oh!

fjmolinas commented Apr 1, 2020

Uh oh!

fjmolinas commented Apr 1, 2020

Uh oh!

aabadie commented Apr 1, 2020

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fjmolinas commented Mar 20, 2020 •

edited

Loading