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

Use Spotifyd instead of vollibrespot to get latest librespot changes #366

Merged
merged 16 commits into from
Nov 3, 2022

Conversation

klay2000
Copy link
Contributor

No description provided.

@klay2000 klay2000 requested a review from linknum23 August 17, 2022 18:29
Copy link
Contributor

@linknum23 linknum23 left a comment

Choose a reason for hiding this comment

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

Looks good! Lets test it.
I can help you add the installer for this in scripts/configure.py when we get there.

@linknum23
Copy link
Contributor

Note most of those changes are only tangentally related to your PR just leftover things to clean up

@linknum23 linknum23 changed the title Spotifyd Use Spotifyd instead of vollibrespot to get latest librespot changes Aug 17, 2022
Copy link
Collaborator

@Lohrer Lohrer left a comment

Choose a reason for hiding this comment

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

Is there any way to download or build the spotifyd binary at install time, instead of adding to the repo?

data = data.replace('5030', f'{self.metaport}')
data = data.replace('device_name_in_spotify_connect', f'{self.name}')
data = data.replace("alsa_audio_device", f"ch{src}")
data = data.replace('1234', f'{self.connect_port}')
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be awesome to expose bitrate and autoplay to the user if possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we support higher bit rates?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do this in the future when we are able to better characterize utilization. We still need a 4th Spotify account!

@linknum23
Copy link
Contributor

Lets test this on Jason's house and see how we like it now that this has gotten past the multiple account crash bug.

@linknum23 linknum23 linked an issue Sep 21, 2022 that may be closed by this pull request
5 tasks
@linknum23 linknum23 removed a link to an issue Sep 21, 2022
5 tasks
@linknum23
Copy link
Contributor

This has the potential to fix #247, #248, #214 and supersedes #318

@linknum23 linknum23 linked an issue Sep 21, 2022 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2022

Codecov Report

Merging #366 (9e90f70) into develop (6dd7492) will decrease coverage by 1.82%.
The diff coverage is 33.33%.

@@             Coverage Diff             @@
##           develop     #366      +/-   ##
===========================================
- Coverage    56.81%   54.98%   -1.83%     
===========================================
  Files           12       13       +1     
  Lines         2920     3039     +119     
===========================================
+ Hits          1659     1671      +12     
- Misses        1261     1368     +107     
Flag Coverage Δ
unittests 54.98% <33.33%> (-1.83%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
amplipi/mpris.py 32.14% <32.14%> (ø)
amplipi/streams.py 42.24% <36.00%> (+0.62%) ⬆️
amplipi/app.py 77.88% <0.00%> (-7.62%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@linknum23
Copy link
Contributor

This needs to be rebased on develop to get the hotfix for uvicorn

@linknum23
Copy link
Contributor

Is this ready without volume adjustment?

@klay2000
Copy link
Contributor Author

I need to change the cache directory, keep forgetting. I'll do it Monday then rebase and we can merge.

@linknum23 linknum23 marked this pull request as ready for review November 3, 2022 14:13
@linknum23 linknum23 merged commit 0719ead into develop Nov 3, 2022
@linknum23 linknum23 deleted the spotifyd branch November 3, 2022 17:15
Lohrer pushed a commit that referenced this pull request Nov 16, 2022
Use Spotifyd instead of vollibrespot to get latest librespot changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade vollibrespot to the latest librespot changes
4 participants