Merge lp:~cjcurran/indicator-sound/playlist-changed-signal into lp:~indicator-applet-developers/indicator-sound/trunk_3

Proposed by Conor Curran
Status: Merged
Merged at revision: 126
Proposed branch: lp:~cjcurran/indicator-sound/playlist-changed-signal
Merge into: lp:~indicator-applet-developers/indicator-sound/trunk_3
Diff against target: 307 lines (+65/-30)
7 files modified
src/common-defs.h (+2/-0)
src/mpris2-controller.vala (+19/-10)
src/mpris2-interfaces.vala (+3/-0)
src/player-item.vala (+8/-8)
src/playlists-menu-item.vala (+26/-9)
src/transport-menu-item.vala (+3/-3)
vapi/common-defs.vapi (+4/-0)
To merge this branch: bzr merge lp:~cjcurran/indicator-sound/playlist-changed-signal
Reviewer Review Type Date Requested Status
Neil J. Patel (community) Approve
Review via email: mp+49041@code.launchpad.net

Description of the change

Fixes both bugs attached

To post a comment you must log in.
Revision history for this message
Neil J. Patel (njpatel) wrote :

Worried about the timeout, but that's an issue bigger than this merge (something we need to figure out with GDBus), so approving.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/common-defs.h'
2--- src/common-defs.h 2011-01-24 22:26:21 +0000
3+++ src/common-defs.h 2011-02-09 11:18:30 +0000
4@@ -64,4 +64,6 @@
5 #define DBUSMENU_PLAYLISTS_MENUITEM_TITLE "x-canonical-sound-menu-player-playlists-title"
6 #define DBUSMENU_PLAYLISTS_MENUITEM_PLAYLISTS "x-canonical-sound-menu-player-playlists-playlists"
7
8+#define DBUSMENU_PLAYLIST_MENUITEM_PATH "x-canonical-sound-menu-player-playlist-path"
9+
10 #endif
11
12=== modified file 'src/mpris2-controller.vala'
13--- src/mpris2-controller.vala 2011-02-01 19:31:32 +0000
14+++ src/mpris2-controller.vala 2011-02-09 11:18:30 +0000
15@@ -48,6 +48,7 @@
16 this.playlists = Bus.get_proxy_sync ( BusType.SESSION,
17 this.owner.dbus_name,
18 "/org/mpris/MediaPlayer2" );
19+ this.playlists.PlaylistChanged.connect (on_playlistdetails_changed);
20 }
21 this.properties_interface = Bus.get_proxy_sync ( BusType.SESSION,
22 "org.freedesktop.Properties.PropertiesChanged",
23@@ -63,7 +64,7 @@
24 HashTable<string, Variant?> changed_properties,
25 string[] invalid )
26 {
27- debug("properties-changed for interface %s and owner %s", interface_source, this.owner.dbus_name);
28+ //debug("properties-changed for interface %s and owner %s", interface_source, this.owner.dbus_name);
29 if ( changed_properties == null ||
30 interface_source.has_prefix ( MPRIS_PREFIX ) == false ){
31 warning("Property-changed hash is null or this is an interface that doesn't concerns us");
32@@ -91,7 +92,9 @@
33 }
34 Variant? playlist_v = changed_properties.lookup("ActivePlaylist");
35 if ( playlist_v != null && this.owner.use_playlists == true ){
36- this.fetch_active_playlist();
37+ // Once again A GDBus race condition, the property_changed signal is sent
38+ // before the value is set on the respective property.
39+ Timeout.add (300, this.fetch_active_playlist);
40 }
41 Variant? playlist_count_v = changed_properties.lookup("PlaylistCount");
42 if ( playlist_count_v != null && this.owner.use_playlists == true ){
43@@ -109,9 +112,9 @@
44 title.alter_label (this.mpris2_root.Identity);
45 }
46 }
47-
48+
49 private bool ensure_correct_playback_status(){
50- debug("TEST playback status = %s", this.player.PlaybackStatus);
51+ //debug("TEST playback status = %s", this.player.PlaybackStatus);
52 TransportMenuitem.state p = (TransportMenuitem.state)this.determine_play_state(this.player.PlaybackStatus);
53 (this.owner.custom_items[PlayerController.widget_order.TRANSPORT] as TransportMenuitem).change_play_state(p);
54 return false;
55@@ -173,7 +176,7 @@
56
57 public void transport_update(TransportMenuitem.action command)
58 {
59- debug("transport_event input = %i", (int)command);
60+ //debug("transport_event input = %i", (int)command);
61 if(command == TransportMenuitem.action.PLAY_PAUSE){
62 this.player.PlayPause.begin();
63 }
64@@ -185,7 +188,6 @@
65 }
66 }
67
68-
69 public bool connected()
70 {
71 return (this.player != null && this.mpris2_root != null);
72@@ -198,6 +200,12 @@
73 }
74 }
75
76+ private void on_playlistdetails_changed (PlaylistDetails details)
77+ {
78+ PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
79+ playlists_item.update_individual_playlist (details);
80+ }
81+
82 public async void fetch_playlists()
83 {
84 PlaylistDetails[] current_playlists = null;
85@@ -214,7 +222,7 @@
86 }
87
88 if( current_playlists != null ){
89- debug( "Size of the playlist array = %i", current_playlists.length );
90+ //debug( "Size of the playlist array = %i", current_playlists.length );
91 PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
92 playlists_item.update(current_playlists);
93 }
94@@ -224,13 +232,14 @@
95 }
96 }
97
98- private void fetch_active_playlist()
99+ private bool fetch_active_playlist()
100 {
101 if (this.playlists.ActivePlaylist.valid == false){
102- debug("We don't have an active playlist");
103- }
104+ debug(" We don't have an active playlist");
105+ }
106 PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
107 playlists_item.update_active_playlist ( this.playlists.ActivePlaylist.details );
108+ return false;
109 }
110
111 public void activate_playlist (ObjectPath path)
112
113=== modified file 'src/mpris2-interfaces.vala'
114--- src/mpris2-interfaces.vala 2011-01-27 23:08:08 +0000
115+++ src/mpris2-interfaces.vala 2011-02-09 11:18:30 +0000
116@@ -72,4 +72,7 @@
117 uint32 max_count,
118 string order,
119 bool reverse_order ) throws IOError;
120+ //signals
121+ public signal void PlaylistChanged (PlaylistDetails details);
122+
123 }
124\ No newline at end of file
125
126=== modified file 'src/player-item.vala'
127--- src/player-item.vala 2010-12-20 16:55:31 +0000
128+++ src/player-item.vala 2011-02-09 11:18:30 +0000
129@@ -52,19 +52,19 @@
130 {
131 debug("PlayerItem::update()");
132 if(data == null){
133- debug("PlayerItem::Update -> The hashtable was null - just leave it!");
134+ warning("PlayerItem::Update -> The hashtable was null - just leave it!");
135 return;
136 }
137
138 foreach(string property in attributes){
139 string[] input_keys = property.split("-");
140 string search_key = input_keys[input_keys.length-1 : input_keys.length][0];
141- debug("search key = %s", search_key);
142+ //debug("search key = %s", search_key);
143 Variant? v = data.lookup(search_key);
144
145 if (v.is_of_type ( VariantType.STRING )){
146 string update = v.get_string().strip();
147- debug("with value : %s", update);
148+ //debug("with value : %s", update);
149 if(property.contains("mpris:artUrl")){
150 // We know its a metadata instance because thats the only
151 // object with the arturl prop
152@@ -75,15 +75,15 @@
153 this.property_set(property, update);
154 }
155 else if (v.is_of_type (VariantType.INT32 )){
156- debug("with value : %i", v.get_int32());
157+ //debug("with value : %i", v.get_int32());
158 this.property_set_int(property, v.get_int32());
159 }
160 else if (v.is_of_type (VariantType.INT64 )){
161- debug("with value : %i", (int)v.get_int64());
162+ //debug("with value : %i", (int)v.get_int64());
163 this.property_set_int(property, (int)v.get_int64());
164 }
165 else if(v.is_of_type ( VariantType.BOOLEAN )){
166- debug("with value : %s", v.get_boolean().to_string());
167+ //debug("with value : %s", v.get_boolean().to_string());
168 this.property_set_bool(property, v.get_boolean());
169 }
170 }
171@@ -93,10 +93,10 @@
172 public bool populated(HashSet<string> attrs)
173 {
174 foreach(string prop in attrs){
175- debug("populated ? - prop: %s", prop);
176+ //debug("populated ? - prop: %s", prop);
177 int value_int = property_get_int(prop);
178 if(property_get_int(prop) != EMPTY){
179- debug("populated - prop %s and value %i", prop, value_int);
180+ //debug("populated - prop %s and value %i", prop, value_int);
181 return true;
182 }
183 }
184
185=== modified file 'src/playlists-menu-item.vala'
186--- src/playlists-menu-item.vala 2011-02-01 01:33:46 +0000
187+++ src/playlists-menu-item.vala 2011-02-09 11:18:30 +0000
188@@ -19,18 +19,20 @@
189
190 using Dbusmenu;
191 using DbusmenuPlaylists;
192+using DbusmenuPlaylist;
193 using Gee;
194
195 public class PlaylistsMenuitem : PlayerItem
196 {
197- private HashMap<int, PlaylistDetails?> current_playlists;
198+ private HashMap<int, Dbusmenu.Menuitem> current_playlists;
199 public Menuitem root_item;
200+
201 public PlaylistsMenuitem ( PlayerController parent )
202 {
203 Object ( item_type: MENUITEM_TYPE, owner: parent );
204 }
205 construct{
206- this.current_playlists = new HashMap<int, PlaylistDetails?>();
207+ this.current_playlists = new HashMap<int, Dbusmenu.Menuitem>();
208 this.root_item = new Menuitem();
209 this.root_item.property_set ( MENUITEM_PROP_LABEL, "Choose Playlist" );
210 }
211@@ -38,23 +40,38 @@
212 public new void update (PlaylistDetails[] playlists)
213 {
214 foreach ( PlaylistDetails detail in playlists ){
215+
216 if (this.already_observed(detail)) continue;
217+
218 Dbusmenu.Menuitem menuitem = new Menuitem();
219 menuitem.property_set (MENUITEM_PROP_LABEL, detail.name);
220- menuitem.property_set (MENUITEM_PROP_ICON_NAME, "source-smart-playlist");
221+ menuitem.property_set (MENUITEM_PROP_ICON_NAME, detail.icon_path);
222+ menuitem.property_set (MENUITEM_PATH, (string)detail.path);
223 menuitem.property_set_bool (MENUITEM_PROP_VISIBLE, true);
224 menuitem.property_set_bool (MENUITEM_PROP_ENABLED, true);
225- this.current_playlists.set( menuitem.id, detail );
226+
227 menuitem.item_activated.connect(() => {
228 submenu_item_activated (menuitem.id );});
229+ this.current_playlists.set( menuitem.id, menuitem );
230 this.root_item.child_append( menuitem );
231 }
232 }
233-
234+
235+ public void update_individual_playlist (PlaylistDetails new_detail)
236+ {
237+ foreach ( Dbusmenu.Menuitem item in this.current_playlists.values ){
238+ if (new_detail.path == item.property_get (MENUITEM_PATH)){
239+ item.property_set (MENUITEM_PROP_LABEL, new_detail.name);
240+ item.property_set (MENUITEM_PROP_ICON_NAME, new_detail.icon_path);
241+ }
242+ }
243+ }
244+
245 private bool already_observed (PlaylistDetails new_detail)
246 {
247- foreach ( PlaylistDetails detail in this.current_playlists.values ){
248- if (new_detail.path == detail.path) return true;
249+ foreach ( Dbusmenu.Menuitem item in this.current_playlists.values ){
250+ var path = item.property_get (MENUITEM_PATH);
251+ if (new_detail.path == path) return true;
252 }
253 return false;
254 }
255@@ -68,12 +85,12 @@
256
257 private void submenu_item_activated (int menu_item_id)
258 {
259- if(!this.current_playlists.has_key(menu_item_id)){
260+ if (!this.current_playlists.has_key(menu_item_id)) {
261 warning( "item %i was activated but we don't have a corresponding playlist",
262 menu_item_id );
263 return;
264 }
265- this.owner.mpris_bridge.activate_playlist ( this.current_playlists[menu_item_id].path );
266+ this.owner.mpris_bridge.activate_playlist ( (GLib.ObjectPath)this.current_playlists[menu_item_id].property_get (MENUITEM_PATH) );
267 }
268
269 public static HashSet<string> attributes_format()
270
271=== modified file 'src/transport-menu-item.vala'
272--- src/transport-menu-item.vala 2011-01-25 20:55:25 +0000
273+++ src/transport-menu-item.vala 2011-02-09 11:18:30 +0000
274@@ -42,8 +42,8 @@
275
276 public void change_play_state(state update)
277 {
278- debug("UPDATING THE TRANSPORT DBUSMENUITEM PLAY STATE WITH VALUE %i",
279- (int)update);
280+ //debug("UPDATING THE TRANSPORT DBUSMENUITEM PLAY STATE WITH VALUE %i",
281+ // (int)update);
282 int temp = (int)update;
283 this.property_set_int(MENUITEM_PLAY_STATE, temp);
284 }
285@@ -60,7 +60,7 @@
286 }
287
288 int32 input = v.get_int32();
289- debug("transport menu item -> handle_event with value %s", input.to_string());
290+ //debug("transport menu item -> handle_event with value %s", input.to_string());
291 //debug("transport owner name = %s", this.owner.app_info.get_name());
292 this.owner.mpris_bridge.transport_update((action)input);
293 }
294
295=== modified file 'vapi/common-defs.vapi'
296--- vapi/common-defs.vapi 2010-12-09 11:36:33 +0000
297+++ vapi/common-defs.vapi 2011-02-09 11:18:30 +0000
298@@ -53,4 +53,8 @@
299 public const string MENUITEM_TYPE;
300 public const string MENUITEM_TITLE;
301 public const string MENUITEM_PLAYLISTS;
302+}
303+[CCode (cheader_filename = "common-defs.h")]
304+namespace DbusmenuPlaylist{
305+ public const string MENUITEM_PATH;
306 }
307\ No newline at end of file

Subscribers

People subscribed via source and target branches