Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename wled00.ino to wled_main.cpp #4090

Merged
merged 3 commits into from
Aug 15, 2024
Merged

Conversation

willmmiles
Copy link
Collaborator

PlatformIO's Arduino compatibility shim forces an intermediate .cpp file to be recreated and rebuilt on every build. Apart from being wasteful of time, this can also cause problems when batch building at the command line when the VSCode IDE is trying to build the cpp plugin configuration, resulting in failed builds as the intermediate file is rewritten while being built.

As the Arduino IDE is no longer supported by this project, it should be safe now to rename the .ino to eliminate these small headaches.

This fixes platformio continually recompiling this file.
@softhack007
Copy link
Collaborator

Thanks @willmmiles I think its reasonable to cut the last ropes for arduino IDE - @blazoncek what do you think?

@blazoncek
Copy link
Collaborator

TBH IDK. 🤷‍♂️
It does not bother me.

@robertlipe
Copy link

Bystander comment: "Yes, please." It gives arbitrary editors a better chance of recognizing the file as C++ and helpfully syntax highlighting it. GitHub, for example, has lots of tools that can automatically recognize .cc and .cpp files.

Not to lose focus, but if renaming it is on the table, why the "00" at all? Would a better name be considered? wled_effects.cpp or ... anything more meaningful.

@softhack007
Copy link
Collaborator

Not to lose focus, but if renaming it is on the table, why the "00" at all? Would a better name be considered?

@robertlipe the "00" is a relic from ArduinoIDE - Arduino expects to find the main sketch in <Folder>/<Folder>.ino. wled00.ino is basically a stub that calls WLED::instance().setup() and WLED::instance().loop(). We could rename the file as wled_main.cpp if that is more descriptive. What do you think?

* added #include <Arduino.h> - this is basically what the preprocessing tool (wled.ino -> wled00.ino.cpp) does
* added a comment that Arduino IDE is not supported, use platformIO.
@robertlipe
Copy link

I don't do Arduino when I can at all avoid it and avoiding their IDE is easy, so that history is new to me. (The whole concept of renaming 'char', not calling source files source files, reinventing STL ... badly, and such is just weird to me.) Thanx for the explanation.

Yes, I think that's a better name. It's just something that's always bugged me, so if you're making the move to rename it, might as well make it count.

Note that I'm not a project owner, so my opinion may not count.

@softhack007 softhack007 changed the title Rename wled00.ino to wled00.cpp Rename wled00.ino to wled_main.cpp Aug 14, 2024
@softhack007
Copy link
Collaborator

🤔 my first local build ended with "NeoPixelBusLg.h not found". After deleting .pio and HOME/.buildcache and .vscode/cpp_properties.json, the build was successful.

@willmmiles @blazoncek can you double-check that local builds still work for you?

@willmmiles
Copy link
Collaborator Author

🤔 my first local build ended with "NeoPixelBusLg.h not found". After deleting .pio and HOME/.buildcache and .vscode/cpp_properties.json, the build was successful.

@willmmiles @blazoncek can you double-check that local builds still work for you?

It built just fine for me, no trouble at all. I tried a couple of things to see if I could get it to fail -- building 0_15, then switching to this branch, then building without reconfiguring PlatformIO, and it still succeeded.

@softhack007 softhack007 merged commit 5cb49c8 into Aircoookie:0_15 Aug 15, 2024
18 checks passed
softhack007 added a commit to MoonModules/WLED that referenced this pull request Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants