volumio / Build

Buildscripts for Volumio System
GNU General Public License v2.0
113 stars 103 forks source link

Include default mpd conf #507

Closed volumio closed 3 years ago

ashthespy commented 3 years ago

Were you able to test the suggestion from https://github.com/volumio/Build/commit/20e3c8c821896d19a48b8e9681de072f39aceb18#commitcomment-48138201 ?

It should be as simple as adding a single line to finalize.sh

systemctl disable mpd.service
systemctl disable mpd.socket

With this, you are still going to have race conditions + dual configurations that are going to go out of sync..

volumio commented 3 years ago

We already tried, but did not work because:

gkkpch commented 3 years ago

This will not work, we already tried it. As soon as a usb drive is mounted, an "mpc update" is issued which starts mpd anyway (via socket??). So we keep being too late with the mpd.conf correction. I believe Mi fixed it

ashthespy commented 3 years ago

ah, that is a shame! Thanks for the clarification. 👍

ashthespy commented 3 years ago

I was reading something else and came across this - FWIW, the socket is a separate service mpd.socket, and probably also needs to be stopped.

ashthespy commented 2 years ago

After a MPD upgrade, I got frustrated with the double configuration templates, so looked into this a bit more.

Mpd is started anyway when the socket is initialized, resulting in a deadlock

No, MPD is started when a connection is initiated via the socket, not on socket initialisation. i.e if mpd.service is stopped, but mpd.socket is started, then any connection to /run/mpd/socket triggers mpd.service

As soon as a usb drive is mounted, an "mpc update" is issued

I don't believe this is a feature of mpc? I couldn't find it anywhere. I would guess it's something the mount handler in the volumio-core does this right?

This is what I have that seems to work in my case From the build side:

systemctl stop mpd.service mpd.socket

Then from volumio-core side

diff --git a/app/plugins/music_service/mpd/index.js b/app/plugins/music_service/mpd/index.js
index b6808a6b..e0ad0db9 100644
--- a/app/plugins/music_service/mpd/index.js
+++ b/app/plugins/music_service/mpd/index.js
@@ -616,6 +616,9 @@ ControllerMpd.prototype.mpdInit = function () {
         self.logger.error('Could not create MPD File on system start: ' + error);
         defer.resolve();
       } else {
+        // Start mpd's socket so it can be started by libMpd.connect's call or subsequent service calls
+        // But start it only if it's not already started. Some NodeShell to the rescue (should be posix and not bash)
+        execSync('[ -S /run/mpd/socket ] || /usr/bin/sudo /bin/systemctl start mpd.socket', { uid: 1000, gid: 1000, encoding: 'utf8' });
         self.restartMpd(function (error) {
           if (error !== null && error != undefined) {
             self.logger.error('Cannot start MPD on system Start: ' + error);

This essentially doesn't let MPD start (the first time) before the configuration file is written by volumio-core. It also starts the mpd.socket service, for subsequent libMpd calls..

PS: mpd/index.js is such a mess and can really do with some ❤️!

ashthespy commented 2 years ago

I would guess it's something the mount handler in the volumio-core does this right?

Yep, here you go networkfs/index.js it should be straightforward to get that function to use updateDb instead.

volumio commented 2 years ago

This solution seems interesting, can you do a PR? Remember to always wrap sync functions in try catch ;)