hnarayanan / shpotify

A command-line interface to Spotify.
https://harishnarayanan.org/projects/
2.02k stars 153 forks source link

`spotify vol` either reads or sets volume incorrectly. #58

Open ericmarkmartin opened 8 years ago

ericmarkmartin commented 8 years ago

I observed the following behavior:

$ spotify vol
Current Spotify volume level is 100.
$ spotify vol 90
$ spotify vol
Current Spotify volume level is 89.

This behavior also occurs with spotify vol up and spotify vol down. Note that this behavior was not changed when #45 was merged which caused spotify vol [amt] to print a message that the volume was being changed.

Please also note that this behavior is erratic and doesn't always result in the volume being (or being read as) 1 less than the requested volume.

hnarayanan commented 8 years ago

I noticed this too. And also noticed this bad behaviour existed before.

ericmarkmartin commented 8 years ago

Unfortunately, I believe this is probably a problem with the Spotify AppleScript interface and thus out of our hands, unless someone wants to submit a bug report to Spotify, and I really have no idea what the process looks like especially in terms of time frame.

hnarayanan commented 8 years ago

I don't think anyone really cares or can discern few % diff in the audio level. Will leave this open anyway for some arbitrary point in the future.

ericmarkmartin commented 8 years ago

Yeah. It's not so much a usability problem but it would make life difficult if I were hypothetically writing a test suite.

hnarayanan commented 8 years ago

This is not (or shouldn't be!) a mission critical app. But even if one were testing, they'd need to code up an approximately_equals operator.

ericmarkmartin commented 8 years ago

That's what I figured

hnarayanan commented 8 years ago

The following highly unscientific patch seems to work. I don't have an explanation other than to blame floating point arithmetic or something, even though we're supposedly working with integers.

--- a/spotify
+++ b/spotify
@@ -235,16 +235,16 @@ while [ $# -gt 0 ]; do
                 break ;
             elif [ "$2" = "up" ]; then
                 if [ $vol -le 90 ]; then
-                    newvol=$(( vol+10 ));
-                    cecho "Increasing Spotify volume to $newvol.";
+                    newvol=$(( vol+10+1 ));
+                    cecho "Increasing Spotify volume to $(( newvol-1 )).";
                 else
                     newvol=100;
                     cecho "Spotify volume level is at max.";
                 fi
             elif [ "$2" = "down" ]; then
                 if [ $vol -ge 10 ]; then
-                    newvol=$(( vol-10 ));
-                    cecho "Reducing Spotify volume to $newvol.";
+                    newvol=$(( vol-10+1 ));
+                    cecho "Reducing Spotify volume to $(( newvol-1 )).";
                 else
                     newvol=0;
                     cecho "Spotify volume level is at min.";
ericmarkmartin commented 8 years ago

sighs. This is why we need DEC64.