RudolphRiedel / FT800-FT813

Multi-Platform C code Library for EVE graphics controllers from FTDI / Bridgetek (FT810, FT811, FT812, FT813, BT815, BT816, BT817, BT818)
MIT License
128 stars 57 forks source link

Definition file for platform IO inclusion #29

Closed obones closed 2 years ago

obones commented 3 years ago

This allows adding this repository URL in the lib_deps section of the platformio.ini file while ensuring the example projects are not compiled. This is required because PlatformIO compiles all C/C++ file it finds, regardless of whether they are applicable to the currently targeted board or not.

RudolphRiedel commented 3 years ago

Interesting. I am not sure though if this actually can work though. One issue would be that you need to rename EVE_commands.c to EVE_commands.cpp and EVE_target.c to EVE_target.cpp for Arduino since the methods of the SPI class can not be called from plain c. I have not found a way around this but I usually do not have this issues anyways since I am programming in plain C.

And what about the necessary changes to EVE_config.h and EVE_target.h if one is using a different display than I selected and/or has the SPI connected to a different set of pins?

If this fetches a copy from Github automatically this might become a major pain when I update the repository.

obones commented 3 years ago

Thank your for your feebdack

One issue would be that you need to rename EVE_commands.c to EVE_commands.cpp and EVE_target.c to EVE_target.cpp for Arduino since the methods of the SPI class can not be called from plain c.

Well, I naively thought that I would simply need to wrap the #include lines for EVE in a extern "C" construct, but you are right, this does not help because the SPI files are using class which is not understood by the C compiler anyway. I looked for a way to force PlatformIO to use the C++ compiler on a C file, but this does not seem to be possible easily. So I changed the content of the library.json file to limit it to the arduino platform and use the cpp files from the associated example. This has been pushed to the branch for this pull request. With this, I'm able to use the library GitHub URL in my lib_deps definition and have it compile and link.

And what about the necessary changes to EVE_config.h and EVE_target.h if one is using a different display than I selected and/or has the SPI connected to a different set of pins?

I did not notice those at first, but after reviewing them, here is what I would do:

  1. Inside EVE_config.h, I would remove line 151/152 to force users to give the definition on the command line (-D option). And to make sure this is done, I would add a test at the end of the file that outputs an error should EVE_HSIZE be missing, like this:
#ifndef EVE_HSIZE
#error Please define the display model using the appropriate preprocessor symbol(s)
#endif

This might need to be adjusted for different compilers, though. Or maybe this could be define a default set of options with a nice warning message.

  1. Inside EVE_target.h I would surround the pin definitions with ifndef blocks so that I can pass the pin definitions on the command line, while using default values if they are fine with my project. Here is an example
#ifndef EVE_CS
#define EVE_CS      13
#endif

This might as well be all placed on the same line, it all depends on coding style taste.

What do you think of this?

obones commented 3 years ago

I looked for a way to force PlatformIO to use the C++ compiler on a C file, but this does not seem to be possible easily Actually, there is an easy trick for this, which is to create a cpp file that includes the c file inside itself. So for instance, create ESP_target.cpp which only content is this:

#include "EVE_target.c"

Then, configuring the library.json file back to only consider cpp files inside the root folder makes it compile and link properly. This "trick" means that you would not need to do any changes to the c files, they are automatically included with these cpp files.

Do you want me to add those two files in this pull request as well? Maybe with a bit of comments at their top to clarify the reason for their existence?

RudolphRiedel commented 3 years ago

Sorry, I have not forgotten this but I am rather busy working currently and somehow last weekend and this weekend just came and went...

obones commented 3 years ago

No worries, I know how it is to see time fly by so fast that everything is blurry. Note that to make my life easier, I have created this branch: https://github.com/obones/FT800-FT813/tree/EVEOpenHAB It contains all the changes mentioned above, as required for inclusion via platformio.ini in my own project and it works quite well. Let me know if I should close this PR and do one on that other branch.

RudolphRiedel commented 3 years ago

Ok, I had a couple of ideas and started to implement now. The issue is of course not with this pull-request but this opened an old wound regarding the need to rename files to .cpp for the Arduino platform.

I can compile for Arduino Uno now with EVE_commands.c. First of all I split up EVE_target into two files, EVE_target.c for the non-Arduino targets and EVE_target.cpp for the Arduino targets. And to make EVE_commands.c compile with the SPI class I added a new file: EVE_cpp_wrapper.cpp.

It starts like this:

if defined (ARDUINO)

include

include

include

ifdef __cplusplus

extern "C" {

endif

#if defined (__AVR__)

uint8_t eve_spi_receive(uint8_t data)
{
    return SPI.transfer(data);
}

So there is a price for this, it costs an extra function call, I have to see how this checks out in the end.

And regarding this pull-request directly, when I am going with this I need to add a whole lot more to the file then there currently is, it's not like there is only ESP32. :-)

RudolphRiedel commented 2 years ago

I am not sure if I am prepared enough for this but I try and go with this now. :-)

RudolphRiedel commented 2 years ago

This rabbit hole is deeper than I imagined. :-) I tried to register this library with PlatformIO but it fails. So I changed at least the Arduino example to reference the library from Github instead of duplicating the source code. In the process I changed EVE_config.h so that a define in the build-enviroment selects the display. But after I just edited the Readme.md to reflect this I noticed that EVE_target.h can no longer be edited to configure the chip-select and power-down pins if this file is not part of the project but referenced externally.

At least the Aduino example builds just fine for every platform right now: Environment Status Duration


uno SUCCESS 00:00:01.170 avr_pro SUCCESS 00:00:01.154 nano328 SUCCESS 00:00:01.149 mega2560 SUCCESS 00:00:01.170 adafruit_metro_m4 SUCCESS 00:00:03.550 samd21_m0-mini SUCCESS 00:00:02.339 ESP32 SUCCESS 00:00:04.880 ESP8266_d1_mini SUCCESS 00:00:05.696 nucleo_f446re SUCCESS 00:00:06.896 teensy41 SUCCESS 00:00:02.764 adafruit_feather_nrf52840 SUCCESS 00:00:04.180 bbcmicrobit_v2 SUCCESS 00:00:01.645 teensy35 SUCCESS 00:00:02.456 xmc1100_xmc2go SUCCESS 00:00:03.286

obones commented 2 years ago

Thanks a lot, this looks wonderful

RudolphRiedel commented 2 years ago

Well now, it's a start. :-)

The "5_plus" branch already has a modified EVE_target.h which makes the pins defineable in the platformio.ini:

if !defined (EVE_CS)

#define EVE_CS      10
#define EVE_PDN     8

endif

And the platformio.ini in the example on my drive starts like this:

[common] lib_deps = https://github.com/RudolphRiedel/FT800-FT813.git#5_plus build_flags = -D EVE_RiTFT43

[env] framework = arduino check_tool = cppcheck check_flags = cppcheck: --addon=cert.py --suppress=unusedFunction

[env:uno] platform = atmelavr board = uno build_flags = ${common.build_flags} -D EVE_CS=10 -D EVE_PDN=8

WIP