Skip to content

sys/arduino: include arduino_sketches in Makefile.dep#14350

Merged
aabadie merged 2 commits intoRIOT-OS:masterfrom
fjmolinas:pr_sketch_module
Jun 25, 2020
Merged

sys/arduino: include arduino_sketches in Makefile.dep#14350
aabadie merged 2 commits intoRIOT-OS:masterfrom
fjmolinas:pr_sketch_module

Conversation

@fjmolinas
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas commented Jun 25, 2020

Contribution description

SKETCH_MODULE is currently declared in Makefile.include which lead to its inclusion as a module to also happen there. Strictly speaking it is not needed for #9913 ince sys/Makefile.include is already included after dependency resolution, for consistency modules should not be included in Makefile.include

In this PR I though to remove the option of changing SKETCH_MODULE name, I'm not sure its useful in any way. But to make the change simpler I just moved it to Makefile.dep.

Testing procedure

  • application with no sketches compile fine

USEMODULE=arduino BOARD=nucleo-l152re make -C examples/hello-world/

  • make -C tests/sys_arduino test passes
main(): This is RIOT! (Version: 2020.07-devel-1416-ga9849-pr_sketch_module)
Hello Arduino!
wrang
UNK
echo quite long string echoing on arduino module test
ECHO: quite long string echoing on arduino module test
numb 4242
4242 4242 1092 10222
time
5145589 5182366 5219138 OK END
print
print(int, BIN): 11111111111111111111101011000111
println(int, BIN): 11111111111111111111101011000111
print(int, OCT): 37777775307
println(int, OCT): 37777775307
print(int, DEC): -1337
println(int, DEC): -1337
print(int, HEX): fffffac7
println(int, HEX): fffffac7
print(unsigned int, BIN): 101010
println(unsigned int, BIN): 101010
print(unsigned int, OCT): 52
println(unsigned int, OCT): 52
print(unsigned int, DEC): 42
println(unsigned int, DEC): 42
print(unsigned int, HEX): 2a
println(unsigned int, HEX): 2a
print(long, BIN): 10110110011010011111110100101110
println(long, BIN): 10110110011010011111110100101110
print(long, OCT): 26632376456
println(long, OCT): 26632376456
print(long, DEC): -1234567890
println(long, DEC): -1234567890
print(long, HEX): b669fd2e
println(long, HEX): b669fd2e
print(unsigned long, BIN): 1001001100101100000001011010010
println(unsigned long, BIN): 1001001100101100000001011010010
print(unsigned long, OCT): 11145401322
println(unsigned long, OCT): 11145401322
print(unsigned long, DEC): 1234567890
println(unsigned long, DEC): 1234567890
print(unsigned long, HEX): 499602d2
println(unsigned long, HEX): 499602d2

Issues/PRs references

#9913

@fjmolinas fjmolinas added Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: arduino API Area: Arduino wrapper API labels Jun 25, 2020
@fjmolinas fjmolinas mentioned this pull request Jun 25, 2020
Merged
aabadie
aabadie reviewed Jun 25, 2020
View reviewed changes
sys/arduino/Makefile.include
DIRS += $(SKETCH_MODULE_DIR)
BUILDDEPS += $(SKETCH_GENERATED_FILES)
else
PSEUDOMODULES += $(SKETCH_MODULE)
Copy link
Copy Markdown
Contributor

@aabadie aabadie Jun 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

why is this needed ?

Copy link
Copy Markdown
Contributor Author

@fjmolinas fjmolinas Jun 25, 2020

Choose a reason for hiding this comment

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

diff --git a/sys/arduino/Makefile.include b/sys/arduino/Makefile.include
index 5e80e8e2ad..a460bbac13 100644
--- a/sys/arduino/Makefile.include
+++ b/sys/arduino/Makefile.include
@@ -10,8 +10,6 @@ ifneq (,$(SKETCHES))
   # Depends on module
   DIRS      += $(SKETCH_MODULE_DIR)
   BUILDDEPS += $(SKETCH_GENERATED_FILES)
-else
-  PSEUDOMODULES += $(SKETCH_MODULE)
 endif
 
 # include the Arduino headers

USEMODULE=arduino BOARD=nucleo-l152re make -C examples/hello-world/
make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Building application "hello-world" for "nucleo-l152re" with MCU "stm32".

"make" -C /home/francisco/workspace/RIOT/boards/nucleo-l152re
"make" -C /home/francisco/workspace/RIOT/boards/common/nucleo
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/stm32
"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/stm32/bootloader
"make" -C /home/francisco/workspace/RIOT/cpu/stm32/periph
"make" -C /home/francisco/workspace/RIOT/cpu/stm32/stmclk
"make" -C /home/francisco/workspace/RIOT/cpu/stm32/vectors
"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/arduino
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/div
"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
"make" -C /home/francisco/workspace/RIOT/sys/xtimer
arm-none-eabi-gcc: error: /home/francisco/workspace/RIOT/examples/hello-world/bin/nucleo-l152re/arduino_sketches.a: No such file or direc

Copy link
Copy Markdown
Contributor Author

@fjmolinas fjmolinas Jun 25, 2020

Choose a reason for hiding this comment

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

I want to keep the behaviour as it was before, so no need to manually include the sketch module, if there are any it works, if there are none, it works.

Copy link
Copy Markdown
Contributor

@aabadie aabadie Jun 25, 2020

Choose a reason for hiding this comment

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

ok, maybe add a comment to explain that ?

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Jun 25, 2020

With this PR a static check can be added so there are no USEMODULE in Makefile.include.

aabadie
aabadie reviewed Jun 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Tested all arduino applications but tests/sys_arduino_analog on nucleo-l476rg and all work.

I'll ACK once the comment about PSEUDOMODULE is added (you can squash the change directly).

@fjmolinas fjmolinas force-pushed the pr_sketch_module branch 2 times, most recently from 2d7883a to e0964a7 Compare June 25, 2020 07:57
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Jun 25, 2020

@aabadie added sanity check and rebased to fix conflict.

aabadie
aabadie reviewed Jun 25, 2020
View reviewed changes
aabadie
aabadie approved these changes Jun 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK!

@aabadie aabadie mentioned this pull request Jun 25, 2020
Merged
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Jun 25, 2020

Dammit! Shouldn't have merged the other one hahaha.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 25, 2020

You are good for a new CI round :)

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Jun 25, 2020

You are good for a new CI round :)

Wasting C02

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 25, 2020

Wasting C02

Yeah...

@aabadie aabadie merged commit 0c1718e into RIOT-OS:master Jun 25, 2020
@miri64 miri64 added this to the Release 2020.07 milestone Jun 25, 2020
@fjmolinas fjmolinas deleted the pr_sketch_module branch June 25, 2020 09:36
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Jun 25, 2020

Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie aabadie approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri

Assignees

No one assigned

Labels

Area: arduino API Area: Arduino wrapper API 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 Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.07

Development

Successfully merging this pull request may close these issues.

3 participants

@fjmolinas @aabadie @miri64