search

TonyTromp / tidal-connect-docker

This is the dockerized version of Tidal Connect Raspberry binairies. Can be seemlessly used in HifiberryOS or any other RPi based operating system running Docker.
162 stars 28 forks source link

Beocreate service call fixes #5

Closed imgrant closed 2 years ago

imgrant commented 2 years ago

Beocreate should use systemctl enable|disable --now to control the systemd service, not start|stop (for which the --now flag has no meaning), this PR fixes that. In addition, a couple of typos/leftovers that made reference to Roon have been fixed.

TonyTromp commented 2 years ago

Hi imgrant,

Can you please explain why you used systemctl enable instead of start/stop?

For stopping and starting services, systemctl stop/start is adequate. For service installation (and starting service on system boot) systemctl enable/disable is used.

https://www.digitalocean.com/community/tutorials/how-to-use-systemctl-to-manage-systemd-services-and-units

imgrant commented 2 years ago

Hi, sure.

As you say, systemctl start|stop starts or stops the service right now (and hence why --now doesn't mean anything in this context), but it doesn't adjust the enabled state, so if the service is still enabled, it will start again when the system reboots. This is, as far as I can see, contrary to the UI design in Beocreate — there is a single toggle for services, described as "On", and, for all the other services, that uses systemctl enable|disable --now. That does two operations: starts or stops the service right now (the --now bit), and also enables or disables it, so that, e.g. if set to off, it doesn't start on reboot.

So the PR just brings this service in line with the others, toggling the "On" switch for the service starts or stops it, and enables or disables it for future reboots. Do you think this is appropriate?

TonyTromp commented 2 years ago

Hi Imgrant,

You are absolutely right and this PR will be merged. I didnt take the reboot behaviour in account at all. Thanks for providing the justification as i now understand why this is beneficial from a design point of view.

Kind regards, Tony

imgrant commented 2 years ago

No worries, glad I could help & glad to see it merged! The next thing on my mind is HiFiBerryOS updates — I believe at the moment, all the config such as linking Beocreate extension, installing systems service file, will be undone at update, because the HiFiBerryOS updater only copies certain files from the root partition. Is that right? Something to think about for another issue/PR.