darthcloud / BlueRetro

Multiplayer Bluetooth controllers adapter for retro video game consoles
https://blueretro.io
Apache License 2.0
1.3k stars 109 forks source link

[LEDs] Add configuration for alternate LED pins #886

Closed patrickmollohan closed 7 months ago

patrickmollohan commented 8 months ago

to accommodate the NGC RetroScaler hardware. Uses pins 14, 32, 33, and 25 for LED pins 1-4 instead of pins 2, 4, 12, and 15.

darthcloud commented 8 months ago

First thanks for the PR.

But I don't think I will accept it. Some previous context in the retroscaler repo: https://github.com/RetroScaler/NGC-BlueRetro/issues/3#issuecomment-1412020474

I'm still butt-hurt about this TBH.

They also got other products with that very same issue, is it the same pins? Who knows anyways.

They do sponsor the project for the last two years monthly.

But in comparison to how much they make selling theses adapters I think its very little.

So I don't think I want to add extra binaries because they didn't take the time to sync with me before releasing their products.

They decided to make custom FW, then its up to them to keep it updated.

I realized its not the best for the end users but I don't feel like having to support their mistakes.

patrickmollohan commented 8 months ago

@darthcloud My pleasure, and thank you (and all the other contributors) for the hard work! It has truly been a pleasure to have the internal GC mod and an external one for my Panasonic Q working as well as they do. I understand your position on the matter, and I don't blame you for feeling like that.

I figured that might have been the case, but as you mentioned, the end-users do get hurt, hence why I decided I would make the PR. The comment right below your own that you linked would be an example of this. Sometimes, consumers don't know better, and end up with the knockoff; in my case, it was listed as the top result for "gamecube bluetooth adapter" on Amazon. I purchased it after reading it used the BlueRetro firmware (which I had a great experience with already), not realising it was a custom firmare. I, too, only realised it after the return period expired.

I 100% agree they should have kept within specs as to avoid this mess, but sadly, they didn't. I was able to figure out what their modifications were to the firmware, and realised (after merging their code with the latest BlueRetro source) that it was really sloppy. I'm not sure my solution is much better, but I tried to write it in a way that hopefully wouldn't cause too much headache to maintain in efforts to find a happy medium for you and the end users.

I absolutely will accept whatever outcome you wish for the PR, but I hope you'll at least consider it for those of us who have been duped.

darthcloud commented 7 months ago

I think I come up with a good compromise for everybody.

I made a feature for allowing makers to customise BlueRetro more easily and we can use the same framework for fixing the RetroScaler LEDS.

See the third section of the docs: https://github.com/darthcloud/BlueRetroWiki/blob/master/BlueRetro-NVS-Config-Generator.md#3---flash-nvsbin-to-esp32

Here a binary that change the LEDs pins: nvs_rs_leds.zip

First unzip it to get the .bin

Then flash it on BlueRetro as the docs say. (You need to have an adapter with a USB port)

You also need to install the latest beta build: v1.9.2-beta-15-g789cca8_hw1.zip

patrickmollohan commented 7 months ago

I haven't had the time to try it out yet, but looks like a better solution. Thank you very much for coming up with this fix! Will close this PR.

Sidenote: the link to copy the Google sheet in BlueRetro-NVS-Config-Generator.md has an extra "\" at the end that results in an error; removing it fixes this.