fruit-bat/pico-zxspectrum

Cannot get back into menu after loading and running a snapshot

Closed this issue · 11 comments

I found this when testing the Olimex port, but it occurs on other boards too.

Steps to reproduce:

  1. Enter the menu, select a snapshot a load it
  2. After the snapshot is running, press F1 to re-enter the menu. The menu snapshot menu flashes up, then goes back to the running program

This only occurs with new builds, it does not occur with the uf2 in the repo.

Note that F1 does work in the initial case to load the snapshot, it is subsequent attempts after the snapshot is running that generate this behaviour.

I wonder if this is an unwanted side effect of 8594253 Is the F1 to enter the menu auto repeating to generate the F1 to exit the snapshot menu?

I reproduced this on the Olimex board (both RP2040 and RP2350), Pimoroni Pico DV (RP2040) and Pimoroni VGA (RP2350)

Sorry, I am struggling to reproduce this problem. Does it happen every time, or just occasionally?

It happens every time for me with the build I made from latest.

I've just retested with a Pimoroni Pico dv and a RP2040. I've got Trust USB keyboard attached to the Pico DV via an OTG cable . The Pico Dv is powered via the USB power connector on the main Pico DV board.

With the uf2 currently in the repo https://github.com/fruit-bat/pico-zxspectrum/blob/main/uf2/ZxSpectrumPicoDv_HdmiAudio_640x480x60Hz.uf2 I can select a snapshot from the snapshot menu. It starts to run. I can then press F1 and go back to the snapshot menu. I can move around the snapshot menu when I return to it. i.e. Everything works as expected.

If I do the same with the uf2 I've built from latest, I press F1 to select a snapshot. The snapshot starts to run. If I then press F1 again, the snapshot menu appears for about 0.1 seconds, then the menu exits and the snapshot restarts from the beginning.

I've attached the uf2 that I've built. I wonder if you can reproduce the issue with that? If not then I guess I need to move the repo back to the submit where the uf2s were built and rebuild at that point to see if the issue occurs or not.

ZxSpectrumPicoDv_HdmiAudio_640x480x60Hz.zip

I've just tested with the PiZero uf2s (as they build early in the script) :-)

I rebuilt with the repo at submit Joystick buttons 2 & 4 for menu

git checkout c23159d

This load worked as expected. I could start a snapshot, then re-enter the snapshot menu by pressing F1

I then moved to the next submit Auto-repeat keys and joystick on menu

git checkout 85942536b7ed21500e59b249f0049bd1593ea4db

This demonstrated the behaviour described, i.e. after the first time into the snapshot menu, subsequent entries bounced straight back, with the snapshot restarting.

So it looks like there is something in the Auto-repeat keys and joystick on menu submit that is causing this issue when I build it.

I can attach the uf2s if needed.

Thanks for the detail. I must have done something silly! I will read through the auto-repeat code.

Push a tiny change to pico-dvi-menu. Much appreciated if you can let me know if it makes any difference.

I pulled the changes to pico-dvi-menu, moved pico-zxspectrum back to main and rebuilt, but unfortunately that did not fix the issue

However, that gave me an idea. I commented out line 347 picoWinHidKeyboard.processKeyRepeat(); from main.cpp in hdmi and rebuilt. The issue then did not happen and I could get in and out of the quicksave menu correctly. So it appears that this is something to do with picoWinHidKeyboard.processKeyRepeat()

I did some more debugging. The issue seems to be that cancelRepeat is not called after a snap is loaded. It is also not called via picoWinHidKeyboard.processHidReport when the menu is toggled.
As a hack I added

  if (r) {
    toggleMenu = true;
    if (!showMenu)
        picoWinHidKeyboard.processHidReport(report, prev_report);
    picoRootWin.repaint();
  }

to process_kbd_report in main.cpp and that cancelled the repeat so that I could get back into the menu, but you have a much better idea of the OO structure, so I expect you will know a better place.

Thanks for your patients with this one. I think I have fixed it now, with a variation of the change you made above.

Changes to both pico-zxspectrum and pico-dvi-menu.

I tested today and it looks good. I think this can be closed now 👍

@ikjordan cheers!