search

ramapcsx2 / gbs-control

GNU General Public License v3.0
771 stars 110 forks source link

Add platformio with small impact #507

Closed Metaln00b closed 5 months ago

Metaln00b commented 8 months ago

This is a variant with a smaller impact to support platformio. This is not fully tested but it runs very well on my odv gbs-c

Fixed #505

kfix commented 7 months ago

I've done something similar on my local GBS-C repo a year ago, because I had used VScode+platIO for other esp32 projects and never really used Arduino Studio in the past and didn't want to invest in another toolchain to make some local modifications.

But now I want to try @nyanpasu64's improvements and don't really want to rebase it, so thank you for this PR! I'll give this a try.

skiphansen commented 5 months ago

@Metaln00b Thank you!

I can independently verify that

clone https://github.com/ramapcsx2/gbs-control.git
git remote add Metaln00b https://github.com/Metaln00b/gbs-control.git
git merge Metaln00b/add-platformio-small-impact
pio run
pio run -t upload

worked beautifully and I'm now playing with the GBS control menu.

I vote for this to be merged, saved me from installing the Ardunio IDE just to build this project.

ramapcsx2 commented 5 months ago

Hey, the problem is still that other developers need to verify that their workflows aren't broken by this. A platform change is substantial.

skiphansen commented 5 months ago

Absolutely! I just wanted to add my 2 cents that it worked for me and that I found it to be valuable.

Speaking of value, this is a FANTASTIC project you have here! Absolutely beautiful !

I'm using it for a different purpose than most. I'm using it for the same thing as this guy (https://github.com/ramapcsx2/gbs-control/issues/285) and I'm having the same problem. I'll provide more information for that issue later.

Thanks for the code !!!

nyanpasu64 commented 5 months ago

If there are multiple people in favor of this change, I'm tentatively in favor of merging it (while tagging the last Arduino-based commit), but we may need to rewrite the docs to avoid confusing new users. However I'm currently dealing with other issues (like getting flagged as under 13 on Discord), so cannot test.

skiphansen commented 5 months ago

I didn't look in detail, but there's no reason that adding platformio support should disable / remove Arduino support. At least not in general. It should just be an alternate method of building and working on the project.

Metaln00b commented 5 months ago

I didn't look in detail, but there's no reason that adding platformio support should disable / remove Arduino support. At least not in general. It should just be an alternate method of building and working on the project.

I agree with that. I also see that the Web UI in the Docs doesn't seem to be up to date either. We have the ability to maintain our own repos, that's true. I don't want to oppose anyone either. The right to decide is @ramapcsx2 and I respect that!

Friendly opinion: But I can see that some people tend to be put off. I'm referring to the OLED menu, for example: That was implemented really nicely and confirmed several times that it works, but it hasn't been merged yet.

Since an OLED darkens quite quickly after a while if it keeps displaying the same thing, I have made an adjustment so that the screen turns off after a few seconds in the main menu. Unfortunately, nobody has tested it yet except me. So I can understand if it is not merged.

I also think that if a commit breaks something, it would be possible to jump back one commit, that's what the VCS is there for. But it's also true that if you don't have time to moderate, that's not so great either.

ramapcsx2 commented 5 months ago

Just keep in mind I can't put any time into this. I'm always busy working on the other projects these days.. So I can not test any of this. If you guys say it works, then we can merge :)

nyanpasu64 commented 5 months ago

I didn't look in detail, but there's no reason that adding platformio support should disable / remove Arduino support. At least not in general. It should just be an alternate method of building and working on the project.

When I tried migrating to platformio (not being familiar with platformio), it would create a new project and copy existing source files to new locations. It seems that this PR adds new build files in parallel with the existing ones, without moving existing source files. If the build works, this is preferable and I'm willing to merge it.