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

DietPi-Software | Add spotifyd #4713

Merged
merged 7 commits into from
Sep 17, 2021
Merged

DietPi-Software | Add spotifyd #4713

merged 7 commits into from
Sep 17, 2021

Conversation

MichaIng
Copy link
Owner

@MichaIng MichaIng commented Sep 7, 2021

@ressu, I just had a look through forks and found your good work on implementing spotifyd. Am I allowed to merge it? 😃

@Joulinar
Copy link
Collaborator

Joulinar commented Sep 7, 2021

@MichaIng why not merging installation and configuration steps? I thought this would be our goal on long term.

@MichaIng
Copy link
Owner Author

MichaIng commented Sep 7, 2021

Yes, not done by me, I just found the brand and opened the PR to not forget about it 😄. The wiki needs to be updated: https://github.com/MichaIng/DietPi/wiki/How-to-add-a-new-software-title

@ressu
Copy link

ressu commented Sep 7, 2021

Hah! Well done finding it.. Yes, you are free to merge. I meant to create a doc pull request for it at some point, but life got in the way. The installer could use a bit of work. I only tested it on a RPi, but there are other binaries there as well.

@MichaIng
Copy link
Owner Author

MichaIng commented Sep 7, 2021

Many thanks! Indeed, currently its only armhf, but shouldn't be hard to add an arch dependent variable to the mawk pattern where pre-compiled binaries are provided.

@ressu
Copy link

ressu commented Sep 7, 2021

Is there an easy way to see what the various ARCH variables look like on other platforms? One of the reasons why I didn't spend too much time fixing the mawk pattern was that I wasn't able to easily find references for those...

@MichaIng
Copy link
Owner Author

MichaIng commented Sep 7, 2021

It's only four:

  • ARMv6: $G_HW_ARCH == 1
  • ARMv7: $G_HW_ARCH == 2
  • ARMv8: $G_HW_ARCH == 3
  • x86_64: $G_HW_ARCH == 10

The filename changes accordingly:

  • ARMv6: spotifyd-linux-armv6-slim.tar.gz (slim variant only, are the others too heavy for RPi1/Zero? 🤔)
  • ARMv6: spotifyd-linux-armhf-full.tar.gz
  • ARMv8: Not provided, so you ruled that out correct already
  • x86_64: Öhm, that will be the one without arch in the filename: spotifyd-linux-full.tar.gz

This change adds support to all supported platforms.
@ressu
Copy link

ressu commented Sep 11, 2021

There we go, added the missing patterns. I used the slim package for older RPi's, but to be honest, I'm not fully certain about the differences so I decided to play it safe.

@MichaIng
Copy link
Owner Author

Many thanks, looks good. I'll not be able to test for first beta tonight, but maybe, as it is a very simple clean implementation, on a future beta iteration next week.

+ DietPi-Software | Spotifyd: Change software ID to 199
+ DietPi-Software | Spotifyd: Change install dir to /opt/spotifyd and config/cache dir to /mnt/dietpi_userdata/spotifyd
+ DietPi-Software | Spotifyd: Pre-create a config template, if not yet present
+ DietPi-Software | Spotifyd: Do not add "spotifyd" user to "dietpi" group
+ DietPi-Software | Spotifyd: Merge install and config code blocks
+ DietPi-Software | Spotify Connect Web: Merge install and config code blocks
+ DietPi-Software | Raspotify: Remove APT key as well on uninstall
@MichaIng
Copy link
Owner Author

@ressu
May you grant me commit permission? I applied a few changes but are currently not permitted to push them.

@MichaIng MichaIng added this to the v7.6 milestone Sep 15, 2021
@ressu
Copy link

ressu commented Sep 15, 2021

Done.. I couldn't find a way to retroactively make a PR committable, so I simply invited you to my fork of the repo.

+ DietPi-Software | Spotifyd: Add default configuration
@MichaIng
Copy link
Owner Author

MichaIng commented Sep 15, 2021

Many thanks, I though about the Allow edits by maintainers checkbox, but probably in this circumstance where I opened the PR this wasn't even visible in your case.

So I aligned it a bit with our other install options:

  • Binary stored to /opt/spotifyd/spotifyd. Downside is that is not in PATH, hence needs to be called with full path from console. But it is meant to run as system service anyway, so I think it's fine.
  • Cache stored to /mnt/dietpi_userdata/spotifyd/cache and the same directory used for a default TOML configuration file, so users can better see what can be done and adjust settings.
  • Spotifyd does not need to be in the dietpi group as long as it does not write or at least read local media files, so let's keep permissions to a minimum.
  • I also merged install and config code blocks, which we do not step by step. This separation has not really any purpose anymore but makes maintenance more difficult when one always need to switch between those two blocks in an increasingly huge file.
  • I had a look into Spotify Connect Web and Raspotify code for comparison and did little changes there as well, but this can be ignored.

What I'm unsure about is what to do about the Spotify account credentials. We could do some input boxes, but there is quite a bunch of possibilities, password, password command and key, so this may be complicated. I'll do an install test now to see how it behaves without credentials given at first.

... with no credentials, it starts up. I have really no idea about Spotify. In this case one authenticates via client, or how does it work?

+ DietPi-Software | Spotifyd: Comment account user and pass by default and add full path to help command in comments
@ressu
Copy link

ressu commented Sep 15, 2021

Normally you would attach this to your spotify account via spotify connect. And what is different from raspotify (for example) is that spotifyd will cache the credentials in cache directory when the connection is first made with a spotify client (mobile or native client) and this will make spotifyd visible in web too.

So unlike with raspotify where adding credentials is recommended for some operational modes, spotifyd goes the extra mile and attempts to work out of the box, even without credentials. In my installation I'm not explicitly defining credentials anywhere and things just work. So did raspotify, but if it disconnected from spotify hotspot the connection would have had to been recreated with a native client.

As for the file locations, those make sense. I was mostly mimicking other installers when I was writing the code and I was unable to find guidance on how DietPi prefers the paths to be created.

@MichaIng
Copy link
Owner Author

Awesome, then I think we're good to go. I'm still not 100% sure in this case whether the executable location is really good there. The official docs likely state to call the spotifyd command without path, so does the config template. But it's a quite typical issue on the other hand, that users call service executables from command line via login user or root user and then wonder why they cannot access data from the service but instead get a new config/data instance created for the login user. It's kinda clashing setups to either run something as a system service, with a dedicated system user, or as login user or systemd --user mode.

+ DietPi-Software | Spotifyd: Add some meta information
@MichaIng
Copy link
Owner Author

MichaIng commented Sep 16, 2021

Ready from my end. @ressu anything missing?

Btw would you find time to write a docs section? https://dietpi.com/docs/software/media/#raspotify

@ressu
Copy link

ressu commented Sep 17, 2021

Looks good from my side. I'll try and get to documentation during the weekend.

@MichaIng
Copy link
Owner Author

As we do the release tomorrow, I'll start with a rough template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants