OpenStickCommunity / GP2040-CE

Multi-Platform Gamepad Firmware for Raspberry Pi Pico and other RP2040 boards
https://gp2040-ce.info
MIT License
1.5k stars 326 forks source link

Turbo rate is half of what is expected #853

Closed haktical closed 8 months ago

haktical commented 8 months ago

Prerequisites

Please check the following before posting an issue / bug report.

Context

Please provide all relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

Expected Behavior

Please describe the behaviour you are expecting.

Setting the turbo shot count to a given value will press the button ShotCount times per second - e.g. a shot count of 20 will press the button 20/s.

Current Behavior

What is the current behaviour?

Setting the turbo shot count to a given value will press the button ShotCount/2 times per second - e.g. a shot count of 20 presses the button 10/s.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. Set turbo rate to an expected value - I have tested at 10, 20, 30.
  2. Enable turbo for a given button.
  3. Measure the shot count - I specifically used a MiSTer via the NES core and the MashyMashy homebrew rom.

Screenshots & Files

My thoughts: Looking at the .cpp for the turbo code, it is only changing the button state ShotCount times per second. However, two state changes are needed for one "shot". I would probably suggest cutting the interval in half e.g. uIntervalMS = (uint32_t)(1000.0 / (shotCount*2)); or similar.

Relevant code from the current turbo.cpp pasted below:

uIntervalMS = (uint32_t)(1000.0 / shotCount);

[...]

if (getMillis() < nextTimer) {
    return; // don't flip if we haven't reached the next timer
}

bTurboFlicker ^= true; // Button ON/OFF State Reverse
nextTimer = getMillis() + uIntervalMS; // interval to flicker-off button
FeralAI commented 8 months ago

Good catch @haktical! This has been fixed and will be in the next release. You can test a prerelease version to verify the fix here https://github.com/OpenStickCommunity/GP2040-CE/actions/runs/8042892109?pr=863

haktical commented 8 months ago

Hey, thanks for looking at it. Not sure if it's the fix itself or another change in the prerelease, but turbo is pretty broken with it. I can't really test the specific rate because it'll go a few seconds at a time with no turbo input at all. Here is a video of a 10s test, with the turbo button at 20/s held the entire time - https://www.youtube.com/watch?v=ttbQOoZ3Mqk - I just did a normal update procedure to the pre-release. I've tried a variet of shot counts and the exact timings aren't the same, but the general behavior is repeatable. I tested a bunch of different shot counts, similar experience with all.

FeralAI commented 8 months ago

Thanks for the quick test and reply!

Interesting this isn't working for you. I've downloaded the same build and put it on an S13 and it seems to be working fine. What program/game are you using to test?

haktical commented 8 months ago

I was testing using the MashyMashy homebrew NES ROM. I tested both on a MiSTer with the NES core and on a PC using the BizHawk (QuickNES) emulator to run the rom. This is with a Haute42 gamepad (and the equivalent build). Is there any kind of factory reset etc. I should do before updating, or should it just work with the standard procedure? Also, looking at the input display on the device itself (maybe this isn't the best indicator, but regardless) seems to match what is being observed in the ROM.

FeralAI commented 8 months ago

Nothing on your side, I believe I see the issue. I'm doing some testing on a fix and should have something ready shortly.

FeralAI commented 8 months ago

Found the issue and a new build is available to test https://github.com/OpenStickCommunity/GP2040-CE/actions/runs/8043559619?pr=863

haktical commented 8 months ago

Initial testing seems good. I'm not getting perfect rates on PC but I'm tentatively chalking that up to a USB polling or emulator issue. Getting a perfect 20 when set to 20 on MiSTer and 29.2 when set to 30. I'll do some more iterations in the next day or two and provide another update. Seems great so far, though, thanks!

FeralAI commented 8 months ago

I'm using a bit of a timing offset to help with consistency. I'll work with that a bit and aim for more consistency.

haktical commented 8 months ago

I've done some more testing and I'm good calling this fixed. PC rates I was getting with my first tests were isolated to the one application I was using. Closing this issue. Thanks a lot for the fix!