fruit-bat / pico-zxspectrum

ZX Spectrum for Raspberry Pico Pi RP2040
484 stars 53 forks source link

It hangs when in the menu you try to refresh Snaps or Tapes without a micro SD inserted #90

Open adamenkov opened 1 year ago

ArnoldUK commented 1 year ago

Put an sd card in or dont press refresh when a card is not inserted ?

adamenkov commented 1 year ago

That would be too easy. :)

A message like "Insert micro SD card" would suffice for me.

ArnoldUK commented 1 year ago

How about a post-it note with "don't forget the sd-card" stuck to your device. Now I know I sound sarcastic but asking for such a pointless feature is asking for a pointless reply. There are so many things that are required that would throw errors. For example, not plugging in the power would cause the emulator to not boot, or unplugging the video cable would show a blank screen :)

Although, I think disabling the full menu system if an sd-card is not inserted would for sure prevent any errors or lockups.

adamenkov commented 1 year ago

That’s funny :) but please bear with me for a minute. I’m not trying to find work for you. The issue is serious enough for me that I made an Issue about it. My situation is that I have a PICOZX, for which I haven’t found a compatible external keyboard yet. This means that if I want to have more than one snap (as a way to save a collection of BASIC programs, for example), I need to rename the default snap. At the moment, when renaming files, I can only add characters to snap file names (to recall, I don’t have an external keyboard), so to do proper renaming I need to take the micro SD out, plug it in another computer, do the renaming, maybe move some snaps to a different directory. It is possible that I forget to plug the micro SD back in the PICOZX, and when I do, the next quick save attempt halts everything. I can’t see if the micro SD is in, the slot is below the keyboard.

I’m just asking to make the thing a little bit more friendly. It’s not a high-priority request, more like a nice-to-have feature.

ArnoldUK commented 1 year ago

Well, the file rename without an external keyboard requires more work and yes would solve your problem. But, I would not like to have a message pop up when a card is not inserted as I use the emulator without an sd card in some of my projects and the constant message popping up would annoy me. I get your point though.

adamenkov commented 1 year ago

No need for a popping up. For example, instead of the list of files, it could just show "(no micro SD)". The hang only happens when I press 5 (refresh) without a micro SD. So the message could only show up in this situation.

ArnoldUK commented 1 year ago

Well, the emulator did not start if a card was not inserted but that was fixed. I still think the emulator menu should be disabled if a card is not inserted. Would be much less prone to errors when the emulator is used without a card.

adamenkov commented 1 year ago

Just disable the menu? Half measures! How about disable everything? Electric shock as penalty for forgetfulness? Self-destruct? :)

Seriously, the menu is still useful to adjust the volume or reset in a different (48K/128K) mode, even without the micro SD. All I want is just that my PICOZX doesn't hang. If there's a short message "(no micro SD)", that would be just super awesome. :)

ArnoldUK commented 1 year ago

I'll go with the self-destruct, gives the user a second chance :)

Oh, yeah I forgot about the 48K/128K reset mode, that is needed. There is only so far a programmer can protect the end user from their sillyness, not pointing fingers :) But let's that a message is added when the card isn't detected, then more checks will be required to check the card is correctly formatted or not corrupted. That's why I mentioned to disable the menu system, or at least all card required menu items.

adamenkov commented 1 year ago

Maybe just a "(can't read micro SD)"? :) - to catch all possible scenarios: no micro SD, micro SD not formatted properly etc.

It's really a good idea to be friendly to the user when possible. One day it might be you! :)

fruit-bat commented 1 year ago

It hangs less now v0.35. There still seems to be an issue if you refresh repeatedly.

ArnoldUK commented 1 year ago

Sure, but only a couple of programmers contribute to this repo and don't get much of a reward from the users so I guess focus is on more making it stable. Hey, I use Windows so I'm the butt end of non user friendly ui's and unwanted intrusions.

fruit-bat commented 1 year ago

The UI needs to operate in a tiny amount of memory (RAM). Just getting a list of files to work with is challenging. Hence all the scanning folders behaviour.

fruit-bat commented 1 year ago

Is it any more stable now? For some reason repeatedly attempting to open a non existent card causes problems for me.

adamenkov commented 1 year ago

The UI needs to operate in a tiny amount of memory (RAM).

Ah ok, I see.

ArnoldUK commented 1 year ago

Is it any more stable now? For some reason repeatedly attempting to open a non existent card causes problems for me.

If you can look at the code below in ZXSpectrumMenu.cpp where I have commented out a line which attempts mount the card when one isn't mounted each time a menu item is added. I had issues until I commented out the lines if (!sd) sd = _sdCard->mount();

  addChild(&_devices, false);
  _devices.onPaint([=](PicoPen *pen) {
    bool sd = false;
    bool jl = _zxSpectrum->joystick()->isConnectedL();
    bool jr = _zxSpectrum->joystick()->isConnectedR();
    bool k1 = _zxSpectrum->keyboard1() && _zxSpectrum->keyboard1()->isMounted();
    bool k2 = _zxSpectrum->keyboard2() && _zxSpectrum->keyboard2()->isMounted();
    sd = _sdCard->mounted();
    //if (!sd) sd = _sdCard->mount();
    pen->printAtF(0, 0, false, "SD Card: %s", (sd ? "Mounted" : "Not Inserted"));
    pen->printAtF(0, 1, false, "USB Port: Joystick%s %s%s%s, Keyboard%s %s%s%s",
       (jl == jr ? "s" : ""),
       (jl ? "L" : ""), (!jl && !jr ? "0" : (jl & jr ? "&" : "")), (jr ? "R" : ""),
       (k1 == k2 ? "s" : ""),
       (k1 ? "1" : ""), (!k1 && !k2 ? "0" : (k1 & k2 ? "&" : "")), (k2 ? "2" : "")
       );
    _devices.repaint();
  });

bool ZxSpectrumMenu::checkFileExists(const char *file) {
  if (!_sdCard->mounted()) return false;
  //if (!_sdCard->mount()) return false;
  FILINFO fno;
  FRESULT fr = f_stat(file, &fno);
  return fr == FR_OK;
}
fruit-bat commented 1 year ago

Thanks @ArnoldUK, this matches what I have found. The repeated attempts to mount the SD card are there to try and cope with a card being removed or inserted (in a limited way). I think the issue lies within the _sdCard->mount() call, which seems to result in memory corruption. Not figured out why so far.

ArnoldUK commented 1 year ago

Yes, I agree and I struggled to pin point the exact code so ended up commenting out all calls to detect or mount the card. Sorry, I've not submitted any pull requests but I'm struggling with github. I will try my best to upload my code addditions and updates to my github page. I'm working on another project at the moment so don't expect anything quick.

Chandler-Kluser commented 1 year ago

I am facing issues of mounting the SD card, too.

It is used to crash, but now I am booting without it connected, so it has been solved.

But now it scans the SD card but do not find anything neither creates any directory inside it, can someone help me?

Thanks in advance!

ArnoldUK commented 1 year ago

I've found If you have too many files in the tapes or snapshots folders then ths can cause the file caching to take some time to rebuild or often freeze. Also some long filenames with odd special characters can cause problems. I suggest adding around 10 tape/snapshot files and try again.

Chandler-Kluser commented 1 year ago

I have placed only one .z80 snapshot and it didn't work

ArnoldUK commented 1 year ago

Try another smaller SD-card and make sure to format as FAT not FAT32. The firmware works fine as I'm using it myself with and without an SD-card.

Chandler-Kluser commented 1 year ago

I have formatted the card into 32MB FAT16 partition (with the directory tree in readme) and it did not work.

I believe it is something with these adapters I am using: IMG_20230828_225525

IMG_20230828_225531

ArnoldUK commented 1 year ago

They should work fine if you have the pins connected correctly. Are you feeding the adapter 5V and not 3.3V as those adapters already have a 3.3V regulator. The SD-card connector pins don't even look soldered from the photo.

Also if using jumper wires make sure they have good conductivity (0.2ohms or less) otherwise they can cause a voltage drop and poor connections.

Chandler-Kluser commented 1 year ago

you are right, these modules have 5V lines + level shifters and I did not observe it.

But now I wired VCC into 5V directly and it did not work either, can I replace the module for an adapter like this one?

image

ArnoldUK commented 1 year ago

Yes, but look at the schematics of all the circuits to check how all the pins connect. A few simple dropper resistors also work fine to level shift the voltage.

Not sure why your module won't work at 5V but possible you may have damaged it or something else. You should read the datasheets when handling electronic devices, they are very easily damaged.

Chandler-Kluser commented 1 year ago

it worked with a cheap SD to microSD adapter!

IMG_20230830_163345

IMG_20230830_163308

ArnoldUK commented 1 year ago

Yeah, JetPac Excellent! I see you have used directly soldered wires. That's a much more reliable setup than jumper wires, I've often wasted hours of problem solving because of cheap poorly connected jumper wires.