Refactoring Away from INO Files?

Hi,

Working with WLED in Visual Studio Code + PlatformIO is pretty nice, except for the fact that *.ino files break the Intellisense parser. New contributors may be more comfortable if this project was formatted as a normal C++ project.

Is there any reason stick with the ino format, or is it just a result of slow growth over time? Would there be objections to refactoring? Obviously we’d lose Arduino Studio support, but I don’t think that’s much of a loss.

Hi,

yes, this is is a point to consider. Having IntelliSense available for WLED would be very nice. But don’t underestimate Arduino IDE, I myself am still regularily using it and a lot of especially beginners rely on it since “proper” IDEs might seem intimidating (although it is actually a lot easier thanks to the PlatformIO library management). First of all we should probably refactor all but one file to .h/.cpp and just keep the main wled00.ino to retain compatibility with arduino IDE.

1 Like

I am one of the folks still using the Arduino “IDE” especially since it is faster installed than any other IDE and if I have a new system, which I have every now and then then I can quickly back to business. Yes, It is a bit slower in compiling big projects, But then I can remember the time when we taped the code paper tape right after the assembler, started the paper tape reader and then went home across town for dinner. By the time we came back 3 hours later we just heard the last sounds of the paper tape puncher with the ready assembled binary or with 500 lines of errors. Waiting a few seconds longer is small to me as to compared to the ease and speed of deployment.

I’ve started a first pass at this.

Since the goal is just to support Arduino Studio, I’ve changed the ino file to just contain setup() and loop().

I’ve done the manual transformation of ino to h/cpp, currently just declaring functions in headers. Global variables are now extern, and a small amount of functionality is now contained in a WLED class.

There’s also the option of embracing OOP further, which would come in handy to expand automated testing in the future… Many of the standalone ino files would make great candidates for classes since they could keep track of their own state.

Before any further refactoring, any thoughts? Here’s my branch refactoring. It’s not ready to merge – I haven’t even tested it on a physical device yet, and there’s a bit of cleanup to do. I’m thinking the “wled_” prefix might be worth getting rid of for the source files as well.

edit: To be clear, I have it compiling for my specific board / pio env override and the others set as default targets. There may be compilation errors with other build flags.

I am certainly no OOP / C++ / Arduino / ESP expert, but I would expect fully utilizing OOP / C++ would use a good bit more memory because for classes, there are a lot more runtime checks required to validate data that are not necessarily there when compiling non OOP code. Sure, you lose some abstracting, but to squeeze as much as possible into 1MB of flash, is this a trade-off that has already been considered?

For example, what is the bin file size of your current refactoring? Kind of a “before” and “after” refactoring comparison? If the compilers are that good they actually reduce memory, then by all means, please continue! :smiley:

Took a look at your branch and I think I like the direction this is going!
Agreed that we can get rid of the wled_ prefix, it just bloats the filenames. It was only there so that arduino IDE would display the files in the correct order, and is not needed for non-.ino files.

Yes, just in my opinion, having classes is worth it for anything you might want to have multiple instances of - most integrations would currently just be a singleton, and keeping them static for now is most efficient.

One thing i’m not yet sure about is the file structure. I’m not sure if we should go for the traditional declaration in header/definition in cpp style (in your branch) or header only. I believe header only is easier to work with (no redundancy in method definition and having to go “back and forth”, half the amount of files), but obviously increases compile times. What are your thoughts on that? It also seems

Thanks for starting to work on a refactor, I believe it is much needed! I will also take a look at the variables, many of the currently globally defined variables are only used in one file and can thus be reduced in scope!

@huggy-d1 the effect of converting from .ino to .h/.cpp should be minimal. The compiled code doesn’t really change that much, just the form the source code is presented does. Transitioning most of the codebase to OOP could indeed increase binary size, but i’d be more worried about heap memory (RAM) usage. We already have only like 20kB to our disposal on ESP8266, and instance pointers and dynamic allocations could quickly worsen the situation, so we should only use them where necessary.

1 Like

With a single class added:
// Original
RAM: [= ] 14.6% (used 47,968 bytes from 327680 bytes)
Flash: [======== ] 79.3% (used 1,039,142 bytes from 1310720 bytes)

// Refactoring usage for my esp32 + flags
RAM: [= ] 14.7% (used 48,256 bytes from 327680 bytes)
Flash: [======== ] 79.5% (used 1,042,650 bytes from 1310720 bytes)

// D1_mini
RAM: [====== ] 60.3% (used 49,400 bytes from 81920 bytes)
Flash: [====== ] 62.5% (used 652,904 bytes from 1044464 bytes)

// Refactored
RAM: [====== ] 60.3% (used 49,412 bytes from 81920 bytes)
Flash: [====== ] 62.6% (used 654,244 bytes from 1044464 bytes)

AFAIK, the main thing we’d have to avoid are virtual functions. I think it’s important to remember that much of the library code we’re using, both imported into our project and from Arduino, uses classes.


Regarding .h/.cpp:
I do agree that header only files are easier to work with. The amount of files effectively doubling is kind of a pain. However, refactoring the existing inos or my branch to be header only would require forward declarations of certain functions (notify’s and led’s functions come to mind), in which case you’re still duplicating the declarations and that declaration must be changed in places you wouldn’t expect.

One thing to note, if any of the non-const global variables are moved to more appropriate files, then we’ll still need the cpp file to define it anyway.

As for compilation times, the difference is nice compared to ino, but it’s a difference of 20 seconds at most.

2 Likes

I think this would help me too. However I’m new user and am honestly not sure, here is why.

I am a newer contributor and right or wrong, 6 months ago, I decided to jump straight into programming ESPxx using MS Visual Studio. The learning curve is steep (my last formal programming was in Pascal) but I’m hoping features like intellisense will pay dividends in the long run. For that reason I intend to stay the course. This strategy has worked great for small projects, but I’ll be honest the size of the project is daunting.

Wled is absolutely AWESOME!!!

I’ve been using Wled successfully in MS Visual Studio for the last few months, but recently started getting more and more errors and can no longer compile. I noticed someone fixed a few of these recently for VS, but I Being new I can’t tell if these new errors are a result of how I am using GitHub, VS or both. Right now “I believe” I have properly “forked” from Aircookie\Wled and then cloned this to my local drive, and am NOT modifying the code.

I’m happy to provide whatever additional details to anyone willing to get me pointed in the right direction.

It will take me a bit more to be able support to others, until then appreciate the any ones support until then.

1 Like

Why not give the user the choice?
We can support both ide systems.

A. Platformio
B. Arduino IDE

There are a lot of users outside who starting with DiY and they are confused enough with IDE things. It is not our task to confuse them more i think.

@Aircoookie you could ask Jason2866 :wink: . He has a ready to use platformio setup for the other dev thing. You know what i mean. With that the user don’t have to do the complete install oif Platformio which is for beginners trouble enough.

Give the user the choice not only some experienced users which have done a lot of things to get to the good platformio work. Let us not forget where we all came from…

So far my 2 cents.

@mike2nl hmm I don’t really get your point about giving the user a choice, of course Arduino IDE will remain supported! It is a lot more familiar to many users even though they have to install many libraries.

For me at least, getting platformio to run is really quite simple for everyone (install Visual studio code, then add the platformio extension from the extension manager thingy), but I agree providing a small guide would be helpful!

I will contact Jason, I am interested in why Tasmota is still primarily using ino files.

I’ve opened a pull request with my changes.

I don’t have an ESP8266 to test with, so I’ve only confirmed working builds with an ESP32.

There shouldn’t be any functional changes, but if there are, they probably stem from differing include orders. Changing the include order exposed a couple other bugs that were masked, such as std::min/max and #define min/max conflicts. Those have been fixed, but I haven’t tested every possible define.

I did import the dmx output library into our modified dependencies folder, since it may require modification by the user anyway, at least according to the comments.

Awesome, thank you! I merged your PR onto a new testing branch. It seems to work well also on ESP8266 and with different combinations of defines.

I’ve consolidated most header files into func_declare.h while keeping the source files. The name should make clear what it does. This significantly cuts down on the amount of files needed.

Great! I’ll be honest, I didn’t consider using a common header, but that definitely solves the file-count problem.

One thing that bothers me still is the global state variables. It’s just so verbose, extern declarations w/ cpp definitions… In a further refactoring branch I started tackling that by giving many of the cpp files their own classes if it would make sense for them to own state. Header only may make sense for those as well.

What more are you aiming for before merging the refactor branch?

As part of the refactoring, is there a more formal api for usermods, or do they still need to modify global vars to take over - when needed / appropriate for the usermod?

Will that simply change how vars are accessed? “Global class instance”.property .method rather than vars? Or, perhaps properties that can be set from a usermod that prevents the normal wled loop from taking control back from the usermod?

These “usermod” API calls to override settings in usermods only applies when needed to bypass / override the APIs/UI settings, perhaps adding custom effect(s), and such.

I applaud the refactoring knowing it adds such a small fraction, only ~0.025% RAM & ~0.205% Flash for D1 Mini, of overhead. With the common header file, how did that affect the refactored values from your post 3d ago?

Is there a performance benchmark the resulting code needs to perform? Frame rate > x fps at y addressable LEDs? If so, has that changed? Either capability, higher fps or more LEDs at stated fps target? I assume fps / max LEDs is less important than being able to squeeze the firmware onto more devices, targeting the 1MB ESP’s as the low-end.

You good peeps already converted me. I’m just wondering as I work on usermods how I might need to refactor my own work so it will work post refactoring, perhaps work even better.

Maybe an example usermod class with properties and methods available for all usermods that allows better integration with the refactored more class-based WLED?

Moving function declarations into the common header didn’t affect space at all. Why would it? :stuck_out_tongue:
I don’t have any performance differences to share, and I can’t expect there will be any. A performance benchmark would be nice! The closest I’ve got is just testing the existing patterns, strobe, pride 2015…


Global variable interface: Honestly, once my IDE actually provided useful help they’re much easier to work with. I don’t see a compelling reason to just move everything into a Config struct at the moment. As for the singleton pattern, avoiding the static order initiliazation problem may be a good reason to use it in the future, if further refactoring provides value. Right now we’d never hit that.

As for usermods, I’m not sure what you mean with “modify global variables to take over”. You could add a flag to the main loop and set that in your usermod, but my impression was that usermods were not really designed for that? I’m trying to see what that would add versus the usermod author just deciding when to yield control back to the main loop… Could you explain what you’re doing? Maybe I’m just tired!

I wouldn’t worry about this making your code harder to write. If everything you’re writing is in the usermod.ino file, then you shouldn’t have to do anything at all.

edit: I saw your post about the QuinLED. I’ll get back to you on that.

@pille would be interesting to know what caused the setup and loop not defined issue and how you got it to compile. (in case others run into it too)

Thanks :slight_smile:

there was some hiccup that led to wled00.ino missing.
at first i blamed the refactoring, then i found a fresh build environment (platformio/WLED) worked as expected and a fresh pull solved this. i don’t think this is something you’d run into with a clean environment.

1 Like