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.

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

Add support for custom defines #571

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

qistoph
Copy link

@qistoph qistoph commented Jul 18, 2018

Could you consider adding support for custom defines, so that something like the example below would work? Or am I overlooking an available approach to make this work?

#ifdef ALTERNATIVE_V1
  void specialCall() {
    // Runs alternative code
  }
#else
  void specialCall() {
    // Runs standard code
  }
#endif

$ make CUSTOM_DEFINES="-DALTERNATIVE_V1"

@tuna-f1sh
Copy link
Contributor

tuna-f1sh commented Jul 19, 2018

You should be able to pass custom defines as part of the compiler flags since they are appended in the Makefile rather than set - I've done it this way before.

For .cpp and .c source you can use CPPFLAGS="-D..." or CXXFLAGS='"-D...". .c source only uses CFLAGS="-D..."

@qistoph
Copy link
Author

qistoph commented Jul 19, 2018

Thanks for your feedback. Somehow using CXXFLAGS does work, where CPPFLAGS doesn't. So it seems my issue is solvable.

Using CPPFLAGS

$ cat Makefile 
# Arduino Make file. Refer to https://github.com/sudar/Arduino-Makefile
BOARD_TAG   = nano
BOARD_SUB   = atmega328
ARDUINO_DIR  = /home/chris/arduino-1.8.5/
include /home/chris/Projects/Arduino-Makefile/Arduino.mk

alt1:
	$(MAKE) CPPFLAGS="-DALT1" OBJDIR=build-alt1-$(BOARD_TAG)-$(BOARD_SUB)

$ make alt1
...
make CPPFLAGS="-DALT1" OBJDIR=build-alt1-nano-atmega328
make[1]: Entering directory '/home/chris/Projects/Test'
mkdir -p build-alt1-nano-atmega328
/home/chris/arduino-1.8.5//hardware/tools/avr/bin/avr-g++ -x c++ -include Arduino.h -MMD -c -DALT1 -fpermissive -fno-exceptions -std=gnu++11 -fno-threadsafe-statics -flto -fno-devirtualize -fdiagnostics-color=always Test.ino -o build-alt1-nano-atmega328/Test.ino.o
cc1plus: fatal error: Arduino.h: No such file or directory
compilation terminated.
/home/chris/Projects/Arduino-Makefile/Arduino.mk:1314: recipe for target 'build-alt1-nano-atmega328/Test.ino.o' failed

Using CXXFLAGS

$ cat Makefile 
# Arduino Make file. Refer to https://github.com/sudar/Arduino-Makefile
BOARD_TAG   = nano
BOARD_SUB   = atmega328
ARDUINO_DIR  = /home/chris/arduino-1.8.5/
include /home/chris/Projects/Arduino-Makefile/Arduino.mk

alt1:
	$(MAKE) CXXFLAGS="-DALT1" OBJDIR=build-alt1-$(BOARD_TAG)-$(BOARD_SUB)

$ make alt1
...
make CXXFLAGS="-DALT1" OBJDIR=build-alt1-nano-atmega328
make[1]: Entering directory '/home/chris/Projects/Test'
mkdir -p build-alt1-nano-atmega328
/home/chris/arduino-1.8.5//hardware/tools/avr/bin/avr-g++ -x c++ -include Arduino.h -MMD -c -D__PROG_TYPES_COMPAT__ -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=185 -DARDUINO_ARCH_AVR -I/home/chris/arduino-1.8.5//hardware/arduino/avr/cores/arduino -I/home/chris/arduino-1.8.5//hardware/arduino/avr/variants/eightanaloginputs    -Wall -ffunction-sections -fdata-sections -Os -DALT1 Test.ino -o build-alt1-nano-atmega328/Test.ino.o

@tuna-f1sh
Copy link
Contributor

Yes I'm not sure why but setting CPPFLAGS external to the Arduino.mk seems to drop all the include flags, despite it being appended here: https://github.com/sudar/Arduino-Makefile/blob/master/Arduino.mk#L1062

Not got time to look into it now but maybe you can spot something. I thought it might be this line doing the expand prior to the includes but commenting it out still presents the same problem.

@qistoph
Copy link
Author

qistoph commented Jul 19, 2018

Just found out that using CXXFLAGS does influence the build. The following arguments are missing when using CXXFLAGS from my Makefile:

-fpermissive -fno-exceptions -std=gnu++11 -f| braries/Servo/src -I/home/chris/arduino-1.8.5//hardware/arduino/avr/libraries/EEPROM/src -Wall -ffun no-threadsafe-statics -flto -fno-devirtualize -fdiagnostics-color=always

I think the variables that are passed as arguments are not overwritten or appended from the Arduino.mk. I think this thread is explaining a possible solution. I'm going to give it a try...

@qistoph
Copy link
Author

qistoph commented Jul 19, 2018

Jup, that seems to make it work. Updated the PR to reflect this :)

Thanks for your help in figuring it out and make the solution much cleaner.

@qistoph
Copy link
Author

qistoph commented Jul 26, 2018

Just checking up if you are going to merge this into master?
I am referring Arduino-Makefile in one of my projects that I'm going to share with some friends. If they could just use the default Arduino-Makefile, that would be much easier for them.

@tuna-f1sh
Copy link
Contributor

I can't merge as I'm a contributor not collaborator. If you want more chance of getting it merge though, it would be worth updating the arduino-vars.md and changelog as described in CONTRIBUTING.MD.

There might be something to be said for sticking to the original sole addition of a variable, in case people are using the passing of CPPFLAGS in the command args to override the default intentionally.

@sudar
Copy link
Owner

sudar commented Sep 30, 2018

@qistoph

Thanks for the PR. Please do add some documentation in the arduino-vars.md file.

@wingunder
Copy link
Contributor

wingunder commented Oct 1, 2018

I patched the current master's HEAD, fe84c59, examples/Blink/Blink.ino with the following patch:
https://gist.github.com/6d223de494e7f5a57a3392b21e819986

I then ran the following in the examples/Blink directory:

$ CPPFLAGS="-DALTERNATIVE_V1 -DUSING_CPPFLAGS" make clean all 2>&1 |grep Blink.ino
/usr/share/arduino/hardware/tools/avr/bin/avr-g++ -x c++ -include Arduino.h -MMD -c -D__PROG_TYPES_COMPAT__ -DALTERNATIVE_V1 -DUSING_CPPFLAGS -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=105  -I/usr/share/arduino/hardware/arduino//cores/arduino -I/usr/share/arduino/hardware/arduino//variants/standard    -Wall -ffunction-sections -fdata-sections -Os -fpermissive -fno-exceptions -std=gnu++11 -fno-threadsafe-statics -flto -fno-devirtualize -fdiagnostics-color=always Blink.ino -o build-uno/Blink.ino.o
Blink.ino:9:2: warning: #warning "Yes, ALTERNATIVE_V1 was assigned!" [-Wcpp]
/usr/share/arduino/hardware/tools/avr/bin/avr-gcc -mmcu=atmega328p -Wl,--gc-sections -Os -flto -fuse-linker-plugin -o build-uno/Blink.elf build-uno/Blink.ino.o build-uno/libcore.a   -lc -lm 

So, using CPPFLAGS works for me.

$ CXXFLAGS="-DALTERNATIVE_V1 -DUSING_CXXFLAGS" make clean all 2>&1 |grep Blink.ino
/usr/share/arduino/hardware/tools/avr/bin/avr-g++ -x c++ -include Arduino.h -MMD -c -D__PROG_TYPES_COMPAT__ -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=105  -I/usr/share/arduino/hardware/arduino//cores/arduino -I/usr/share/arduino/hardware/arduino//variants/standard    -Wall -ffunction-sections -fdata-sections -Os -DALTERNATIVE_V1 -DUSING_CXXFLAGS -fpermissive -fno-exceptions -std=gnu++11 -fno-threadsafe-statics -flto -fno-devirtualize -fdiagnostics-color=always Blink.ino -o build-uno/Blink.ino.o
Blink.ino:9:2: warning: #warning "Yes, ALTERNATIVE_V1 was assigned!" [-Wcpp]
/usr/share/arduino/hardware/tools/avr/bin/avr-gcc -mmcu=atmega328p -Wl,--gc-sections -Os -flto -fuse-linker-plugin -o build-uno/Blink.elf build-uno/Blink.ino.o   build-uno/libcore.a -lc -lm 

And using CXXFLAGS also work for me.

$ DUFF_FLAGS="-DALTERNATIVE_V1 -DUSING_DUFF_FLAGS" make clean all 2>&1 |grep Blink.ino
/usr/share/arduino/hardware/tools/avr/bin/avr-g++ -x c++ -include Arduino.h -MMD -c -D__PROG_TYPES_COMPAT__ -mmcu=atmega328p -DF_CPU=16000000L -DARDUINO=105  -I/usr/share/arduino/hardware/arduino//cores/arduino -I/usr/share/arduino/hardware/arduino//variants/standard    -Wall -ffunction-sections -fdata-sections -Os -fpermissive -fno-exceptions -std=gnu++11 -fno-threadsafe-statics -flto -fno-devirtualize -fdiagnostics-color=always Blink.ino -o build-uno/Blink.ino.o
Blink.ino:11:2: error: #error "No, ALTERNATIVE_V1 was NOT assigned!"
make: *** [../../Arduino.mk:1311: build-uno/Blink.ino.o] Error 1

And finally, as expected, not setting either CPPFLAGS or CXXFLAGS doesn't work.

Maybe I'm missing something here. If not, I think CPPFLAGS or CXXFLAGS could be used as I demonstrated above and there is really no need for CUSTOM_DEFINES.

I think that the override directive was not used correctly in this patch, as:
From https://www.gnu.org/software/make/manual/html_node/Override-Directive.html:

If you want to set the variable in the makefile even though it was set with a command argument, you can use an override directive,

The way I understand it, we're always extending (prepending to, using :=, or appending to, using +=) CXXFLAGS/CPPFLAGS. We never override them (using =). Or do we want to override them? If so, overriding should from my point of view be in a separate patch, as it has nothing to do with the pure appending of CUSTOM_DEFINES to CPPFLAGS.

If we want it and if I'm not completely missing the point, adding CUSTOM_DEFINES should be as simple as adding the following to Arduino.mk, before the first usage of $(CPPFLAGS):

ifdef CUSTOM_DEFINES
    CPPFLAGS += $(CUSTOM_DEFINES)
endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants