Unify AVR and SAM cores/arduino

[cousteaulecommandant]

Right now hardware/arduino/avr/cores/arduino and hardware/arduino/sam/cores/arduino contain redundant files, for example Print.cpp, WString.cpp, etc. This makes maintenance complicated, since changes on one won't affect the other (even when those changes are platform-independent), so the developer will have to make the changes on multiple files (and something tells me SAM won't have the same attention as AVR). I think it would be a good idea to unify them so that all platform-independent sources are common, and then each derivative has a small set of platform-specific sources.

[dlabun]

If you look at Print.cpp on both the AVR and SAM cores you'll see they're different... That's not to say there are not identical files shared between the cores but many of the files are tweaked for their specific core.

[cousteaulecommandant]

True, but the differences are minimal. In fact I only found one (that's 1 more than I initially assumed). Still, I think those small differences should be separated as much as possible so that as many files as possible are identical and can be merged. (Maybe this suggestion is not as immediate as I would have imagined)

Also, this separation between high-level functionality and low-level details seems logical from an organization point of view.

[q2dg]

something tells me SAM won't have the same attention as AVR

Well, until recent .cc/.org merge, SAM architecture (present only in Due board) was deprecated...

[eric-wieser]

If you look at Print.cpp on both the AVR and SAM cores you'll see they're different

Then put the one different function in PrintFlashString.cpp. C++ does not require all of a class to be in a single file.

[mbanzi]

This is something we're investigating, how to move hw independent files in a different location than the hw dependent files. We'll bring the discussion to the dev list

@q2dg it was never deprecated as a Core, the Arduino Due as a product was marked as retired because we were not going to maintain it further.

[bperrybap]

Now that the IDE is smarter about the way it handles libraries and can automatically locate sub libraries, it should be possible to make Print its own library and move it to the common bundled libraries area.
The Print class should be core independent.
My guess is that the only reason that certain header files like Print were duplicated in each core was because until recently the IDE was not smart enough to locate sub libraries and the only way to ensure that files like Print.h, Printable.h, Stream.h and a few others were always located was to put them in the core libraries directory. This was because the core libraries directory was always on the include path.
Now that the IDE is smarter, these files could be moved into the bundled libraries area as separate libraries.

[eric-wieser]

@bperrybap: What about Print::print(const __FlashStringHelper *ifsh), which may not even exist on all cores?

[PaulStoffregen]

The Print class should be core independent.

My Print class implementations for Teensy have hardware-specific optimizations, some even in assembly.

Just because many boards use Arduino's code as-is, please don't assume everyone does.

[bperrybap]

Which cores don't have F() and the __FlashStringHelper class?
They exist in these cores: IDE bundled AVR, Arduino SAM, teensy AVR, teensy3 ARM, ATTinyCore, ESP8266, STM, chipKit,

But, yeah all the crap to support the proprietary AVR progmem stuff really sucks and has made a wreck of things.
In this case, I don't think including a common Print class as a bundled library wouldl be a problem.
Cores that choose to not provide all the common services (which I would consider the F() macro to be part of standard Arduino services at this point in time), or wish to provide their own can always include their own Print class, which they are already doing today in their core. The Print.h in the core would trump the one in a common libraries area so nothing would break even for those cores that want to provide their own Print class.

However in the case of using _FlashStringHelper class in the common bundled Print class, the common bundled Print class header and code could be made smarter to allow for cores that don't provide the F() service/capability.

IMO, any core that provides an F() macro but does not provide for a __FlashStringHelper class is broken and will have to ship its own Print class vs use the one bundled with the IDE.

To account for F() and __FlashStringHelper class not existing, all that is necessary is to check for the existence of the F macro.
i.e.

#if defined(F)
// put all the code that depends on or uses __FlashStringHelper class in here
#endif

This should work since __FlashStingHelper class should exist whenever F() exists and F should be defined in a core specific file. (currently WString.h)

[PaulStoffregen]

I believe you are correct on this point about the header file:

The Print.h in the core would trump the one in a common libraries area so nothing would break even for those cores that want to provide their own Print class.

Indeed the group of -I command line used the compile the sketch code has the core library first.

However, the linker command which assembles the final .elf file has core.a last. All the .o files from libraries appear earlier on the linker command. That detail worries me a bit....

You're probably right, but I really hope whatever changes are considered can be throughly tested for "nothing would break even for those cores that want to provide their own Print class".

[dlabun]

@bperrybap Did you check the SAMD21 core?

[bperrybap]

When I said "including a common Print class as a bundled library" I was meaning a separate new bundled library called Print that has a directory under the common libraries directory where Print.h and Print.cpp would live. It would be sitting in the same place with the other bundled libraries like LiquidCrystal. Not a new bundled library that includes other things besides the Print class, like perhaps Printable, or Stream (which, BTW, have similar needs/issues). I'm talking about a very simple change that uses and preserves all the existing IDE build mechanisms.
I believe that there are better ways to handle this kind of stuff - but they are more intrusive, and would probably be more likely to break things.
This was simply a proposal to create a common Print class library that cores could choose to start using, without having to modify the existing build methodology in the IDE and without breaking any existing cores.

But Paul, you bring up a good point about the order of the linked objects and how it has the potential to break things depending on how these common service libraries are implemented.
However, the object ordering issue starts to get very tricky and ugly since the IDE is currently stomping on the users sketchbook libraries area to hold its bundled library updates.
IMO, the IDE should never do this, but it was done this way to avoid doing IDE in place updates.
I believe IDEs should update bundled libraries in place since updates by a given IDE are generally specific to that IDE and should not affect or even break other IDEs that happen to be installed on the same machine. Doing bundled updates in the users sketchbook area is fraught with issues, but I lost that discussion back on the mailing list.
All that said, IDE bundled library updates is for the most part off topic with respect to this topic but it might have some relevance depending on the implementation of these common services; like if multiple of these services (i.e. Print, Printable, Stream) are bundled together into a new library the way the core objects are bundled.

[eric-wieser]

@PaulStoffregen

My Print class implementations for Teensy have hardware-specific optimizations, some even in assembly.

Can you link to these? For the little the print class does, I'm surprised that there's a hardware-specific optimization in there.