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

PlatformIO integration #47

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

quotschmacher
Copy link

  • modified for easy usage with PlatformIO
  • removed compiler warnings

svenalbert and others added 3 commits September 19, 2019 08:03
- moved ino-file to new folder
- removed compiler warnings
- changed arduino libraries to direct links to arduino lib directory
@quotschmacher
Copy link
Author

With those changes you just have to clone the TonUINO project into the PlatformIO projects folder, open the *.ino and build it.

@DerTomm
Copy link

DerTomm commented Jan 1, 2020

Hi quotschmacher,

I also just cloned the Tonuino repo and made the DEV branch PlatformIO ready as I do want to contribute to the project and need a real IDE for development.
After comparing my setup with the one of this pull request some remarks have come to my mind:

  • Moving Tonuino.ino into a subfolder TonUINO will break the default directory structure for all users and developers who still want to use Arduino IDE.
    If you use this platformio.ini setting the .ino file can stay where it is to keep the directory structure compatible with Arduino IDE:
[platformio]
src_dir = ./
  • I would recommend to add all PIO specific files to .gitignore so that regular users wont get confused by files which are not needed for them.

Finally I realized that you used the master branch as source for your modifications (and you want to merge the changes into master again).
@xfjx: Am I right that the intended contribution process uses DEV branch as source and target for pull requests?

Regards,
Thomas

junkdna added a commit to junkdna/TonUINO that referenced this pull request Jan 3, 2020
inspired by
xfjx#55
xfjx#47

Signed-off-by: Tillmann Heidsieck <theidsieck@leenox.de>
junkdna added a commit to junkdna/TonUINO that referenced this pull request Jan 4, 2020
inspired by
xfjx#55
xfjx#47

Signed-off-by: Tillmann Heidsieck <theidsieck@leenox.de>
@AlexanderWillner
Copy link

@DerTomm : Moving Tonuino.ino into a subfolder TonUINO will break the default directory structure for all users and developers who still want to use Arduino IDE. -> why should this break the Arduino IDE? Works for me in https://github.com/AlexanderWillner/TonUINO/tree/cleanup

@DerTomm
Copy link

DerTomm commented Jan 8, 2020

But why it is even necessary to move the file into a subfolder? At least in my environment PIO just takes the standard Tonuino.ino as main source file without any modifications. It was neither needed to rename its extension to .cpp nor did I have to move it to get it compiled.

@AlexanderWillner
Copy link

But why it is even necessary to move the file into a subfolder?

Not necessary, but best practice to have documentation, sources, tests, resources, ... in separate folders. Further it is necessary to use arduino-cli (however, I wrote a workaround).

At least in my environment PIO just takes the standard Tonuino.ino as main source file without any modifications.

This is correct. The current dev branch also follows this approach. However, the cleanup branch is using this ... well ... cleaner approach.

It was neither needed to rename its extension to .cpp nor did I have to move it to get it compiled.

Correct, not needed. However, best practice to use cpp file extensions for cpp source code. One reason: https://docs.platformio.org/en/latest/faq.html#convert-arduino-file-to-c-manually

@DerTomm
Copy link

DerTomm commented Jan 9, 2020

Okay, understood. On the other side I think that - as they want to keep the code and directory structure as close to plain Arduino IDE as possible - the maintainers won't accept PIO related PRs which will change this structure quite substantially.

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