corneliusmunz / legoino

Arduino Library for controlling Powered UP and Boost controllers
MIT License
257 stars 34 forks source link

Make PowerFunctions library useable for ESP8266 #42

Closed Mattze0815 closed 3 years ago

Mattze0815 commented 3 years ago

First of all congratulations to your wonderful library. As discussed, we are using it in our Mattzobricks project of automated LEGO trains (www.mattzobricks.com).

When having the Legoino library installed, it is impossible to compile a firmware for an ESP8266 that uses the PowerFunctions library.

1. If you include the PowerFunctions library, the compile will fail, because all other parts of the Legoino will also be compiled - this will fail on ESP8266 because it does not support Bluetooth.

2. If you install the PowerFunctions library in an extra folder, this will lead to a conflict between the PowerFunctions library that is included in the Legoino library.

It would be great if you had a solution for this.

Thanks a lot!

corneliusmunz commented 3 years ago

Hi @Mattze0815 ! Thanks for opening the issue. Now it is visible to all users 👍

I can reproduce the issue on my side. I have tried several things (remove *.h files out of the library.properties definition) but unfortunately without luck. The IDE always tries to compile all files in a library folder.

So i think the only way to overcome this is to split the PowerFunction stuff into a separate library which then could be marked as compatible with more architectures (avr, esp8266, esp32, ...) I will think of that and maybe split the libs into two libraries and mark the PowerFunction library only as dependency to the legoino library. Overall this would be better in the sense of "seperation of concerns"

If you have another idea, just give me a hint

fvanroie commented 3 years ago

Maybe I don't fully understand the problem, but can't you just encapsulate the ESP32 code with:

#if defined(ARDUINO_ARCH_ESP32)

or

#ifndef ARDUINO_ARCH_ESP8266

However since PowerFunctions (IR) and Lpf2 (BLE) are two separate protocols, it might make more sense just to split them.

corneliusmunz commented 3 years ago

@fvanroie Good idea! I will try it out and it should work as you proposed. I think i will do this as a quick fix. Nevertheless i will think about a split of the functionality into two libraries.

corneliusmunz commented 3 years ago

@fvanroie : Your #if defined(ESP32) idea has worked without any issues. Now the PowerFunction stuff will work for all platforms (AVR, ESP8266, ESP32) I have added a github action workflow to ensure that the PowerFunctions.ino file will compile on the desired hardware targets. Thanks again for that easy and obvious hint which i have overseen 😄 @Mattze0815 : Would be great if you can give it a try with the commit https://github.com/corneliusmunz/legoino/commit/b9f76b1ce43f7418a1de2883ca0fb72c5fe9c8da on the develop branch and give feedback if it will work on your side

corneliusmunz commented 3 years ago

Issue will be fixed with the newest library release: https://github.com/corneliusmunz/legoino/releases/tag/1.0.3