After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 765609 - MPRIS: Setting Shuffle raises an exception
MPRIS: Setting Shuffle raises an exception
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-music-maint
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2016-04-26 13:31 UTC by Gaurav Narula
Modified: 2016-05-12 15:55 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
mpris: fix method call to set_repeat_mode (1.26 KB, patch)
2016-04-26 13:40 UTC, Gaurav Narula 		none Details | Review
mpris: fix method call to set_repeat_mode (1.27 KB, patch)
2016-05-12 10:47 UTC, Gaurav Narula 		committed Details | Review

Description Gaurav Narula 2016-04-26 13:31:44 UTC 
Steps to reproduce:

1. Set shuffle manually in the player
2. Try setting shuffle to On using an MPRIS client.

Exception:
Traceback (most recent call last):

+ Trace 236207

  • File "/home/gaurav/jhbuild/checkout/gnome-music/gnomemusic/mpris.py", line 73 in on_method_call 
    result = getattr(self, method_name)(*args)
  • File "/home/gaurav/jhbuild/checkout/gnome-music/gnomemusic/mpris.py", line 754 in Set 
    self.set_repeat_mode(RepeatType.NONE)
AttributeError: 'MediaPlayer2Service' object has no attribute 'set_repeat_mode' 






Comment 1


Gaurav Narula





          2016-04-26 13:40:26 UTC
        



Created attachment 326770 [details] [review]
mpris: fix method call to set_repeat_mode






Comment 2


Marinus Schraal





          2016-05-10 22:24:18 UTC
        



Review of attachment 326770 [details] [review]:

Oversight by the original author it seems, no-one ever noticed. Looks ok to me.






Comment 3


Marinus Schraal





          2016-05-12 10:31:07 UTC
        



Review of attachment 326770 [details] [review]:

I'm flip-flopping a bit, sorry.

::: gnomemusic/mpris.py
@@ +752,3 @@
                 if (new_value and self.player.get_repeat_mode() != RepeatType.SHUFFLE):
+                    self.player.set_repeat_mode(RepeatType.SHUFFLE)
+                elif not new_value and self.player.get_repeat_mode() == RepeatType.SHUFFLE:

Why does this go from 'elif new_value' to 'elif not new_value'. I assume from your code that new_value is a bool (0 or 1?) that is either off or on depending on if shuffle is wanted or not.

Why not write

if new_value
  set_repeat_mode(SHUFFLE)
else
  set_repeat_mode(None)






Comment 4


Gaurav Narula





          2016-05-12 10:47:29 UTC
        



Created attachment 327690 [details] [review]
mpris: fix method call to set_repeat_mode

My bad, I just stuck with the semantics of the old code. The second part of the if condition isn't really necessary. Resubmitted the patch.






Comment 5


Marinus Schraal





          2016-05-12 11:03:19 UTC
        



Review of attachment 327690 [details] [review]:

Looks fine.






Comment 6


Marinus Schraal





          2016-05-12 15:55:16 UTC
        



Thanks for the patch, committed.

This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.