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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Jun 25, 2020
Merged

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

merged 2 commits into from
Jun 25, 2020

Conversation

fjmolinas
Copy link
Contributor

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
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
Contributor Author

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
Contributor Author

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
Contributor

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
Contributor Author

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
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
Contributor Author

@aabadie added sanity check and rebased to fix conflict.

aabadie
aabadie approved these changes Jun 25, 2020
View reviewed changes
Copy link
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
Contributor Author

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

@aabadie
Copy link
Contributor

aabadie commented Jun 25, 2020

You are good for a new CI round :)

@fjmolinas
Copy link
Contributor Author

You are good for a new CI round :)

Wasting C02

@aabadie
Copy link
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
Contributor Author

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