pimoroni / pirate-audio

Examples and documentation for the Pirate Audio range of Raspberry Pi add-ons
MIT License
247 stars 48 forks source link

Mopidy config may contain wrong settings #1

Closed kingosticks closed 4 years ago

kingosticks commented 4 years ago

Your install procedures suggests running:

mopidy config | sudo tee /etc/mopidy/mopidy.conf

but this isn't ideal for two reasons:

  1. The default config values for some settings depend on the user running Mopidy. i.e. running mopidy config gives different settings compared to running sudo mopidyctl config. Below shows the differences for the core section but other sections (e.g. logging, m3u, local) may be different also.
    
    $ mopidy config
    [core]
    cache_dir = $XDG_CACHE_DIR/mopidy
    config_dir = $XDG_CONFIG_DIR/mopidy
    data_dir = $XDG_DATA_DIR/mopidy

$ sudo mopidyctl config [sudo] password for pi: Running "/usr/bin/mopidy --config /usr/share/mopidy/conf.d:/etc/mopidy/mopidy.conf config" as user mopidy [core] cache_dir = /var/cache/mopidy config_dir = /etc/mopidy data_dir = /var/lib/mopidy

If you really must do this (see #2 for why not) then instead use:

sudo mopidyctl config | sudo tee /etc/mopidy/mopidy.conf



2. You'll need to be more careful when updating Mopidy and check the old default values you've specified are still suitable with the new version you just upgrade to. 

The [Mopidy documentation](https://docs.mopidy.com/en/latest/config/) suggests:
> When you have created the configuration file, open it in a text editor, and add the config values you want to change. If you want to keep the default for a config value, you should not add it to the config file, but leave it out so that when we change the default value in a future version, you won’t have to change your configuration accordingly.
Gadgetoid commented 4 years ago

Thanks, this is some really useful insight. I've applied the mopidyctrl based "fix" for the time being, but I think in light of your point 2 this only constitutes a temporary fix.

This clued me into the existence of /usr/share/mopidy/conf.d though, and I wonder if that's perhaps a better place to drop plugin-specific config, and setup-specific settings so that they can be maintained independent of the default settings?

kingosticks commented 4 years ago

I wanted to use that folder a while back for pimusicbox but I think I was talked out of it. Sadly I can't remember the exact reason but I think we might have been considering removing it at the time - people already struggle with the different config files (service vs user) so having more isn't helpful. Maybe @jodal remembers.

In general, /etc/mopidy/mopidy.conf is the right place for a user's specific config overrides, we just like to avoid redefining the entire config again. I think having those 3-4 specific settings you want to change there would be fine, and a bit clearer when people come to upgrade.

jodal commented 4 years ago

/usr/share/mopidy/conf.d is a directory created by the mopidy Debian package for extension Debian packages to install defaults into, in case they need different defaults when installed as a Debian package than what they ship on PyPI. I think it is used very little, if at all, as most extensions these days use the core.{cache,config,data}_dir paths, that are set correctly by the mopidy Debian package, and create their own subdirs inside those three directories.

/etc/mopidy/mopidy.conf is for the user's specific config overrides. The less you put in here, the easier it becomes to upgrade Mopidy in the future as you automatically get updated sane default values for all config values you don't specify.

So, use /etc/mopidy/mopidy.conf for your changes and use sudo mopidyctl config to see the total picture from the perspective of the Mopidy system service.

Update: Ref. https://github.com/mopidy/mopidy-spotify/blob/debian/debian/changelog#L53-L54 it looks like the last package that used /usr/share/mopidy/conf.d stopped using it in 2015. It was replaced by the core.{cache,config,data}_dir paths mentioned above.

Gadgetoid commented 4 years ago

Thank you for weighing in here- it's greatly appreciated. It looks like the path forward is clear, and I'll aim to echo out a basic starter config into /etc/mopidy/mopidy.conf rather than using Mopidy to build out a complete list of config info.

sandyjmacdonald commented 4 years ago

Would you be interested in playing with one of the new Pirate Audio boards, @kingosticks ? No strings attached, just pop us an email with a shipping address and we can sort it out.

kingosticks commented 4 years ago

Thanks but mine came in the post this morning! Will play with it this weekend.

On Thu, 21 Nov 2019, 12:48 Sandy Macdonald, notifications@github.com wrote:

Would you be interested in playing with one of the new Pirate Audio boards, @kingosticks https://github.com/kingosticks ? No strings attached, just pop us an email with a shipping address and we can sort it out.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pimoroni/pirate-audio/issues/1?email_source=notifications&email_token=AAHEHKGZ524K4XXYKC24L6TQUZ7TDA5CNFSM4JPFMZK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE2DOBY#issuecomment-557070087, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHEHKGBEFPKJYKSV3MJKO3QUZ7TDANCNFSM4JPFMZKQ .

sandyjmacdonald commented 4 years ago

πŸ™‚

jodal commented 4 years ago

If you ship to Norway, I'd love one! My 7 year old has already begged me to get him one of these music devices running the software dad spends his nights hacking on for Christmas ;-)

sandyjmacdonald commented 4 years ago

@jodal Pop an email to sandy at pimoroni dot com and I'll sort it πŸ™‚

Gadgetoid commented 4 years ago

In a bid to make our install script less terrible it now backs up /etc/mopidy/mopidy.conf and applies a default configuration for raspberry-gpio and pidi including some additional minimal options for mpd, http, audio and spotify.

kingosticks commented 4 years ago

Looks good. I left a comment at https://github.com/pimoroni/pirate-audio/commit/a5740728dd7d12717b7759b442035a33b16a5eaa#r36074921

kingosticks commented 4 years ago

Ahh ok, not quite there. The problem with this is that the Mopidy Debian package ships some service-specific config vales (below) and now these are lost. These are some of the ones getting the wrong values with the original implementation.

[core]
cache_dir = /var/cache/mopidy
config_dir = /etc/mopidy
data_dir = /var/lib/mopidy

[logging]
config_file = /etc/mopidy/logging.conf
debug_file = /var/log/mopidy/mopidy-debug.log

[local]
media_dir = /var/lib/mopidy/media

[m3u]
playlists_dir = /var/lib/mopidy/playlists

I think you could argue that we should have technically put the above stuff in /usr/share/mopidy/conf.d...

As things currently stand, data_dir is set incorrectly and this seems to break Iris (and probably other things). I'd just append your changes rather than replace the entire file.

Gadgetoid commented 4 years ago

Thank you. I've addressed your comment (ooh that was a silly mistake, in my defence I've been really ill over the last couple of weeks) and changed the config to be appended rather than simply overwriting mopidy.conf.

Looks like I need to re-image and see what I end up with now.

Furthermore some way to detect an old botched config and repair it might be handy- I guess a sudo apt install --reinstall mopidy would be a good start.

kingosticks commented 4 years ago

I don't think that apt command will touch any existing config files, they are treated as a special case in the Deb package world. You'd want to first manually remove the config file, or get apt to remove the whole package (with --purge) first.

But how to detect the config file is botched? We do our best to validate and emit log messages when a value is clearly wrong but it's otherwise hard to tell.

Gadgetoid commented 4 years ago

Turns out the secret sauce for fixing (read: eliminating and replacing with the default config) a previously botched config file is:

sudo apt install --reinstall -o Dpkg::Options::="--force-confask,confnew,confmiss" mopidy

For existing users with possibly botched configs, it might be worth just giving them some manual steps to fix everything up.

kingosticks commented 4 years ago

Im surprised that deleting the config file and then doing a normal reinstall doesn't fix it. But whatever works.

Ennneerrrgggyyy commented 4 years ago

Is there a problem with the config at the moment?

I followed the instructions from https://learn.pimoroni.com/tutorial/sandyj/getting-started-with-pirate-audio on my Zero WH. As soon as I try to connect to Iris the settings page won't load. I see only a black background. When I reload the page I briefly see the actual settings page, but it vanishes right away.

Gadgetoid commented 4 years ago

Not insofar as I'm aware- you don't have any kind of adblock or other browser plugins enabled that might break it?

kingosticks commented 4 years ago

Not sure if this related to this original bug to be honest. I think you'd be better off opening a new issue on the mopidy-iris bug tracker.

aculver28 commented 4 years ago

FWIW, I just got one in the mail today and am having the exact same issue as @Ennneerrrgggyyy so there's definitely something wrong. Region is defaulting to NZ, mpd config is missing, I'm seeing some python websocket exceptions in the logs, etc. I'm going to dig into the code a bit before logging an issue, but something is definitely off.

Gadgetoid commented 4 years ago

This black screen issue is apparently already fixed in Mopidy-Iris 3.52.2-

run:

sudo python3 -m pip install mopidy-iris --upgrade

or this should also work:

sudo pip3 install mopidy-iris --upgade

Then restarty Mopidy with:

sudo systemctl restart mopidy

(or reboot your Pi)

Ennneerrrgggyyy commented 4 years ago

Awesome! That worked, thank you @Gadgetoid

aculver28 commented 4 years ago

Ha...was just coming to post that an update fixed it. Thanks @Gadgetoid!