Fix missing imports in font files for PlatformIO

[MarcelRobitaille]

Fixes #416

This adds support for PlatformIO (and other pure C++ build tools) and shouldn't change compatibility with Arduino.

Arduino is very similar to C++, but it does some magic stuff that makes it "easier" for beginners like function hoisting. If you try to use this library from PlatformIO, there will be a compilation error for every font file.

.../Adafruit GFX Library/Fonts/FreeMonoBold9pt7b.h:1:7: error: 'uint8_t' does not name a type
    1 | const uint8_t FreeMonoBold9pt7bBitmaps[] PROGMEM = {
      |       ^~~~~~~
.../Adafruit GFX Library/Fonts/FreeMonoBold9pt7b.h:87:7: error: 'GFXglyph' does not name a type
   87 | const GFXglyph FreeMonoBold9pt7bGlyphs[] PROGMEM = {
      |       ^~~~~~~~
.../Adafruit GFX Library/Fonts/FreeMonoBold9pt7b.h:184:7: error: 'GFXfont' does not name a type
  184 | const GFXfont FreeMonoBold9pt7b PROGMEM = {(uint8_t *)FreeMonoBold9pt7bBitmaps,
      |       ^~~~~~~

The solution is to #include <Adafruit_GFX.h> in each of these font files. This will bring uint8_t, GFXglyph, and GFXfont into scope.

I have tested this and I'm now successfully able to compile in PlatformIO. Furthermore, I went into ~/Arduino/libraries/Adafruit_FGX_Library and made this change, and I was still able to compile the code in Arduino IDE, so nothing should be broken from Arduino.

[BillyDonahue]

I have some feedback from the sidelines about this PR.

• Font files are more inlined source than traditional declaration-only headers. As you noted, the only reason they are .h files is to support the Arduino single .ino link model. If you include them more than once, you're still going to have a system that doesn't work properly, because each translation unit that includes the font header will have a private definition of the font and glyph array objects. They are not declared in such a way that they can be shared between translation units. It may compile, but it isn't logically correct and you can end up with duplicated arrays or ODR violations. For a more standard linker system like platformIO, I think you need something like a handrolled all_my_gfx_fonts.cpp file that includes all the fonts for the project, with the correct prerequisite headers (e.g. #include <Adafruit_GFX.h>) preceding their inclusion, and these gfx fonts must not be included elsewhere in the project. You could export them all with an all_my_gfx_fonts.h file that consists of extern declarations for the fonts in the .cpp file:

  // all_my_gfx_fonts.h:
  #include <Adafruit_GFX.h>
  extern const GFXfont TomThumb;
  extern const GFXfont FreeMonoBold9pt7b;
  // etc

  Or provide a nice wrapper function that returns a const GFXfont* or something.

• These are generated files. As such, the fontconvert generator would have to be modified to regenerate these changes. or they'll be lost on first regen.

• #pragma once is nonstandard and we deal with a lot of compilers. It has technical problems having to do with symlinks etc. Probably better to use a traditional #ifdef include guard (possibly with a GUID).

• The Adafruit GFX font header is something of a file format that is phrased as C, similar to an XBM image file. Adding preprocessor directives to it could break tooling outside the repo. I know there are web visualizers etc for them.

[MarcelRobitaille]

Hey @BillyDonahue. Thanks for your feedback. I really appreciate it.

From points 1 and 3, it seems like there should be no #pragma once. It is the job of some all_my_gfx_fonts.h to do some kind of guard (#pragma once or #ifndef). In 3, you say I should do #ifndef instead of #pragma once, but in 1 and 4 it sounds like I should not be changing these files at all.

In 2, you say I should have modified fontconvert instead. But in 4, you say that I should not change these files at all because external tooling depends on these headers.

Based on 4, this PR should be closed as wontfix, right?

[BillyDonahue]

Hey @BillyDonahue. Thanks for your feedback. I really appreciate it.

From points 1 and 3, it seems like there should be no #pragma once. It is the job of some all_my_gfx_fonts.h to do some kind of guard (#pragma once or #ifndef). In 3, you say I should do #ifndef instead of #pragma once, but in 1 and 4 it sounds like I should not be changing these files at all.

In 2, you say I should have modified fontconvert instead. But in 4, you say that I should not change these files at all because external tooling depends on these headers.

Based on 4, this PR should be closed as wontfix, right?

First, I just want to emphasize that I'm just an engineer on the internet and I'm really just lobbing feedback in. I've worked on this library and used it a fair amount, but I'm not a maintainer or reviewer. But I would suggest closing it as wontfix if asked for my opinion as a spectator, yes.

After seeing the ecosystem of Adafruit-GFX tooling out there, I think the bar for changing these files is higher than one might initially assume and I wanted to draw your attention to that and provide a PlatformIO workaround for you.

The Arduino sketch single-source model is beginner-friendly but can be in conflict with good C++ modular design and this is such a case. So you have to impose some discipline externally with wrappers to reuse Arduino-targeted sources in traditional C++ projects and that's just how it is, IMO.

[ladyada]

hiya! thanks so much for submitting a PR! we can review & merge PRs once they have passed continuous integration (CI). that means that code compiles cleanly for all tested platforms, is clang formatted so we maintain the same text formatting for all new code, and is doxygen documented. if your code isnt passing, check the CI output (click on the red X next to the PR to scroll through the log and find where the error is

here is a tutorial on doxygen: https://learn.adafruit.com/the-well-automated-arduino-library/doxygen

and clang-format: https://learn.adafruit.com/the-well-automated-arduino-library/formatting-with-clang-format

and overall how to contribute PRs to libraries: https://learn.adafruit.com/contribute-to-arduino-with-git-and-github

once you get that green checkmark that indicates CI has passed, please comment reply to this post so we know its time for another review (we may not get notified on CI pass and miss that its time to look!)

[MarcelRobitaille]

@ladyada all green now. Sorry about that.

[ladyada]

this is a side effect of how platformio 'optimized' compilation. it does not follow the standard C/C++ language - but its also not harmful and still passes arduino CI so its ok

[MarcelRobitaille]

@BillyDonahue

* The Adafruit GFX font header is something of a file format that is phrased as C, similar to an XBM image file. Adding preprocessor directives to it could break tooling outside the repo. I know there are web visualizers etc for them.

You were right. This change broke https://github.com/tchapi/Adafruit-GFX-Font-Customiser. I opened a PR there. Is this the one you meant or are there others?

[BillyDonahue]

@BillyDonahue

* The Adafruit GFX font header is something of a file format that is phrased as C, similar to an XBM image file. Adding preprocessor directives to it could break tooling outside the repo. I know there are web visualizers etc for them.

You were right. This change broke https://github.com/tchapi/Adafruit-GFX-Font-Customiser. I opened a PR there. Is this the one you meant or are there others?

Yeah I fixed a bug in that one. That was the one I was thinking of, but I have an impression of having seen similar tools out there at the time.

https://github.com/tchapi/Adafruit-GFX-Font-Customiser/pull/35 needs to go further. I'll comment on that thread though.