Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implemented fixes for RPi recognition and new spotify GST plugin compile #2064

Closed
wants to merge 1 commit into from
Closed

Implemented fixes for RPi recognition and new spotify GST plugin compile #2064

wants to merge 1 commit into from

Conversation

The-Judge
Copy link

This patch (in addition to #2063 ; since the Raspberry Pi 3B's 1 GB RAM is much too less for Rust and GST plugin compile) should apply all the fixes introduced by #1815 report, plus my own experiments on mentioned RPi 3B.

@The-Judge The-Judge mentioned this pull request Sep 6, 2023
Closed
@s-martin
Copy link
Collaborator

Is there a possibility to build the gst plugin and host it somewhere to download it by the script?

Personally I’m a little bit reluctant to have an installation routine where you have to install otherwise unrelated dev tools and run the build process for hours.

But if that’s the only way we could do it this way.

@s-martin s-martin linked an issue Sep 11, 2023 that may be closed by this pull request
🐛 | Spotify does not work anymore #1815
Closed
@The-Judge
Copy link
Author

Is there a possibility to build the gst plugin and host it somewhere to download it by the script?

Personally I’m a little bit reluctant to have an installation routine where you have to install otherwise unrelated dev tools and run the build process for hours.

But if that’s the only way we could do it this way.

I also thought about that. But there are some issues I see with this:

  1. I am too unfamiliar with cross-compiling and hardware architectures to know what needs to be build, with which hardware/software combinations and so on. Will the plugin which got compiled on my Raspberry Pi 3B running a 64 Bit Bullseye work on a Raspberry Pi 2/3/4/Zero/... with 32 and 64 Bits as well? Or are there alternate builds needed?
  2. Closely related to 1: I do not have all these hardwares to build it for everyone; and as already said: I am too unfamiliar with cross-compiling to do that without the hardwares. I will come back to this in a sec.
  3. What's about licenses? Is it legally allowed to distribute binaries build from that project? I am not experienced in legal topics enough to know this.

Back to point 2: I might have an idea for that:
What if we have an option (which one can opt-out from) to check if a binary for that specific Raspberry + OS version + CPU Architecture is already available at some online - space (like GitHub files for example) and if not: This build is made as my script currently is doing always. If that succeeds and one has not opt-out, that is uploaded to that GitLab pool ready for pickup by the next one running with that combination?

@The-Judge
Copy link
Author

What is missing? How to go on from here? Is anything needed from my side?

@The-Judge
Copy link
Author

Everything seems answered and users report in #1815 that it's working for them.
What else is required? Or can we merge?

@The-Judge
Copy link
Author

This is done for 2 weeks now - what are we waiting for?

@s-martin
Copy link
Collaborator

While I understand that waiting on reactions may be frustrating please be aware that all people here contribute to this project in their spare time.

Life gets in the way pretty often for most people. People answer here when they feel to do so, but have no obligation whatsoever.

For me personally:
While I may be commenting on small stuff this does not mean automatically I have the time to look at bigger things (especially PRs) right now.

If you are interested in participating as a contributer, please meet us at the chat. New hands on deck are always welcome. :-)

@s-martin
Copy link
Collaborator

The test for installation (on a docker image) does not work: https://github.com/MiczFlor/RPi-Jukebox-RFID/actions/runs/6101951055/job/16692967788?pr=2064

The script in the test checks for installed packages, looks like mopidy-spotify is not installed as a package, pyhton3-spotify check could maybe removed or replaced with a correct package.

Starting from line 2797:

  ERROR: mopidy-spotify is not installed
  libspotify12 is installed
  python3-cffi is installed
  python3-ply is installed
  python3-pycparser is installed
  ERROR: python3-spotify is not installed

If you need some pointers for help, chat us up here or on our chat.

@s-martin s-martin mentioned this pull request Nov 8, 2023
Merged
3 tasks
@s-martin
Copy link
Collaborator

s-martin commented Nov 8, 2023

The test for installation (on a docker image) does not work: https://github.com/MiczFlor/RPi-Jukebox-RFID/actions/runs/6101951055/job/16692967788?pr=2064

The script in the test checks for installed packages, looks like mopidy-spotify is not installed as a package, pyhton3-spotify check could maybe removed or replaced with a correct package.

Starting from line 2797:

  ERROR: mopidy-spotify is not installed
  libspotify12 is installed
  python3-cffi is installed
  python3-ply is installed
  python3-pycparser is installed
  ERROR: python3-spotify is not installed

If you need some pointers for help, chat us up here or on our chat.

@The-Judge, as soon as the tests are fixed and the conflicts are solved we could merge this PR and Spotify would work again.

@s-martin s-martin added this to the 2.5 milestone Nov 8, 2023
@s-martin s-martin mentioned this pull request Nov 8, 2023
Closed
AlvinSchiller
AlvinSchiller reviewed Nov 9, 2023
View reviewed changes
scripts/installscripts/buster-install-default.sh
@@ -798,7 +803,7 @@ install_main() {
${apt_get} upgrade

# some packages are only available on raspberry pi's but not on test docker containers running on x86_64 machines
if [[ $(uname -m) =~ ^armv.+$ ]]; then
if grep -E '^Model\s' /proc/cpuinfo | grep -q "Raspberry Pi"; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be changes in the test_installation script aswell

scripts/installscripts/buster-install-default.sh
@@ -834,23 +835,46 @@ install_main() {
echo "${VERSION_NO} - ${COMMIT_NO} - ${USED_BRANCH}" > ${jukebox_dir}/settings/version
chmod 777 ${jukebox_dir}/settings/version

sudo mkdir -p /etc/apt/keyrings
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplication with line 800. was a move intended?

scripts/installscripts/buster-install-default.sh Show resolved Hide resolved
@s-martin s-martin mentioned this pull request Nov 14, 2023
Merged
@s-martin s-martin added the legacy_v2 Issues, discussions and PRs related to Version 2.x label Dec 24, 2023
@s-martin s-martin removed this from the 2.5 milestone Dec 24, 2023
@s-martin
Copy link
Collaborator

s-martin commented Mar 2, 2024

Prebuilds of GST plug-in are now available, see #1815 (comment) and https://github.com/kingosticks/gst-plugins-rs-build/releases/latest

mopidy-spotify has also been updated: https://github.com/mopidy/mopidy-spotify/releases/tag/v5.0.0a1

Is this PR necessary anymore?

@s-martin s-martin mentioned this pull request Mar 2, 2024
Closed
@The-Judge
Copy link
Author

No, not anymore. Successfully delayed.

@The-Judge The-Judge closed this Mar 6, 2024
@s-martin s-martin removed a link to an issue Apr 14, 2024
🐛 | Spotify does not work anymore #1815
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlvinSchiller AlvinSchiller AlvinSchiller left review comments

@s-martin s-martin s-martin left review comments

Assignees
No one assigned
Labels
bug installation legacy_v2 Issues, discussions and PRs related to Version 2.x spotify edition
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@The-Judge @s-martin @AlvinSchiller