Allow variants to define an initVariant() function that is called at startup

[cmaglie]

Following #2080, this is a way to allow "variants" to define an initVariant() function (in a file called pins_arduino.cpp for example).

/cc @matthijskooijman@vicatcu

[matthijskooijman]

Ok, this looks like it would work. I'd say the commit message is a bit brief, especially since it doesn't mention the changes to the compile process. I had to look realy closely to find out what was happening exactly - it seems there are some whitespace changes that blur the diff. Perhaps you could put the whitespace changes in a separate commit?

In any case, I think your commit:

• Makes sure that source files in the variant folder are included in the build
• Defines a weak initVariant function
• Unconditionally calls initVariant

Because the variant source files are included in the build directly (not through core.a), they are unconditionally included, even when there are no undefined references to them. This solves the problem I described in #2080 in a rather elegant way.

However, I think that in my previous testing, source files in the variant directory were already included in the build somehow. Looking at #2080, I didn't change the compilation process, but I'm sure I tested this using a pins_arduino.cpp in the variant folder. This leads me to believe that now the variant source files are included in the build twice (though I haven't tried to locate the other spot in the code yet).

[matthijskooijman]

Ah, seems your pullreq is against master, while mine was against 1.5.x. That probably explains the difference?

[cmaglie]

Hi @matthijskooijman, thanks for reviewing it and sorry about the white-spaces mess, I was a bit in a hurry when I made it.

The patch is against master, IDE 1.5 already compiles files in variant folder, basically this is a backport to 1.0 of this part:

https://github.com/arduino/Arduino/blob/ide-1.5.x/app/src/processing/app/debug/Compiler.java#L710

So, yes, all your assumptions are correct.

[matthijskooijman]

Ok, then the changes you present here look good to me (except for the whitespace mess ;-p)

For 1.5, however, the variant files are included in the build through core.a, which breaks the weak approach you're using here. I've just tested a patch to fix that, I'll push that to #2080 in a minute.

However, I did come across something else - since main.cpp is a C++ file, the initVariant() function is subjected to name mangling, which means you can really only define initVariant from a .cpp file in the variant, not a .c file. I'm not sure we want this, I'd have expected that a C function would also work and others will probably too.

If we declare initVariant() as extern "C", then both .cpp and .c files can supply an implementation. Furthermore, if we put this declaration in Arduino.h, then the author of a variant doesn't even have to take care of this (other than to include Arduino.h, which they'll likely do anyway).

Am I making any sense? I'll push the implementation of my proposal to #2080 in a minute.

[vicatcu]

@cmaglie@matthijskooijman I can confirm this pull request works as expected for my board. I vote for a merge 👍. I tested it running Arduino from within Eclipse and using my board variant to blink the LED from within the initVariant function. I also confirmed that the presence of variant.h and variant.cpp in my variant folder is harmlessly ignored to my existing Arduino 1.0.5 installation. So when does this become part of what you can download from arduino.cc as a normal user?

[cmaglie]

Thanks for testing this out, I'll merge that before IDE release 1.5.7 and 1.0.6.

[matthijskooijman]

@cmagilie, Please also add the declaration of initVariant() to Arduino.h (inside the extern "C" block, like I did in #2080, to allow defining initVariantin a .c file, before/while merging!

[cmaglie]

Here we go!

[matthijskooijman]

Looks good to me.