search

balena-io-experimental / balena-sound

Build a single or multi-room streamer for an existing audio device using a Raspberry Pi! Supports Bluetooth, Airplay and Spotify Connect
https://balena.io/blog/turn-your-old-speakers-or-hi-fi-into-bluetooth-receivers-using-only-a-raspberry-pi/
MIT License
2.43k stars 430 forks source link

fix for bug 626 - Spotify customization variables #628

Closed laytonhayes closed 1 year ago

laytonhayes commented 1 year ago

Fix for Spotify customization variables SOUND_SPOTIFY_DISABLE_NORMALISATION and SOUND_SPOTIFY_ENABLE_CACHE. Now the variables can be defined and have no value and work correctly.

Closes: https://github.com/balena-labs-projects/balena-sound/issues/626

maggie44 commented 1 year ago

Nice, thanks!

It looks like the script in this container is run by bash though:

CMD [ "/bin/bash", "/usr/src/start.sh" ]

So the shebang would be:

#!/usr/bin/env bash

With the correct shebang in place, you can then paste the whole script in to this awesome tool to take care of any other syntax: https://www.shellcheck.net

laytonhayes commented 1 year ago

Thanks for reviewing and pointing me to shellcheck.net, @maggie0002! Great resource. I've updated the code based on your comments, rebased, and pushed.

maggie44 commented 1 year ago

Looks good to me, thanks.

Have you been able to test it all? I am not able to on my end, but if you are happy with the fix and to take the wrath of the balenaSound community then I will go ahead and merge.

maggie44 commented 1 year ago

Closes: https://github.com/balena-labs-projects/balena-sound/issues/626

lgtm

maggie44 commented 1 year ago

Did this also end up able to close: https://github.com/balena-labs-projects/balena-sound/issues/620 ?

laytonhayes commented 1 year ago

@maggie0002 - I've tested it on two devices with all variable permutations and had no issues. I'd rather not face the wrath ;) I haven't added a fix for #620 but I imagine it's a similar fix. I can have a look after this is merged in. Thanks for reviewing!

maggie44 commented 1 year ago

I'd rather not face the wrath

Merges only impact new deploys, not old ones, so it's relatively low risk in the sense we can always go back if someone raises an issue. 👍

fgatin commented 1 year ago

Hello, did this end up fixing #620 or has there been a fix for #620? It's quite important to me since I'm trying to run the software on the Pi Zero and without the possibility to disable all these services the Pi is not powerful enough.