Merge lp:~xavi-garcia-mena/indicator-sound/mpris-bug-1373313 into lp:indicator-sound/15.10

Proposed by Xavi Garcia
Status: Merged
Approved by: Xavi Garcia
Approved revision: 506
Merged at revision: 506
Proposed branch: lp:~xavi-garcia-mena/indicator-sound/mpris-bug-1373313
Merge into: lp:indicator-sound/15.10
Diff against target: 462 lines (+251/-85)
6 files modified
src/media-player-mpris.vala (+22/-0)
src/media-player.vala (+4/-0)
src/mpris2-interfaces.vala (+4/-1)
src/sound-menu.vala (+61/-6)
tests/media-player-mock.vala (+6/-0)
tests/sound-menu.cc (+154/-78)
To merge this branch: bzr merge lp:~xavi-garcia-mena/indicator-sound/mpris-bug-1373313
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Xavi Garcia Approve
Review via email: mp+274680@code.launchpad.net

Commit message

Wily branch for MPRIS controls

Description of the change

Wily branch for MPRIS controls

To post a comment you must log in.
Revision history for this message
Xavi Garcia (xavi-garcia-mena) wrote :

I'm approving as the original branch was approved by Charles Kerr and nothing has changed since then.

This is just a wily MP.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/media-player-mpris.vala'
2--- src/media-player-mpris.vala 2015-03-26 15:56:49 +0000
3+++ src/media-player-mpris.vala 2015-10-16 09:00:52 +0000
4@@ -79,6 +79,24 @@
5 }
6 }
7
8+ public override bool can_do_play {
9+ get {
10+ return this.proxy.CanPlay;
11+ }
12+ }
13+
14+ public override bool can_do_prev {
15+ get {
16+ return this.proxy.CanGoPrevious;
17+ }
18+ }
19+
20+ public override bool can_do_next {
21+ get {
22+ return this.proxy.CanGoNext;
23+ }
24+ }
25+
26 /**
27 * Attach this object to a process of the associated media player. The player must own @dbus_name and
28 * implement the org.mpris.MediaPlayer2.Player interface.
29@@ -272,6 +290,10 @@
30 if (changed_properties.lookup ("PlaybackStatus", "s", null)) {
31 this.state = this.proxy.PlaybackStatus != null ? this.proxy.PlaybackStatus : "Unknown";
32 }
33+ if (changed_properties.lookup ("CanGoNext", "b", null) || changed_properties.lookup ("CanGoPrevious", "b", null) ||
34+ changed_properties.lookup ("CanPlay", "b", null) || changed_properties.lookup ("CanPause", "b", null)) {
35+ this.playbackstatus_changed ();
36+ }
37
38 var metadata = changed_properties.lookup_value ("Metadata", new VariantType ("a{sv}"));
39 if (metadata != null)
40
41=== modified file 'src/media-player.vala'
42--- src/media-player.vala 2014-02-12 16:21:32 +0000
43+++ src/media-player.vala 2015-10-16 09:00:52 +0000
44@@ -26,6 +26,9 @@
45
46 public virtual bool is_running { get { not_implemented(); return false; } }
47 public virtual bool can_raise { get { not_implemented(); return false; } }
48+ public virtual bool can_do_next { get { not_implemented(); return false; } }
49+ public virtual bool can_do_prev { get { not_implemented(); return false; } }
50+ public virtual bool can_do_play { get { not_implemented(); return false; } }
51
52 public class Track : Object {
53 public string artist { get; construct; }
54@@ -44,6 +47,7 @@
55 }
56
57 public signal void playlists_changed ();
58+ public signal void playbackstatus_changed ();
59
60 public abstract void activate ();
61 public abstract void play_pause ();
62
63=== modified file 'src/mpris2-interfaces.vala'
64--- src/mpris2-interfaces.vala 2015-03-26 15:56:49 +0000
65+++ src/mpris2-interfaces.vala 2015-10-16 09:00:52 +0000
66@@ -37,7 +37,10 @@
67 // properties
68 public abstract HashTable<string, Variant?> Metadata{owned get; set;}
69 public abstract int32 Position{owned get; set;}
70- public abstract string? PlaybackStatus{owned get; set;}
71+ public abstract string? PlaybackStatus{owned get; set;}
72+ public abstract bool CanPlay{owned get; set;}
73+ public abstract bool CanGoNext{owned get; set;}
74+ public abstract bool CanGoPrevious{owned get; set;}
75 // methods
76 public abstract async void PlayPause() throws IOError;
77 public abstract async void Next() throws IOError;
78
79=== modified file 'src/sound-menu.vala'
80--- src/sound-menu.vala 2015-02-12 00:44:34 +0000
81+++ src/sound-menu.vala 2015-10-16 09:00:52 +0000
82@@ -157,18 +157,26 @@
83 this.update_playlists (player);
84
85 var handler_id = player.notify["is-running"].connect ( () => {
86- if (player.is_running)
87- if (this.find_player_section(player) == -1)
88+ if (player.is_running) {
89+ int index = this.find_player_section(player);
90+ if (index == -1) {
91 this.insert_player_section (player);
92- else
93+ }
94+ else {
95+ update_player_section (player, index);
96+ }
97+ }
98+ else {
99 if (this.hide_inactive)
100 this.remove_player_section (player);
101+ }
102
103 this.update_playlists (player);
104 });
105 this.notify_handlers.insert (player, handler_id);
106
107 player.playlists_changed.connect (this.update_playlists);
108+ player.playbackstatus_changed.connect (this.update_playbackstatus);
109 }
110
111 public void remove_player (MediaPlayer player) {
112@@ -215,6 +223,23 @@
113 return -1;
114 }
115
116+ MenuItem create_playback_menu_item (MediaPlayer player) {
117+ var playback_item = new MenuItem (null, null);
118+ playback_item.set_attribute ("x-canonical-type", "s", "com.canonical.unity.playback-item");
119+ if (player.is_running) {
120+ if (player.can_do_play) {
121+ playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
122+ }
123+ if (player.can_do_next) {
124+ playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id);
125+ }
126+ if (player.can_do_prev) {
127+ playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id);
128+ }
129+ }
130+ return playback_item;
131+ }
132+
133 void insert_player_section (MediaPlayer player) {
134 if (this.hide_players)
135 return;
136@@ -240,9 +265,21 @@
137
138 var playback_item = new MenuItem (null, null);
139 playback_item.set_attribute ("x-canonical-type", "s", "com.canonical.unity.playback-item");
140- playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
141- playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id);
142- playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id);
143+ playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id + ".disabled");
144+ playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id + ".disabled");
145+ playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id + ".disabled");
146+
147+ if (player.is_running) {
148+ if (player.can_do_play) {
149+ playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
150+ }
151+ if (player.can_do_next) {
152+ playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id);
153+ }
154+ if (player.can_do_prev) {
155+ playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id);
156+ }
157+ }
158 section.append_item (playback_item);
159
160 /* Add new players to the end of the player sections, just before the settings */
161@@ -262,6 +299,17 @@
162 this.menu.remove (index);
163 }
164
165+ void update_player_section (MediaPlayer player, int index) {
166+ var player_section = this.menu.get_item_link(index, Menu.LINK_SECTION) as Menu;
167+ if (player_section.get_n_items () == 2) {
168+ // we have 2 items, the second one is the playback item
169+ // remove it first
170+ player_section.remove (1);
171+ MenuItem playback_item = create_playback_menu_item (player);
172+ player_section.append_item (playback_item);
173+ }
174+ }
175+
176 void update_playlists (MediaPlayer player) {
177 int index = find_player_section (player);
178 if (index < 0)
179@@ -292,6 +340,13 @@
180 submenu.append_section (null, playlists_section);
181 player_section.append_submenu (_("Choose Playlist"), submenu);
182 }
183+
184+ void update_playbackstatus (MediaPlayer player) {
185+ int index = find_player_section (player);
186+ if (index != -1) {
187+ update_player_section (player, index);
188+ }
189+ }
190
191 MenuItem create_slider_menu_item (string label, string action, double min, double max, double step, string min_icon_name, string max_icon_name) {
192 var min_icon = new ThemedIcon.with_default_fallbacks (min_icon_name);
193
194=== modified file 'tests/media-player-mock.vala'
195--- tests/media-player-mock.vala 2014-02-12 19:54:59 +0000
196+++ tests/media-player-mock.vala 2015-10-16 09:00:52 +0000
197@@ -28,6 +28,9 @@
198
199 public override bool is_running { get { return mock_is_running; } }
200 public override bool can_raise { get { return mock_can_raise; } }
201+ public override bool can_do_next { get { return mock_can_do_next; } }
202+ public override bool can_do_prev { get { return mock_can_do_prev; } }
203+ public override bool can_do_play { get { return mock_can_do_play; } }
204
205 public override MediaPlayer.Track? current_track { get { return mock_current_track; } set { this.mock_current_track = value; } }
206
207@@ -40,6 +43,9 @@
208
209 public bool mock_is_running { get; set; }
210 public bool mock_can_raise { get; set; }
211+ public bool mock_can_do_next { get; set; }
212+ public bool mock_can_do_prev { get; set; }
213+ public bool mock_can_do_play { get; set; }
214
215 public MediaPlayer.Track? mock_current_track { get; set; }
216
217
218=== modified file 'tests/sound-menu.cc'
219--- tests/sound-menu.cc 2014-03-03 21:38:38 +0000
220+++ tests/sound-menu.cc 2015-10-16 09:00:52 +0000
221@@ -27,87 +27,163 @@
222
223 class SoundMenuTest : public ::testing::Test
224 {
225- protected:
226- GTestDBus * bus = nullptr;
227-
228- virtual void SetUp() {
229- bus = g_test_dbus_new(G_TEST_DBUS_NONE);
230- g_test_dbus_up(bus);
231- }
232-
233- virtual void TearDown() {
234- g_test_dbus_down(bus);
235- g_clear_object(&bus);
236- }
237-
238- void verify_item_attribute (GMenuModel * mm, guint index, const gchar * name, GVariant * value) {
239- g_variant_ref_sink(value);
240-
241- gchar * variantstr = g_variant_print(value, TRUE);
242- g_debug("Expecting item %d to have a '%s' attribute: %s", index, name, variantstr);
243-
244- const GVariantType * type = g_variant_get_type(value);
245- GVariant * itemval = g_menu_model_get_item_attribute_value(mm, index, name, type);
246-
247- ASSERT_NE(nullptr, itemval);
248- EXPECT_TRUE(g_variant_equal(itemval, value));
249-
250- g_variant_unref(value);
251- }
252+ protected:
253+ GTestDBus * bus = nullptr;
254+
255+ virtual void SetUp() {
256+ bus = g_test_dbus_new(G_TEST_DBUS_NONE);
257+ g_test_dbus_up(bus);
258+ }
259+
260+ virtual void TearDown() {
261+ g_test_dbus_down(bus);
262+ g_clear_object(&bus);
263+ }
264+
265+ void verify_item_attribute (GMenuModel * mm, guint index, const gchar * name, GVariant * value) {
266+ g_variant_ref_sink(value);
267+
268+ gchar * variantstr = g_variant_print(value, TRUE);
269+ g_debug("Expecting item %d to have a '%s' attribute: %s", index, name, variantstr);
270+
271+ const GVariantType * type = g_variant_get_type(value);
272+ GVariant * itemval = g_menu_model_get_item_attribute_value(mm, index, name, type);
273+
274+ ASSERT_NE(nullptr, itemval);
275+ EXPECT_TRUE(g_variant_equal(itemval, value));
276+
277+ g_variant_unref(value);
278+ }
279+
280+ void verify_item_attribute_is_not_set(GMenuModel * mm, guint index, const gchar * name, const GVariantType * type) {
281+ GVariant * itemval = g_menu_model_get_item_attribute_value(mm, index, name, type);
282+ EXPECT_EQ(itemval, nullptr);
283+ }
284+
285+ void check_player_control_buttons(bool canPlay, bool canNext, bool canPrev)
286+ {
287+ SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
288+
289+ MediaPlayerTrack * track = media_player_track_new("Artist", "Title", "Album", "http://art.url");
290+
291+ MediaPlayerMock * media = MEDIA_PLAYER_MOCK(
292+ g_object_new(TYPE_MEDIA_PLAYER_MOCK,
293+ "mock-id", "player-id",
294+ "mock-name", "Test Player",
295+ "mock-state", "Playing",
296+ "mock-is-running", TRUE,
297+ "mock-can-raise", FALSE,
298+ "mock-current-track", track,
299+ "mock-can-do-play", canPlay,
300+ "mock-can-do-next", canNext,
301+ "mock-can-do-prev", canPrev,
302+ NULL)
303+ );
304+ g_clear_object(&track);
305+
306+ sound_menu_add_player(menu, MEDIA_PLAYER(media));
307+
308+ ASSERT_NE(nullptr, menu->menu);
309+ EXPECT_EQ(2, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
310+
311+ GMenuModel * section = g_menu_model_get_item_link(G_MENU_MODEL(menu->menu), 1, G_MENU_LINK_SECTION);
312+ ASSERT_NE(nullptr, section);
313+ EXPECT_EQ(2, g_menu_model_get_n_items(section)); /* No playlists, so two items */
314+
315+ /* Player display */
316+ verify_item_attribute(section, 0, "action", g_variant_new_string("indicator.player-id"));
317+ verify_item_attribute(section, 0, "x-canonical-type", g_variant_new_string("com.canonical.unity.media-player"));
318+
319+ /* Player control */
320+ verify_item_attribute(section, 1, "x-canonical-type", g_variant_new_string("com.canonical.unity.playback-item"));
321+ verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string(canPlay ? "indicator.play.player-id" : "indicator.play.player-id.disabled"));
322+ verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string(canNext ? "indicator.next.player-id" : "indicator.next.player-id.disabled"));
323+ verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string(canPrev ? "indicator.previous.player-id" : "indicator.previous.player-id.disabled"));
324+
325+ g_clear_object(&section);
326+
327+ sound_menu_remove_player(menu, MEDIA_PLAYER(media));
328+
329+ EXPECT_EQ(1, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
330+
331+ g_clear_object(&media);
332+ g_clear_object(&menu);
333+ }
334 };
335
336 TEST_F(SoundMenuTest, BasicObject) {
337- SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
338-
339- ASSERT_NE(nullptr, menu);
340-
341- g_clear_object(&menu);
342- return;
343+ SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
344+
345+ ASSERT_NE(nullptr, menu);
346+
347+ g_clear_object(&menu);
348+ return;
349 }
350
351 TEST_F(SoundMenuTest, AddRemovePlayer) {
352- SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
353-
354- MediaPlayerTrack * track = media_player_track_new("Artist", "Title", "Album", "http://art.url");
355-
356- MediaPlayerMock * media = MEDIA_PLAYER_MOCK(
357- g_object_new(TYPE_MEDIA_PLAYER_MOCK,
358- "mock-id", "player-id",
359- "mock-name", "Test Player",
360- "mock-state", "Playing",
361- "mock-is-running", TRUE,
362- "mock-can-raise", FALSE,
363- "mock-current-track", track,
364- NULL)
365- );
366- g_clear_object(&track);
367-
368- sound_menu_add_player(menu, MEDIA_PLAYER(media));
369-
370- ASSERT_NE(nullptr, menu->menu);
371- EXPECT_EQ(2, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
372-
373- GMenuModel * section = g_menu_model_get_item_link(G_MENU_MODEL(menu->menu), 1, G_MENU_LINK_SECTION);
374- ASSERT_NE(nullptr, section);
375- EXPECT_EQ(2, g_menu_model_get_n_items(section)); /* No playlists, so two items */
376-
377- /* Player display */
378- verify_item_attribute(section, 0, "action", g_variant_new_string("indicator.player-id"));
379- verify_item_attribute(section, 0, "x-canonical-type", g_variant_new_string("com.canonical.unity.media-player"));
380-
381- /* Player control */
382- verify_item_attribute(section, 1, "x-canonical-type", g_variant_new_string("com.canonical.unity.playback-item"));
383- verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string("indicator.play.player-id"));
384- verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string("indicator.next.player-id"));
385- verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string("indicator.previous.player-id"));
386-
387- g_clear_object(&section);
388-
389- sound_menu_remove_player(menu, MEDIA_PLAYER(media));
390-
391- EXPECT_EQ(1, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
392-
393- g_clear_object(&media);
394- g_clear_object(&menu);
395- return;
396-}
397+ SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
398+
399+ MediaPlayerTrack * track = media_player_track_new("Artist", "Title", "Album", "http://art.url");
400+
401+ MediaPlayerMock * media = MEDIA_PLAYER_MOCK(
402+ g_object_new(TYPE_MEDIA_PLAYER_MOCK,
403+ "mock-id", "player-id",
404+ "mock-name", "Test Player",
405+ "mock-state", "Playing",
406+ "mock-is-running", TRUE,
407+ "mock-can-raise", FALSE,
408+ "mock-current-track", track,
409+ "mock-can-do-play", TRUE,
410+ "mock-can-do-next", TRUE,
411+ "mock-can-do-prev", TRUE,
412+ NULL)
413+ );
414+ g_clear_object(&track);
415+
416+ sound_menu_add_player(menu, MEDIA_PLAYER(media));
417+
418+ ASSERT_NE(nullptr, menu->menu);
419+ EXPECT_EQ(2, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
420+
421+ GMenuModel * section = g_menu_model_get_item_link(G_MENU_MODEL(menu->menu), 1, G_MENU_LINK_SECTION);
422+ ASSERT_NE(nullptr, section);
423+ EXPECT_EQ(2, g_menu_model_get_n_items(section)); /* No playlists, so two items */
424+
425+ /* Player display */
426+ verify_item_attribute(section, 0, "action", g_variant_new_string("indicator.player-id"));
427+ verify_item_attribute(section, 0, "x-canonical-type", g_variant_new_string("com.canonical.unity.media-player"));
428+
429+ /* Player control */
430+ verify_item_attribute(section, 1, "x-canonical-type", g_variant_new_string("com.canonical.unity.playback-item"));
431+ verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string("indicator.play.player-id"));
432+ verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string("indicator.next.player-id"));
433+ verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string("indicator.previous.player-id"));
434+
435+ g_clear_object(&section);
436+
437+ sound_menu_remove_player(menu, MEDIA_PLAYER(media));
438+
439+ EXPECT_EQ(1, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
440+
441+ g_clear_object(&media);
442+ g_clear_object(&menu);
443+ return;
444+}
445+
446+TEST_F(SoundMenuTest, AddRemovePlayerNoPlayNextPrev) {
447+ check_player_control_buttons(false, false, false);
448+}
449+
450+TEST_F(SoundMenuTest, AddRemovePlayerNoNext) {
451+ check_player_control_buttons(true, false, true);
452+}
453+
454+TEST_F(SoundMenuTest, AddRemovePlayerNoPrev) {
455+ check_player_control_buttons(true, true, false);
456+}
457+
458+TEST_F(SoundMenuTest, AddRemovePlayerNoPlay) {
459+ check_player_control_buttons(false, true, true);
460+}
461+
462+//

Subscribers

People subscribed via source and target branches