search

ducalex / retro-go

Retro emulation for the ODROID-GO and other ESP32 devices
GNU General Public License v2.0
489 stars 114 forks source link

On develop branch reset init lcd screen missing #99

Closed Akrobate closed 5 months ago

Akrobate commented 5 months ago

Hello,

Working on reintegration of my updates on the updated develop branch, and noticed that in your refactor of screen init to move it in target you erased this part:

#if defined(RG_GPIO_LCD_RST)
    gpio_set_direction(RG_GPIO_LCD_RST, GPIO_MODE_OUTPUT);
    gpio_set_level(RG_GPIO_LCD_RST, 0);
    usleep(100 * 1000);
    gpio_set_level(RG_GPIO_LCD_RST, 1);
    usleep(10 * 1000);
#endif

in lcd_init() function in rg_display.

Without this block my screen remains empty. Adding it back fixes my problem.

Just for your intention: My screen doesn't have a CS pin. Maybe this block is not required on screen that do have it, but in my case reseting the screen like this is required to work. I think it should not have a great impact on those who do not need it.

Furthermore, this little refactoring of the code is very elegant. I like it!

Also Just an idea but maybe thinking about a change of inputs in this way would be coool! I'm aware that it's not so easy and structured as for the screen, but just for instance, in my case I do use only pins with pull up resistors, so the OrdoidGo input is'nt correct for me because of the two commented lines of setting the pull up resistors. So I duplicated this zone putted if with RG_GAMEPAD_DRIVER = 7 . Not the best way to fix, but haven't see something better to not broke the working odroid drive.

And for information all your fixes on TARGETS, and adress of the image to write works perfectly on esp32s3 =)

ducalex commented 5 months ago

Thanks, I don't know what I was thinking and I added back support for the reset pin!

I agree about the GPIO driver, we need a better system. We don't even currently support the extra buttons X/Y/L/R. Or digital IO for the dpad.

In fact I already built a better system that simply reused the RG_GAMEPAD_MAP and got rid of the GPIO constants but I never merged it. I'll see if I can fix it up and push it soon!

But if you want to do your PR before I do that, feel free to just use the #ifdefs. And if absolutely necessary adding a new driver is also fine.

ducalex commented 5 months ago

Oh, I just saw you already did your PR ;)

We can continue this conversation there.

Akrobate commented 5 months ago

Hello!

Ok understood! So what do you think about my PR?

Just by curiosity you said "We don't even currently support the extra buttons X/Y/L/R", how many are you to working on this project? I though you was the only developper on this project ;-)

ducalex commented 5 months ago

Just by curiosity you said "We don't even currently support the extra buttons X/Y/L/R", how many are you to working on this project? I though you was the only developper on this project ;-)

There's been a few contributions over the years, that's why I say "we", but yes it's mostly just me :).

Ok understood! So what do you think about my PR?

The code is all good but I needed some time to review the docker changes!

Akrobate commented 5 months ago

Ok! Let me know if you need some explanations