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.

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

Spotify fixes #2315

Merged

Conversation

AlvinSchiller
Copy link
Collaborator

@AlvinSchiller AlvinSchiller commented Apr 3, 2024

Like described here mopidy has a new pre-release which features spotify calls via the webui.
This PR adds the needed packages to use this pre-release.

Changes:

  • update packages installed from apt and pip. Install gst-plugin-spotify manually as it is not available via apt.
  • removed unnecessary mopidy user conf
  • fixed services start during installation
  • replaced youtube-dl with yt-dlp, as the ci was failing on bookworm. For unkown reasons the package was missing in the end in the spotify installation path. Works with the new package (also easier installation)
  • fixed background execution of youtube download
  • add mopidy scan to startup script
  • put sudoers entry in seperate files in "/etc/sudoers.d/" instead of manipulating the original sudoers file
  • fixed mopidy sudoers entry python path

@AlvinSchiller AlvinSchiller added enhancement spotify edition installation legacy_v2 Issues, discussions and PRs related to Version 2.x labels Apr 3, 2024
@AlvinSchiller AlvinSchiller added this to the v2.7.0 milestone Apr 3, 2024
@AlvinSchiller AlvinSchiller requested a review from s-martin April 3, 2024 01:08
@AlvinSchiller AlvinSchiller self-assigned this Apr 3, 2024
@coveralls
Copy link

coveralls commented Apr 3, 2024

Pull Request Test Coverage Report for Build 8666808786

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 78.435%

Totals Coverage Status
Change from base Build 8622057992: 0.0%
Covered Lines: 451
Relevant Lines: 575

💛 - Coveralls

@AlvinSchiller
Copy link
Collaborator Author

AlvinSchiller commented Apr 3, 2024

Additionally:
The mopidy packages are not compatible with buster (python >=3.9 is needed), so the ci fails here.
As buster is running out of support by June 30th, 2024, i would suggest to remove buster support with the next release.
The needed changes will be introduced with another PR.

@s-martin
Copy link
Collaborator

s-martin commented Apr 3, 2024

#2063 would then not be needed anymore.

@AlvinSchiller
Copy link
Collaborator Author

#2063 would then not be needed anymore.

Yes, that can be closed.

Copy link
Collaborator

@Groovylein Groovylein left a comment

Choose a reason for hiding this comment

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

I did not test the installation (no test system available) but from the review point of view, it looks good :)

@s-martin
Copy link
Collaborator

s-martin commented Apr 4, 2024

#1837 could be closed as well then.

@kingosticks
Copy link

Additionally: The mopidy packages are not compatible with buster (python >=3.9 is needed), so the ci fails here. As buster is running out of support by June 30th, 2024, i would suggest to remove buster support with the next release. The needed changes will be introduced with another PR.

Just as an FYI, the next Mopidy major release is due shortly after Ubuntu 24.04, later this month. As part of that, we'll also drop support for Python 3.10 (and therefore also Debian Bullseye).

@AlvinSchiller
Copy link
Collaborator Author

Additionally: The mopidy packages are not compatible with buster (python >=3.9 is needed), so the ci fails here. As buster is running out of support by June 30th, 2024, i would suggest to remove buster support with the next release. The needed changes will be introduced with another PR.

Just as an FYI, the next Mopidy major release is due shortly after Ubuntu 24.04, later this month. As part of that, we'll also drop support for Python 3.10 (and therefore also Debian Bullseye).

I read about this. We need to stay on this version for now, as bookworm has incompatible changes for the GPIO libs that are not resolved yet. So we can't currently move from bullseye.

@s-martin
Copy link
Collaborator

Would that work?
https://pypi.org/project/Mopidy-WaitForInternet/

@kingosticks
Copy link

Looks like it would. Don't think I've ever seen that one. Of course won't help if you have any other services starting that need a working internet but maybe everyone else does it better than Mopidy.

@AlvinSchiller
Copy link
Collaborator Author

@jonaskor I saw that you used the "autohotspot" features. I had a deeper look and identified a problem which might intensified your "login" problem. I made some changes and from my view its good to use now.
Would be awesome if you could test your case again.

@AlvinSchiller
Copy link
Collaborator Author

AlvinSchiller commented Apr 12, 2024

Would that work?
https://pypi.org/project/Mopidy-WaitForInternet/

I didn't use this package for now, as my tests where successful and it might not fit every case or is not even wanted. But will add it to the Spotify wiki entry.

@kingosticks
Copy link

It feels like a hack and takes advantage of Mopidy internals that could change. To me, a working network dependency is exactly the sort of thing that should be handled by systemd.

@kingosticks
Copy link

@AlvinSchiller you might also want to escape special characters in the various wifi passphrases too?

@jonaskor
Copy link

@jonaskor I saw that you used the "autohotspot" features. I had a deeper look and identified a problem which might intensified your "login" problem. I made some changes and from my view its good to use now. Would be awesome if you could test your case again.

This fixed things, awesome! Now the box connects to wifi within 20sec! Mopidy still needs another 3 to 4 minutes to connect to the server, but then everything works, no need for rescanning library or restarting mopidy anymore. How cool is that! Thank you!

Since all the brain power already seems to be gathered here: any suggestions on how to speed up the connection time to the mopidy server? ;)

@kingosticks
Copy link

kingosticks commented Apr 12, 2024

Mopidy still needs another 3 to 4 minutes to connect to the server,

Do you mean that Mopidy takes a long time from starting to becoming ready to use? That'll be the time it spends refreshing your Spotify playlists, I noticed in your log you had quite a lot (and immediately felt your pain).

The very latest (unreleased) version of Mopidy-Spotify is much improved in that respect and the refresh time is now hidden. I wasn't planning to do another pre-release that would include the fix since it'll be part of the upcoming Mopidy 4.0 release later this month. However, I've since learned/realised that this project (and maybe others) won't immediately be able to upgrade to Mopidy 4 because they need to remain on Debian Bullseye for reasons outside their control.

So long story short, I will see if I can easily backport the playlist refresh improvement since it's such a huge quality of life improvement.

@jonaskor
Copy link

jonaskor commented Apr 12, 2024

So long story short, I will see if I can easily backport the playlist refresh improvement since it's such a huge quality of life improvement.

That would be so great!! Thank you for all your work, guys!

EDIT: For now I just set allow_playlists = false in the config file /etc/mopidy/mopidy.conf which fixes things for me for now! 🥳🎉

@kingosticks
Copy link

I published another Mopidy-Spotify pre-release including the playlist refreshing speedup.

sudo python3 -m pip install --break-system-packages Mopidy-Spotify==5.0.0a2

@AlvinSchiller this is the better version to use. It makes a considerable difference to startup time.

@AlvinSchiller
Copy link
Collaborator Author

I published another Mopidy-Spotify pre-release including the playlist refreshing speedup.

sudo python3 -m pip install --break-system-packages Mopidy-Spotify==5.0.0a2

@AlvinSchiller this is the better version to use. It makes a considerable difference to startup time.

That was fast 🙂.
I will make the change to use it, asap 🙂👍

@AlvinSchiller
Copy link
Collaborator Author

Version updated to 5.0.0a2.
@jonaskor if you by chance could verify this a last time, i think we would be ready to go :)

@AlvinSchiller AlvinSchiller linked an issue Apr 12, 2024 that may be closed by this pull request
@AlvinSchiller AlvinSchiller merged commit d76a7ef into MiczFlor:develop Apr 14, 2024
27 checks passed
@AlvinSchiller AlvinSchiller deleted the feature/mopidy-spotify-fixes branch April 14, 2024 18:50
@jonaskor
Copy link

Version updated to 5.0.0a2. @jonaskor if you by chance could verify this a last time, i think we would be ready to go :)

Just installed the develop branch with the following command (in case anyone else wants to install it soon: cd; rm install-jukebox.sh; wget https://raw.githubusercontent.com/MiczFlor/RPi-Jukebox-RFID/develop/scripts/installscripts/install-jukebox.sh; chmod +x install-jukebox.sh; GIT_BRANCH=develop bash ./install-jukebox.sh ).

And it works absolutely amazing! Thank you so much! Start up time of around 30-40 sec (on Raspberry Pi 4, Debian 32bit, Bullseye) until I am able to playback spotify songs and playlists! Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement installation legacy_v2 Issues, discussions and PRs related to Version 2.x spotify edition
Projects
None yet
6 participants