Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Compiler flags aren't properly expanded #48

Open
foriequal0 opened this issue Oct 6, 2018 · 8 comments
Open

Compiler flags aren't properly expanded #48

foriequal0 opened this issue Oct 6, 2018 · 8 comments
Assignees
Labels
bug Potential bug in code priority: normal Normal priority - Probably will be implemented some time soon

Comments

@foriequal0
Copy link

foriequal0 commented Oct 6, 2018

OS: Linux
Distribution (Linux Only): Ubuntu
OS Version: 18.04
Platform: Arduino Micro
Platform SDK Version: 1.8.7

I've modified /examples/hello-world/CMakeFiles.txt as below for my Arduino Micro, and then I tried to build this, but it failed with the following build errors.

cmake_minimum_required(VERSION 3.8.2)

project(Hello_World)
get_board_id(board_id micro atmega32u4)

add_arduino_executable(Hello_World ${board_id} helloWorld.cpp)

Error:

[  3%] Building CXX object CMakeFiles/micro_core_lib.dir/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp.obj
/home/foriequal0/bin/arduino-1.8.7/hardware/tools/avr/bin/avr-g++   -I/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino -I/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/variants/micro  -c -g -Os -w -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics -Wno-error=narrowing -MMD -flto -mmcu=atmega32u4 -DF_CPU=16000000L -DARDUINO=10807 "-DARDUINO_AVR_MICRO " -o CMakeFiles/micro_core_lib.dir/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp.obj -c /home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp
In file included from /home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBAPI.h:44:0,
                 from /home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp:20:
/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp:73:29: error: 'USB_VID' was not declared in this scope
  D_DEVICE(0xEF,0x02,0x01,64,USB_VID,USB_PID,0x100,IMANUFACTURER,IPRODUCT,ISERIAL,1);
                             ^
/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.h:267:61: note: in definition of macro 'D_DEVICE'
  { 18, 1, USB_VERSION, _class,_subClass,_proto,_packetSize0,_vid,_pid,_version,_im,_ip,_is,_configs }
                                                             ^
/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp:73:37: error: 'USB_PID' was not declared in this scope
  D_DEVICE(0xEF,0x02,0x01,64,USB_VID,USB_PID,0x100,IMANUFACTURER,IPRODUCT,ISERIAL,1);
                                     ^
/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.h:267:66: note: in definition of macro 'D_DEVICE'
  { 18, 1, USB_VERSION, _class,_subClass,_proto,_packetSize0,_vid,_pid,_version,_im,_ip,_is,_configs }
                                                                  ^
CMakeFiles/micro_core_lib.dir/build.make:497: recipe for target 'CMakeFiles/micro_core_lib.dir/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp.obj' failed

Recipe should be expanded with the board_id

I've tracked down this error.

avr-g++   -I/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino -I/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/variants/micro  -c -g -Os -w -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics -Wno-error=narrowing -MMD -flto -mmcu=atmega32u4 -DF_CPU=16000000L -DARDUINO=10807 "-DARDUINO_AVR_MICRO " -o CMakeFiles/micro_core_lib.dir/home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp.obj -c /home/foriequal0/bin/arduino-1.8.7/hardware/arduino/avr/cores/arduino/USBCore.cpp

This compiler invocation command is an expansion of following recipe.

# arduino-1.8.7/hardware/arduino/avr/platforms.txt
recipe.cpp.o.pattern="{compiler.path}{compiler.cpp.cmd}" {compiler.cpp.flags} (...) {compiler.cpp.extra_flags} {build.extra_flags} {includes} "{source_file}" -o "{object_file}"                                                                               

And placeholder {build.extra_flags} should be overriden by following definition.
refer: https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5-3rd-party-Hardware-specification#boardstxt

# arduino-1.8.7/hardware/arduino/avr/board.txt
micro.build.extra_flags={build.usb_flags}

And finally {build.usb_flags} should be expanded by following definition.

# arduino-1.8.7/hardware/arduino/avr/platforms.txt
build.usb_flags=-DUSB_VID={build.vid} -DUSB_PID={build.pid} '-DUSB_MANUFACTURER={build.usb_manufacturer}' '-DUSB_PRODUCT={build.usb_product}'

But recipe.cpp.o.pattern is expanded without the board id which used to override {build.extra_flags}
Refer: https://github.com/arduino-cmake/Arduino-CMake-NG/blob/v0.6/cmake/Platform/Properties/PropertiesReader.cmake#L14

This leads to the expansion of recipe.cpp.o.pattern doesn't contains {build.extra_flags} or {micro.build.extra_flags}

I've commented that line to fix this issue, expecting later expansion should handle them with the board id, then retried a build, but I encountered the second issue.

The expansion is not recursive

Let's look at the recipie again.

# arduino-1.8.7/hardware/arduino/avr/platforms.txt
recipe.cpp.o.pattern="{compiler.path}{compiler.cpp.cmd}" {compiler.cpp.flags}  ...

It contains {compiler.cpp.flags}, and it is defined as follows.

# arduino-1.8.7/hardware/arduino/avr/platforms.txt
compiler.cpp.flags=-c -g -Os {compiler.warning_flags} -std=gnu++11  (...)

compiler.warning_flags is defined as follows.

# arduino-1.8.7/hardware/arduino/avr/platforms.txt
compiler.warning_flags=-w

But the expansion of recipe.cpp.o.pattern stops at the first level, this leads to an incorrect command.

avr-g++ (...) -c -g -Os {compiler.warning_flags} -std=gnu++11 -fpermissive (...)

(it still conatins {compiler.warning_flags}

I think _resolve_recipe_property function doesn't expand recursively.
refer: https://github.com/arduino-cmake/Arduino-CMake-NG/blob/v0.6/cmake/Platform/Properties/RecipePropertyValueResolver.cmake#L16

@foriequal0
Copy link
Author

https://github.com/foriequal0/Arduino-CMake-NG/tree/feature/fix-recursive-resolve
I tried to fix this. It just works for the hello-world example, but I think I'm breaking some other things.

@MrPointer
Copy link
Member

MrPointer commented Oct 8, 2018

Whoa! That's a lot of info!
@foriequal0 Thanks so much for providing so many diagnostics, definitely will ease solving the bug 😄

As to the bug itself - Yes, it looks like something I'm partly aware of, as I've never fully completed the work there, thus recursive expansion isn't supported.
One of the main reasons for this is my lack of knowledge of Arduino development (ironic right?) - I only manage this project due to my CMake knowledge.

Anyways I'll have a look at it and your proposed solution as well.

@MrPointer MrPointer added the bug Potential bug in code label Oct 8, 2018
@MrPointer MrPointer self-assigned this Oct 12, 2018
@MrPointer
Copy link
Member

@foriequal0 Hi, just a little update:
I'm currently on a very busy schedule and don't have much time to work on the project, however, I can say that I'm in the middle of solving this bug and getting close to it.
It's a tough one, and it lays its roots in the core of the framework, and there's no "quick" way of temporarily fixing it.
Because of that, I'll integrate this fix officially into the next, v0.7 release (which refactors the entire framework anyways), and you could "enjoy" the fix once it gets PRed into master. This will be the first feature to PR in this release, so it won't drift much from the v0.6 release.

@foriequal0
Copy link
Author

Thank you! I'm looking forward to it!

@pjwebster
Copy link

Just encountered this and have a temporary workaround for compiling at least. You can add the appropriate USB_VID and USB_PID defines in the CMakeLists.txt:

cmake_minimum_required(VERSION 3.8)

set(CMAKE_TOOLCHAIN_FILE cmake/Arduino-Toolchain.cmake)

add_definitions("-DUSB_VID=0x2a03")
add_definitions("-DUSB_PID=0x8036")

project(myproject)

set(ARDUINO_TARGET_BOARD leonardo)
set(ARDUINO_SERIAL_PORT /dev/ttyACM0)

get_board_id(board_id ${ARDUINO_TARGET_BOARD})
add_arduino_example(${PROJECT_NAME} ${board_id} Blink)
upload_arduino_target(${PROJECT_NAME} ${board_id} ${ARDUINO_SERIAL_PORT})

I haven't got any further yet though, because the upload isn't working for my Leonardo based board. The avrdude and avrdude.conf in the Arduino 1.8.7 SDK gives the error " programmer type jtagice3_updi not found". Still digging into that one

@MrPointer
Copy link
Member

MrPointer commented Nov 1, 2018

@pjwebster Thanks for the suggestion, although not pretty it's a perfectly valid fix!
About the upload issues you're having - no need to dig further since programmer support isn't available yet. Only the basic uploaders are supported, and I guess the jtagice3_updi isn't one of them ☹️

@pjwebster
Copy link

Ironically, I don't want to use that programmer, just the normal one.

After no success, I tried using add_custom_target to add an upload step using avrdude directly and bricked my board. Had to dig up my usbasp and revert to using the Arduino IDE to reload the bootloader via the icsp 😢

@MrPointer
Copy link
Member

I'm sorry to hear that... Currently I have no solution for this, and unfortunately no time to investigate & possibly solve it either.

@MrPointer MrPointer added the priority: normal Normal priority - Probably will be implemented some time soon label Dec 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Potential bug in code priority: normal Normal priority - Probably will be implemented some time soon
3 participants