Idea: Port WLED to PlatformIO IDE (PIO)

wiesendaniel commented 5 years ago

Idea

Port this Project to PlatformIO IDE (PIO).

Reason

With PIO it's far easier to manage larger projects like WLED with multiple dependencies and multiple build targets than it is with Arduino IDE. As well as it is far more feature rich and more comfortable to use as Arduino IDE. (IMHO)

Origin Story

This all started in another issue which I link here for reference. https://github.com/Aircoookie/WLED/issues/69

TL;DR

I had an issue and tried to compile from source with PIO and failed. I then proposed the idea to port WLED to PIO as it might help with exactly that kind of issues and the reasons stated above. Aircoookie wants to give it at least a shot and I'd like to help out with an initial port for testing.

Progress

It's not much (as in not working right now) but it's an initial commit. I'm going to keep committing to this branch as I go and keep you posted about progress in this issue. https://github.com/wiesendaniel/WLED/tree/feature/pio-port

Aircoookie commented 5 years ago

Awesome, thanks for your help!

I downloaded PIO (VSCode version), gave it a try and managed to successfully compile and upload a version to my Wemos D1 via OTA! That said, I couldn't yet figure out the whole /src and /lib folders. I really like the interface and features of the IDE. There are really only two gripes with it I found: The autocomplete doesn't really work with functions in another file. Probably this can be resolved by switching from .ino to .cpp. The second one is that I found the function where you can find the connected devices (serial or mDNS), but can't actually select them for upload, I have to manually add it in the platform.ini file. Is that the same for you?

wiesendaniel commented 5 years ago

The least I can do is give back. I can't imagine how much time you must have put in this project by now. :-)

Something came up today and I didn't really get a chance to work on this. Would you mind pushing your changes to a new branch? I'd really like to take a look. :-D

I struggle with the hole concept of this lib folder myself. In my project I ended up with a lot off OOP code (which wasn't exactly ideal in terms of memory usage as I think by now) and it looked something like this:

├── lib
│   ├── ConfigManager
│   │   ├── ConfigManager.cpp
│   │   └── ConfigManager.h
│   ├── HardwareController
│   │   ├── AbstractHardwareController.cpp
│   │   ├── AbstractHardwareController.h
│   │   ├── LightController.cpp
│   │   └── LightController.h
│   ├── LedEffects
│   │   ├── Effect.cpp
│   │   ├── LedEffects.cpp
│   │   ├── LedEffects.h
│   │   └── State.cpp
│   ├── MqttJsonClient
│   │   ├── MqttJsonClient.cpp
│   │   └── MqttJsonClient.h
│   ├── OTAUpdate
│   │   ├── OTAUpdate.cpp
│   │   └── OTAUpdate.h
│   └── readme.txt
├── platformio.ini
└── src
    └── main.cpp

Where every Class under a folder in lib had had a loop and a setup method which were glued together in the src/main.cpp. Autocomplete might has something todo with the code placement and/or the extension as you already set. But actually it is working for me when I try to autocompletet userBegin() from usermod.ino in wled00.ino.

I flash my ESP01 chips via CH340 usb serial programmes. On my mac it is auto detecting the serial programmer and flashing to it. I believe in an earlier version I had to selected from a drop down. I press Cmd + Shift + p and type in upload and select PlatformIO: Upload.

Output:

...
Configuring upload protocol...
Looking for upload port...
Auto-detected: /dev/cu.usbserial-14310
Uploading .pioenvs/esp01/firmware.bin
...

wiesendaniel commented 5 years ago

While working on this and still having the .ino extension, I get prompted a lot to install the Arduino extension. I installed it by accident and turns out to conflict with PIO. Maybe thats whats causing your autocomplete issue.

wiesendaniel commented 5 years ago

Ok, I made a clean install of VSCode as I had a bunch of extensions installed. Now I can reproduce the behaviour you have.

I have a few boards/targets building now:

Environment nodemcuv2   [SUCCESS]
Environment d1_mini     [SUCCESS]
Environment esp01_1m    [SUCCESS]
Environment esp01       [ERROR]
Environment esp32dev    [ERROR]

Where esp01 is still the size issue. My build flags might be wrong.

I tried a simple travis ci config: https://travis-ci.org/wiesendaniel/WLED/builds/453647351 The results are different. I need to look into them tomorrow.

I pushed my changes. To the branch linked above.

Aircoookie commented 5 years ago

Nice, I'll check out your code in detail tomorrow!

Regarding your targets, for esp01 you need to uncomment some of the #define DISABLE_STUFF in wled00.ino for the binary to be small enough to fit. For esp32, ARDUINO_ARCH_ESP32 must be defined. The arduino IDE does that when building for esp32, but I'm not sure about PIO. Also, in NpbWrapper.h, either the WORKAROUND_ESP32_BITBANG must be defined, or a custom version of NeoPixelBus with custom RMT pixel driver installed.

debsahu commented 5 years ago

Greetings, I just compiled on PIO platform.ini

[platformio]
src_dir = ./wled00
;env_default = nodemcuv2
;env_default = lolin32
;env_default = lolin32RMT
data_dir = ./wled00/data
lib_extra_dirs = ./wled00/src

[common]
build_flags =
  -D VERSION=0.8.1
  -D DEBUG=1
monitor_speed = 115200
framework = arduino
board_build.flash_mode = dout
upload_speed = 921600
upload_resetmethod = nodemcu
lib_deps =
  NeoPixelBus
  FastLED

[env:nodemcuv2]
board = nodemcuv2
framework =  ${common.framework}
platform = espressif8266@1.8.0
build_flags = 
  ${common.build_flags}
  -DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH
  -Teagle.flash.4m.ld
monitor_speed = ${common.monitor_speed}
upload_speed = ${common.upload_speed}
upload_resetmethod = ${common.upload_resetmethod}
lib_deps = ${common.lib_deps}

[env:esp32dev]
platform = espressif32
board = esp32dev
monitor_speed = ${common.monitor_speed}
upload_speed = ${common.upload_speed}
framework = ${common.framework}
lib_deps = ${common.lib_deps}
build_flags =
  ${common.build_flags}
  -DARDUINO_ARCH_ESP32
  -DWORKAROUND_ESP32_BITBANG
;#define WORKAROUND_ESP32_BITBANG in NpbWrapper.h

[env:lolin32]
platform = espressif32
board = lolin32
framework = ${common.framework}
lib_deps = ${common.lib_deps}
monitor_speed = ${common.monitor_speed}
upload_speed = ${common.upload_speed}
upload_protocol = esptool
build_flags =
  ${common.build_flags}
  -DARDUINO_ARCH_ESP32
  -DWORKAROUND_ESP32_BITBANG
;#define WORKAROUND_ESP32_BITBANG in NpbWrapper.h

[env:lolin32RMT]
platform = espressif32
board = lolin32
framework = ${common.framework}
lib_deps =
  FastLED
  https://github.com/svenihoney/NeoPixelBus/archive/b0485e09b374b368d5445d319b12501886da9788.zip
monitor_speed = ${common.monitor_speed}
upload_speed = ${common.upload_speed}
upload_protocol = esptool
build_flags =
  ${common.build_flags}
  -DARDUINO_ARCH_ESP32

I copied NpbWrapper.h, palettes.h, WS2812FX.h & WS2812FX.cpp into src/dependencies/ws2812fx folder and changed #include "WS2812FX.h" to #include "src/dependencies/ws2812fx/WS2812FX.h"

debsahu commented 5 years ago

@wiesendaniel Make changes to WS2812FX.h as mentioned in https://github.com/Aircoookie/WLED/issues/61#issuecomment-438432884 to get this to compile for ESP32.

wiesendaniel commented 5 years ago

I got esp32 working! Thank you @debsahu

512k support is still at 102% of the size limit. Even with all the disabled flags set. I tried with and without spaces in between the -D and the name. No luck...

build_flags =
  -DWLED_DISABLE_MOBILE_UI
  -DWLED_DISABLE_OTA
  -DWLED_DISABLE_ALEXA
  -DWLED_DISABLE_BLYNK
  -DWLED_DISABLE_CRONIXIE
  -DWLED_DISABLE_HUESYNC

I configured travis to log verbose, to see whats wrong there. Im out of time and look into it tomorrow.

I found some information concerning the .ino extension and code completion: https://docs.platformio.org/en/latest/faq.html#convert-arduino-file-to-c-manually As the conversion is not a simple rename (tried that before searching. 🤦‍♂️ ) im not going to do this just for PIO support. As far as I understand the code it might involve a bit more work and moving code around. I think it requires #include statements in order to have functions from other files in scope when using .cpp. Besides that code order matters if you don't define a function before usage. I always wondered why code order isn't a thing in Arduino code... Might be a thing to keep it easy for beginners. I didn't looked into the code to see if code order is a problem for WLED as it is tho.

wiesendaniel commented 5 years ago

I couldn't resist and took a quick look. TravisCI Build log: https://travis-ci.org/wiesendaniel/WLED/builds/454748674

It seams like a weird path issue. I think (and thats a very quick and rather vague assumption) it's due to the .ino extension. I noticed a lot of temporary renaming/copying to .cpp in my source folder. Im not quite sure but I think I got some unreliable build results. From time to time it seemed to fail when I ran the build right after changing something in the PIO config. Maybe it's a race condition in PIOs toolchain and maybe it's reproducible in CI due to its quick and "throw away" nature. I remember something about PIO using plain GCC in the background. Assuming that, at some point the files need to be valid .cpp files. So at least the renaming PIO does makes kind of sense.

Aircoookie commented 5 years ago

Ok, seems like the "magic" Arduino/PIO does boils down to:

copy-pasting all .ino file into one huge .cpp (at least that's likely)
defining all functions at the top, that way function order doesn't matter
adding #include <Arduino.h>

If I have time, I might reduce the code to a single controller .ino file, with the individual features all in separate .cpp/.h files.

@wiesendaniel About esp01, PIO is probably using the 2.4.0 arduino framework or newer. I still use 2.3.0 because it works just as well for my application while using up way less flash. I don't know though whether there is a way to change the version in PIO.

debsahu commented 5 years ago

Ok, seems like the "magic" Arduino/PIO does boils down to:

copy-pasting all .ino file into one huge .cpp (at least that's likely)

defining all functions at the top, that way function order doesn't matter

adding #include <Arduino.h>

If I have time, I might reduce the code to a single controller .ino file, with the individual features all in separate .cpp/.h files.

I will look into it, .ino -> .cpp may not be necessary.

@wiesendaniel About esp01, PIO is probably using the 2.4.0 arduino framework or newer. I still use 2.3.0 because it works just as well for my application while using up way less flash. I don't know though whether there is a way to change the version in PIO.

platform = espressif8266@1.5.0 < core 2.3.0 platform = espressif8266@1.6.0 < core 2.4.0 platform = espressif8266@1.7.0 < core 2.4.1 platform = espressif8266@1.8.0 < core 2.4.2

wiesendaniel commented 5 years ago

I've got everything to compile now (locally) and pushed my changes.

Aircoookie commented 5 years ago

Thanks for the PR! Awesome you got it working nicely! Also thanks for your help @debsahu !

Just two things:

I noticed you moved WS2812FX into the src/dependencies folder. Is that required or can we keep it in the root directory? I know it technically makes more sense to have it in the dependencies since it's a library, but this is an integral part of the program and it would be great if we could still edit it from within the Arduino IDE, so I'd rather we keep it in the root if possible. Same for NpbWrapper.h. It contains some settings like LED pin that might have to be changed by the user frequently.
Can you open a PR against my development branch? That would make it way easier for me to merge with my local changes.

Thank you for getting it to work nicely with PIO! I highly appreciate your work :)

wiesendaniel commented 5 years ago

Actually It would be best if all libs were left unchanged and are downloaded by PIOs dependency management. I my self encountered the situation with pubsubclient were it wasn't possible (change of payload sizing) without changing it directly. For that I made a fork of pubsubclient on github and configured it in platformio.ini. (using the git url instead of the lib name under lib_deps)

In case of WS2812FX it seems to me that it's completely written in OOP code. Inheritance might be the best solution to keep the lib as it is.

I think there was no reason for me to move it besides the hints from @debsahu and the feeling that libs shouldn't be copied (commited) and altered.

I can restore it later this day to be in the root folder if you like. I changed the PR base branch already. ;-)

wiesendaniel commented 5 years ago

I moved them back to where they were. :-)

Aircoookie commented 5 years ago

Thnaks! :) I just merged your PIO branch with my gzip changes!

wiesendaniel commented 5 years ago

Oh thanks for the merge and your support. 👍 Can't wait to test gzip it. I hope I get travis to work on sunday. Maybe even some of the homeassistant stuff I have in mind.

Aircoookie commented 5 years ago

Thanks again for your help with PIO! 0.8.2 has a new external dependency, IRremoteESP8266@2.5.3. Even though it seems quite straightforward, I am not 100% sure on how to add it to the platformio.ini, because it is only a dependency for ESP8266 (ESP32 doesn't support IR that easily because of some interrupt issues). Also esp8266_512k should get a new build flag WLED_DISABLE_INFRARED. Could you do a quick PR with those changes?

debsahu commented 5 years ago

Add IRremoteESP8266@2.5.3 into lib_deps = for each of the ESP8266 environment. And @Aircoookie you can include IRremoteESP8266.h only for ESP8266 in your code with #if defined(ESP8266) and #endif

Aircoookie commented 5 years ago

Thank you, I'll take care of that tomorrow!

Aircoookie / WLED