-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Update snesdev.sh #4019
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
Update snesdev.sh #4019
Conversation
Point to an updated driver repository that uses the WiringPi library for compatibility with all Respberry Pi Versions.
@MonsterJoysticks thank you for the PR. I noticed you opened also a PR on the upstream repository, I'd rather wait for it to be concluded before accepting this PR as-is. Florian might take some time to respond, so we'll have to wait a bit and see which repository can be used in this scriptmodule. I've taken a look at the new version and even tried to install it, but I hit a couple of errors on the way, on my Pi4 system:
Which system(s) did you use to test the new driver installation/operation ? EDIT: I also tested the installation on a Pi3 with RaspiOS Buster. While the 2 previous errors are no longer present, the build step fails with:
|
Thanks for looking into it. Im using the 32bit versions of the Raspberry Pi OS and WiringPi on a Pi 5. I had the same issue with the confuse library but i thought i had resolved that by adding the "cd libs/confuse-2.7 && make" line to the Makefile https://github.com/MonsterJoysticks/SNESDev-RPi-Wiring-Pi/blob/master/Makefile you'll have to forgive my lack of knowledge on these scripts but I’ll see about adding some kind of statement to detect the architecture of the system and install the corresponding library. Let me sort these issues out and get back to you. |
RetroPie has a |
Thanks, i have already pushed an update to my repository and i believe it should work on both versions. I ended up using the ARCH = $(shell getconf LONG_BIT) to determine 64 or 32 bit OS install, it then gets the version 3.2 of the wiringpi library and installs that. I also added an install for the libconfuse-dev package using apt-get as that was also arch dependant. |
sorry i think i spoke too soon, i have just made another change to the repo i'll let you know when its all ready. |
Ok i think that should all work correctly now. Apologies. |
No need for apologies, there's nothing broken and regardless - this is your own free time you're donating. I'll take a look and test on my systems, thanks you for changes. |
@MonsterJoysticks I re-tested the installation from your repository and
Leaving aside the WirinPi installation, one improvement for the script would be to add specifically the
I'll if WirinPi can be independently compiled and included so we don't have to rely on the |
Thats annoying that you get those errors, on the github page they make a point of it being compatible with all verisons of the Pi so it may just be the operating system itself or maybe need compiling on the older OS. Lets see if the PR is considered/accepted and in the meantime ill look at adding the dependancy to the script. |
Sure, but that doesn't mean the |
I have adjusted the Makefile on my fork of the SNESDev Driver so that it clones into the WiringPi git and then runs the build process. I think this should improve compatibility as mentioned, im not sure if this is considered the "correct" way to do this but it does seem to work on my tests. |
I think it's best to leave the WirinPi installation alone (and mention it as a depedency, like |
i have rolled back the building and installing of |
@MonsterJoysticks I have a preliminary set of modifications at master...cmitu:RetroPie-Setup:snesdev-update In order to be able to compile a local static library (for WiringPi), there's a minor modification that needs to be added to the diff --git a/src/Makefile b/src/Makefile
index e2ee013..19bb070 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -1,8 +1,8 @@
CC=gcc
SRCDIR=.
BUILDDIR=../build
-CFLAGS=-Wall -O3
-LIBS=-lwiringPi -lpthread -lrt -lconfuse
+CFLAGS+=-Wall -O3
+LIBS=-lwiringPi -lpthread -lrt -lconfuse $(LDFLAGS)
TARGET := SNESDev
SRCEXT := c I say it's preliminary since I also want to replace the service part with a |
Ok, added the |
I have just tested it on the Pi 5 and it worked great after i removed the |
@MonsterJoysticks oh yes, I forgot to remove the Pi5 restriction, I updated my branch. I don't see the modification to |
Apologies i must have forgot to push the change up. Thats all sorted out now. |
@MonsterJoysticks thank you for the changes and modifications added. |
Point to an updated driver repository that uses the WiringPi library for compatibility with all Respberry Pi Versions.