-
Notifications
You must be signed in to change notification settings - Fork 662
Fix and enhancements for rpi-retroflag-AdvancedSafeShutdown.py #13789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- shutdown hardware fixed - user configurable LED - user configurable splash
|
PowerEN never should setted to low this will cause in an immediate power cut and you will lose meta-data. There is a good reason that even retroflag (the manufacturer does not use this). |
|
@crcerror Okay, that's not good. I made an assumption based on the analysis of ai (claude, chatgpt and deepseek all came up with this solution after analyzing this script and the official retroflag script). And it was working, so it seemed right to me. But yeah, we don't want to loose metadata. I'll go back to the drawing board. Do you have an idea why the original script doesn't shut off the power? Any thoughts on the other changes? |
|
I do not know why the current script isn't working for you. Can you check in /boot/boot/config.txt how many entries with dto-overlays are set active? The one with gpio-poweroff or gpio-shutdown is not the correct one. |
|
@crcerror So it is working for you? V41? When I have time next week I'll start from scratch and give you the answer. But I was thinking about what you said about PowerEN; I think this script still mitigates that issue with the metadata, because I'm stopping ES before any command. Stopping ES saves the metadata, right? Maybe not pretty, but it works? |
|
Yes quitting ES before will save metadata. Yes dman and me tested the script for v41 with Pi3 (on myself) and Pi4 (by User report) on NesPi+ housings - so the answer if it worked is yes. |
|
Nevertheless I think your additions are nice but the customised part could be made better with an external config file. See, nobody wants to edit a system file and use an overlay for making changes permanent. So in current state I doubt the PR will be accepted. @dmanlfc |
|
@crcerror So not tested with a Nespi 4 Case and RPI 4b? Is there possibly something different with this combo? If not, then it must be something on my end. I'll dig into it next week. And yes, not accepting the PR is completely understandable. Although it seems to be working fine with a Nespi 4 case, the basis of this PR seems to be completely wrong, that this script wasn't working in the first place. First need to diagnose that before adding the enhancement seems to be the best route. The config file was indeed mentioned as a possible enhancement of this script, but I wasn't sure if this was allowed for such a specific script and how I needed to implement this. I know Retroflag script creates a Retroflag folder inside /userdata/. Would that be the preferred way? My thinking was based on reading the wiki https://wiki.batocera.org/splash_boot#changing_emulationstation_s_default_loading_splash where using batocera-save-overlay was already being used to change the ES or boot splash so I didn't think my approach was that different. |
|
I rechecked the discussion that was done and test were performed on a Pi4 and NESPi 4 case. It would be nice if you could check your config file in boot partition for dto overlays |
|
@crcerror Okay, that's really strange... I have written down exactly what I did yesterday. TLDR: No difference in dtoverlay? Retroflag touches the dtbo file, but nothing is changed? Only other difference I have found is the use of the shutdown command. Changing that in rpi-retroflag-AdvancedSafeShutdown makes it work. Used that in my script, removed the PowerEN pin parts and implemented your suggestion of a config.py file. My findings;
So I went ahead and tested the rpi-retroflag-AdvancedSafeShutdown script by changing the Hardware used: Using Batocera Script
Using Official Retroflag Script
Because
So I guess this PR is already outdated haha. Should I close this and open a new one or should I just change the files in my fork? I'll wait with doing anything until we have figured out the |
|
You do not need to worry about So what is correct: So what is off: @FlashyDuck Did all work with v39? This was the last version using rpi.GPIO lib, just to check if nothing is off with your case. But I doubt because you showed me clean tests. |
|
Retroflag_ADV in V39 works (nespi 4 and rpi 4b) Remembered I had an Nespi Plus case laying around... Put in a rpi 3b+
So riddle me this; why does Retroflag_ADV V41 work on the NESPi Plus case and not the NESPi 4 case? The pins being used are the same. So it's either something with the case or the rpi 4. Because you mentioned that V41 was tested with a nespi 4 and rpi 4b, then this excluded the case. The only thing I can think of that it's a firmware thing with rpi 4b? So installed Raspberry Pi OS to check the eeprom... BOOTLOADER: update available VL805_FW: Dedicated VL805 EEPROM Updated. Reflashed V41. No luck. Still not working. So I'm really out of ideas here... |
|
Okay thanks for feedback about v39 You already pointed it out. There is no difference in connecting between these 2 cases..... So you are also right for your outfindings --- the Pi4 behaves completely differently compared to its predecessors. Sooooo.... you confirmed a working v39, then the last conclusion I can made ... There is something wrong in the script Please explain: You say if you change from And if I apply these changes to a Pi3 then -r would reboot the board, and -h would cut power :) It would be nice if you would join discord. From here be can better bisect the issue. |
Can you explain this further? Because it's definitely a Pi 4B issue. Pi 3B+ works in both Nespi 4 and Nespi Plus case - shutdown and power is cut. Fan stops. So no case issue.
Yes, changing it to reboot makes everything work. It starts to reboot but shortly after that cuts power. Like I mentioned in the issue topic, by changing the code to this makes it work: It's basically the exact same thing that the official Retroflag script does. They also only use Tested above code with he Pi 3B+ in a Nespi Plus case and this works exactly the same as the Pi 4B and/or Nespi 4 case.
Are the GPIO pins 'reset' when rebooting? And can they detect the position of the Power button? I'm just guessing here... |
Yes, as I got my Pi4 - I saw that it did not work properly with the cases. So I added the line below to /boot/config.txt and then it worked. This worked till v39 as the change from rpi.GPIO to gpiod was done. So let's say .... 3-4 years it worked without any problem. With the change to gpiod this did not work proper anymore and we switched to the one from retroflag.
I really can't say about it. But let me retry later on Pi3B. But this is a good explaination of things I was facing. So I guess you know how to enter SSH or plain terminal? |
|
I an confirm that reboot + retroflags overlay cuts power on the Pi3. For me there is no difference between So I diassembled the retroflag-overlay. Can you please add following two lines to config.txt? and comment/remove line |
Yeah, the Pi4 really needs the reboot for the Pi4 (or case?) to be able to cut power a few seconds after the reboot. My suggestion is to just go with that fix? It's the same thing Retroflag does, it's the simplest change to the original script and it just seems to work?
Tried it, but doesn't work. After the change and pressing shutdown results in the same issue. No power cut. And after a reboot 'dtoverlay=RetroFlag_pw_io.dtbo' is again written to /boot/config.txt. Tried both RETROFLAG and RETROFLAG_ADV in batocera.conf. I went the same route. Added logging to the script for the Power button press specifically, but that didn't show anything weird. Converting the dtbo files (both the Retroflag one as the one you said you used in V39) to dts and have them checked with several llm's. The only issues they all seem to find right away was with the Retroflag one:
Where I'm left; The way gpiod communicates with Pi4 (EEPROM) is different? But honestly, no clue where to go from here... |
|
@FlashyDuck Yeah I did forget to say ... Where did you read |
|
This weekend your outfindings will be tested and fixed then. It seems that only a few people do have a Pi4 in a NESPi case nowadays. |
Not sure what you meant with the 'next 5 lines' because some of those lines were already commented out. This is what that code block looked like after changing it. Tried it with RETROFLAG_ADV and this seems to break the script
I converted RetroFlag_pw_io.dtbo with WSL to RetroFlag_pw_io.dts. Line 24 says "output-hight;". According to all LLM's this should be "output-high;"? But changing this and converting the dts back to dtbo does not change the behavior.
Great! Look forward to the results. |
|
@FlashyDuck your script-looks odd. The original script does not have any code-lines commented out. It's very likely that therefore you can also adjust the code because the code is broken. The "correct way" to disable the setup to write a new overlay is this |
|
@crcerror Okay, I flashed batocera again and started over to make sure I wasn't working with a previous mistake. Script works again (LED and Reset works), but same result unfortunately for the Power button. Only Batocera shuts down, the Pi/case don't. |
|
@FlashyDuck This was expectable. So we need the reboot commands in or in our config.txt, I'm not sure what you tested if you reflahsed. |
|
Sorry for the confusion; with reflashed I meant I started fresh again and then applied the changes you mentioned in your previous posts. The behavior compared to 'stock' config.txt was the same. The case/Pi didn't turn off after shutdown. Yeah, I think we should just switch to the reboot commands for shutdown. That way you can keep the Retroflag dtbo file and change as little as possible. |
|
@FlashyDuck So we make minimal changes to the default script. But as bonus.... Please use the script attached for own usage. It's a dedicated shutdown script for NESPi4-cases ... it will not work on Pi3. It gives write errors if the backgroundimage is applied. Place the script to REMOVE .log EXTENSION BEFORE USAGE |
|
@crcerror Any info on the write errors with the Pi3? I'll try it out on my Pi3 and Nespi Plus case and see if I encounter the same issue. I'm guessing using it as a Service keeps it safe from Batocera updates? I'll try that out. Would this still work with a seperate config file? Because I had already made some improvements to the script based on your suggestion. Just a how to for anyone that also wants to try it out;
I wasn't sure which folder I should use for the config file, so I added some fallback paths in the script to look for a config. As far as I can tell this is working on a Pi4 and Nespi 4 case. |
|
Yes if you use it as service then you can upgrade safely. It is userdata and will not touched then. With this possibility there is no need for an external file. |
|
@crcerror Apologies for the delay. I couldn't get your userservice script to work. Buttons worked, but the splash wasn't being cleared. Went ahead to try and convert my latest version and oh boy... learned that you can't use "tabs" in bash the hard way. Anyway, I have made some enhancements to the service script. Improved readability for other users, changed the shutdown to also use -r and some other small stuff like only using the dtbo file (easier to copy/paste). I have tested this and it's working correctly on a PI4 with NESPi4 case. NESPI4_CASE_BUTTONS.txt Is this something to be added to system services? Or is there some place where you can share this script for people to find it? And bbout the PI3B.... EDIT: |
PROBLEM
The current script does not do a proper shutdown of the hardware.
FIXED
ENHANCEMENTS
Note:
First time coding - This script was developed with assistance from several AI tools to help with code structure, debugging, and optimization.
Hardware: