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

Toggle mute/unmute #40

Closed
pheck opened this issue Apr 24, 2018 · 7 comments
Closed

Toggle mute/unmute #40

pheck opened this issue Apr 24, 2018 · 7 comments

Comments

@pheck
Copy link
Contributor

pheck commented Apr 24, 2018

Hi MiczFlor,

I followed your guide and have the system up & running. Thanks for your efforts!
(the components are still spread out accross my desk; fitting them into a nice case will be the harder part ;))

I made two small additions in ~/scripts/playout_controls.sh

  1. the mute command now toggles between mute/unmute,
  2. the volumeup/-down commands now also unmutes if needed and starts from the volume level that was set before sound was muted.

To do that, I basically just save the current volume level to a file and read it from there if the file is present (=volume was muted), otherwise the commands just work as before.

This is just mimicking the common behaviour from remote controls.
(I've added a cheap 5-button remote Seki Slim + IR-receiver TSOP4838 over GPIO + jumper cables - no soldering, right? - for less than 10€)

Code is:

[...]
VOLSTEP=5
VOLFILE=/var/tmp/volume
[...]
elif [ $COMMAND == "mute" ]
then
	[ ! -e $VOLFILE ] && (amixer sget \'$DEVICE\' | egrep -o '[[:space:]][0-9]+[[:space:]]' | tail -n1> $VOLFILE && amixer sset \'$DEVICE\' 0%) || (amixer sset \'$DEVICE\' `<$VOLFILE` && rm -f $VOLFILE)

elif [ $COMMAND == "setvolume" ]
then
    amixer sset \'$DEVICE\' $VALUE%

elif [ $COMMAND == "volumeup" ]
then
    [ -e $VOLFILE ] && (vol=`<$VOLFILE` && vol=`expr ${vol} + ${VOLSTEP}` && amixer sset \'$DEVICE\' $vol && rm -f $VOLFILE) || (amixer sset \'$DEVICE\' ${VOLSTEP}+)

elif [ $COMMAND == "volumedown" ]
then
	[ -e $VOLFILE ] && (vol=`<$VOLFILE` && vol=`expr ${vol} - ${VOLSTEP}` && amixer sset \'$DEVICE\' $vol && rm -f $VOLFILE) || (amixer sset \'$DEVICE\' ${VOLSTEP}-)
[...]

Let me know what you think!

@ralle12345
Copy link
Contributor

Good idea! But to save writes on the SD card please set VOLFILE=/var/tmp/volume. With the read only changes I'm about to commit /var/tmp will be a volatile directory on a tmpfs.

@pheck
Copy link
Contributor Author

pheck commented Apr 24, 2018

Good point, edited my 1st comment accordingly.

@MiczFlor
Copy link
Owner

Hi @pheck
I like your approach. And would like to include it. There are two ways to do this. The cooler one would be if you created a pull request from a forked version. Because that would imply that the code works for you in production. Here is howto:
https://www.digitalocean.com/community/tutorials/how-to-create-a-pull-request-on-github

The second way would be, I tweak it into the code. Which is totally fine, if you prefer that way.

The added advantage of the pull request would be: you become an official contributor :) fame!

Here is a small "howto".
https://www.digitalocean.com/community/tutorials/how-to-create-a-pull-request-on-github

Tell me what you would prefer.

All the best, micz

@pheck
Copy link
Contributor Author

pheck commented Apr 27, 2018

I'll go with the fame-thing then! :p
Let me put together a PR; thanks for the links.
Stay tuned...

pheck added a commit to pheck/RPi-Jukebox-RFID that referenced this issue Apr 27, 2018
pheck added a commit to pheck/RPi-Jukebox-RFID that referenced this issue May 9, 2018
pheck added a commit to pheck/RPi-Jukebox-RFID that referenced this issue May 9, 2018
MiczFlor added a commit that referenced this issue May 11, 2018
#40-enable mute toggle. vol-up/dn also unmutes if needed
@MiczFlor
Copy link
Owner

Hi @pheck
I created a new branch with your pull request and now made some changes to make the code more human readable ;) I need your feedback and thumbs up if my changes work for you.

also I changed a few things:

  • if mute and press volume up or down, then firstly set volume to saved value before changing the value
  • always read volume percentage and step up step down is also volume percentage. This way it should be universal no matter what the audio iface is
  • if somebody does a git pull, their script will still work (once committed) because the commit changes only the .sample version of the file. However, I still added a check if file exists else create

Please take a look and tell me what you think:
https://github.com/MiczFlor/RPi-Jukebox-RFID/blob/pr-testing-volume-toggle/scripts/playout_controls.sh.sample

@pheck
Copy link
Contributor Author

pheck commented May 23, 2018

Hi @MiczFlor,
right now I'm more on the hardware-side of things and I have the parts scattered on my workbench waiting to be put together into a jukebox, so I won't be able to test the PR until I get enough spare time to finally assemble the box.

As for your changes:
percentage: 100% agreed.

From looking at the code:
May I draw your attention to my comment here: #42 (comment)
IMHO, the one-file-per-setting approach is a bit over the top with all the file-exists/echo to/read from stuff one has to do for each individual setting.
Using files/contents is a perfect fit if you are actually dealing with lists (i.e. playlists) or want to make things more "tangible" and therefore use files & folders (i.e. shortcuts, audiofolders).

But for the settings, I'd rather like to see sth. like
../settings/settings.sh
DEVICE=whatever
VOLSTEP=5
VOLFILE=somefile
which gets called at the beginning of playout_controls.sh to make all settings available to the rest of the script in one go.

@MiczFlor
Copy link
Owner

fdac4a2

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

No branches or pull requests

3 participants