listen out for the playlistdetails changed signal and accommodate the race condition in gdbus for the active playlist property changed issue

7 files changed, 65 insertions, 30 deletions

diff --git a/src/common-defs.h b/src/common-defs.h
index 5458dc5..63d9d40 100644
--- a/src/common-defs.h
+++ b/src/common-defs.h
@@ -64,4 +64,6 @@ typedef enum {
 #define DBUSMENU_PLAYLISTS_MENUITEM_TITLE       "x-canonical-sound-menu-player-playlists-title"
 #define DBUSMENU_PLAYLISTS_MENUITEM_PLAYLISTS   "x-canonical-sound-menu-player-playlists-playlists"
 
+#define DBUSMENU_PLAYLIST_MENUITEM_PATH         "x-canonical-sound-menu-player-playlist-path"
+
 #endif
diff --git a/src/mpris2-controller.vala b/src/mpris2-controller.vala
index d4cdc0c..bf930fc 100644
--- a/src/mpris2-controller.vala
+++ b/src/mpris2-controller.vala
@@ -48,6 +48,7 @@ public class Mpris2Controller : GLib.Object
         this.playlists = Bus.get_proxy_sync ( BusType.SESSION,
                                               this.owner.dbus_name,
                                               "/org/mpris/MediaPlayer2" );
+        this.playlists.PlaylistChanged.connect (on_playlistdetails_changed);
       }
       this.properties_interface = Bus.get_proxy_sync ( BusType.SESSION,
                                                        "org.freedesktop.Properties.PropertiesChanged",
@@ -63,7 +64,7 @@ public class Mpris2Controller : GLib.Object
                                     HashTable<string, Variant?> changed_properties,
                                     string[] invalid )
   {
-    debug("properties-changed for interface %s and owner %s", interface_source, this.owner.dbus_name);
+    //debug("properties-changed for interface %s and owner %s", interface_source, this.owner.dbus_name);
     if ( changed_properties == null ||
         interface_source.has_prefix ( MPRIS_PREFIX ) == false ){
       warning("Property-changed hash is null or this is an interface that doesn't concerns us");
@@ -91,7 +92,9 @@ public class Mpris2Controller : GLib.Object
     }
     Variant? playlist_v = changed_properties.lookup("ActivePlaylist");
     if ( playlist_v != null && this.owner.use_playlists == true ){
-      this.fetch_active_playlist();
+      // Once again A GDBus race condition, the property_changed signal is sent
+      // before the value is set on the respective property.
+      Timeout.add (300, this.fetch_active_playlist);
     }
     Variant? playlist_count_v = changed_properties.lookup("PlaylistCount");
     if ( playlist_count_v != null && this.owner.use_playlists == true ){
@@ -109,9 +112,9 @@ public class Mpris2Controller : GLib.Object
       title.alter_label (this.mpris2_root.Identity);
     }
   }
-  
+                                      
   private bool ensure_correct_playback_status(){
-    debug("TEST playback status = %s", this.player.PlaybackStatus);
+    //debug("TEST playback status = %s", this.player.PlaybackStatus);
     TransportMenuitem.state p = (TransportMenuitem.state)this.determine_play_state(this.player.PlaybackStatus);
     (this.owner.custom_items[PlayerController.widget_order.TRANSPORT] as TransportMenuitem).change_play_state(p);
     return false;
@@ -173,7 +176,7 @@ public class Mpris2Controller : GLib.Object
 
   public void transport_update(TransportMenuitem.action command)
   {
-    debug("transport_event input = %i", (int)command);
+    //debug("transport_event input = %i", (int)command);
     if(command == TransportMenuitem.action.PLAY_PAUSE){
       this.player.PlayPause.begin();              
     }
@@ -185,7 +188,6 @@ public class Mpris2Controller : GLib.Object
     }
   }
 
-
   public bool connected()
   {
     return (this.player != null && this.mpris2_root != null);
@@ -198,6 +200,12 @@ public class Mpris2Controller : GLib.Object
     }
   }
 
+  private void on_playlistdetails_changed (PlaylistDetails details)
+  {
+    PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
+    playlists_item.update_individual_playlist (details);    
+  }
+
   public async void fetch_playlists()
   {
     PlaylistDetails[] current_playlists = null;
@@ -214,7 +222,7 @@ public class Mpris2Controller : GLib.Object
     }
     
     if( current_playlists != null ){
-      debug( "Size of the playlist array = %i", current_playlists.length );
+      //debug( "Size of the playlist array = %i", current_playlists.length );
       PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
       playlists_item.update(current_playlists);
     }
@@ -224,13 +232,14 @@ public class Mpris2Controller : GLib.Object
     }
   }
 
-  private void fetch_active_playlist()
+  private bool fetch_active_playlist()
   {    
     if (this.playlists.ActivePlaylist.valid == false){
-      debug("We don't have an active playlist");
-    }
+      debug(" We don't have an active playlist");
+    }    
     PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
     playlists_item.update_active_playlist ( this.playlists.ActivePlaylist.details );
+    return false;
   }
 
   public void activate_playlist (ObjectPath path)
diff --git a/src/mpris2-interfaces.vala b/src/mpris2-interfaces.vala
index 0a0909f..5506a47 100644
--- a/src/mpris2-interfaces.vala
+++ b/src/mpris2-interfaces.vala
@@ -72,4 +72,7 @@ public interface MprisPlaylists : Object {
                                                           uint32 max_count,
                                                           string order,
                                                           bool reverse_order ) throws IOError;
+  //signals
+  public signal void PlaylistChanged (PlaylistDetails details);
+  
 }
\ No newline at end of file
diff --git a/src/player-item.vala b/src/player-item.vala
index f883a1e..e146d4a 100644
--- a/src/player-item.vala
+++ b/src/player-item.vala
@@ -52,19 +52,19 @@ public class PlayerItem : Dbusmenu.Menuitem
   {
     debug("PlayerItem::update()");
     if(data == null){
-      debug("PlayerItem::Update -> The hashtable was null - just leave it!");
+      warning("PlayerItem::Update -> The hashtable was null - just leave it!");
       return;
     }
     
     foreach(string property in attributes){
       string[] input_keys = property.split("-");
       string search_key = input_keys[input_keys.length-1 : input_keys.length][0];
-      debug("search key = %s", search_key);
+      //debug("search key = %s", search_key);
       Variant? v = data.lookup(search_key);
       
       if (v.is_of_type ( VariantType.STRING )){
         string update = v.get_string().strip();
-        debug("with value : %s", update);
+        //debug("with value : %s", update);
         if(property.contains("mpris:artUrl")){    
             // We know its a metadata instance because thats the only
             // object with the arturl prop            
@@ -75,15 +75,15 @@ public class PlayerItem : Dbusmenu.Menuitem
         this.property_set(property, update);                      
       }         
       else if (v.is_of_type (VariantType.INT32 )){
-        debug("with value : %i", v.get_int32());
+        //debug("with value : %i", v.get_int32());
         this.property_set_int(property, v.get_int32());
       }
       else if (v.is_of_type (VariantType.INT64 )){
-        debug("with value : %i", (int)v.get_int64());
+        //debug("with value : %i", (int)v.get_int64());
         this.property_set_int(property, (int)v.get_int64());
       }
       else if(v.is_of_type ( VariantType.BOOLEAN )){
-        debug("with value : %s", v.get_boolean().to_string());        
+        //debug("with value : %s", v.get_boolean().to_string());        
         this.property_set_bool(property, v.get_boolean());
       }
     }
@@ -93,10 +93,10 @@ public class PlayerItem : Dbusmenu.Menuitem
   public bool populated(HashSet<string> attrs)
   {
     foreach(string prop in attrs){
-      debug("populated ? - prop: %s", prop);
+      //debug("populated ? - prop: %s", prop);
       int value_int = property_get_int(prop);
       if(property_get_int(prop) != EMPTY){
-        debug("populated - prop %s and value %i", prop, value_int);        
+        //debug("populated - prop %s and value %i", prop, value_int);        
         return true;
       }
     }
diff --git a/src/playlists-menu-item.vala b/src/playlists-menu-item.vala
index b8c6e7d..8a2ccac 100644
--- a/src/playlists-menu-item.vala
+++ b/src/playlists-menu-item.vala
@@ -19,18 +19,20 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 using Dbusmenu;
 using DbusmenuPlaylists;
+using DbusmenuPlaylist;
 using Gee;
 
 public class PlaylistsMenuitem : PlayerItem
 {
-  private HashMap<int, PlaylistDetails?> current_playlists;    
+  private HashMap<int, Dbusmenu.Menuitem> current_playlists;    
   public Menuitem root_item;
+  
   public PlaylistsMenuitem ( PlayerController parent )
   {
     Object ( item_type: MENUITEM_TYPE, owner: parent );
   }
   construct{
-    this.current_playlists = new HashMap<int, PlaylistDetails?>();
+    this.current_playlists = new HashMap<int, Dbusmenu.Menuitem>();
     this.root_item = new Menuitem();
     this.root_item.property_set ( MENUITEM_PROP_LABEL, "Choose Playlist" );
   }
@@ -38,23 +40,38 @@ public class PlaylistsMenuitem : PlayerItem
   public new void update (PlaylistDetails[] playlists)
   {
     foreach ( PlaylistDetails detail in playlists ){
+      
       if (this.already_observed(detail)) continue;
+      
       Dbusmenu.Menuitem menuitem = new Menuitem();
       menuitem.property_set (MENUITEM_PROP_LABEL, detail.name);
-      menuitem.property_set (MENUITEM_PROP_ICON_NAME, "source-smart-playlist");
+      menuitem.property_set (MENUITEM_PROP_ICON_NAME, detail.icon_path);
+      menuitem.property_set (MENUITEM_PATH, (string)detail.path);
       menuitem.property_set_bool (MENUITEM_PROP_VISIBLE, true);
       menuitem.property_set_bool (MENUITEM_PROP_ENABLED, true);
-      this.current_playlists.set( menuitem.id, detail );
+      
       menuitem.item_activated.connect(() => {
         submenu_item_activated (menuitem.id );});
+      this.current_playlists.set( menuitem.id, menuitem );      
       this.root_item.child_append( menuitem );
     }
   }
-  
+
+  public void update_individual_playlist (PlaylistDetails new_detail)
+  {
+    foreach ( Dbusmenu.Menuitem item in this.current_playlists.values ){
+      if (new_detail.path == item.property_get (MENUITEM_PATH)){
+        item.property_set (MENUITEM_PROP_LABEL, new_detail.name);
+        item.property_set (MENUITEM_PROP_ICON_NAME, new_detail.icon_path);      
+      }
+    }
+  }
+                                                  
   private bool already_observed (PlaylistDetails new_detail)
   {
-    foreach ( PlaylistDetails detail in this.current_playlists.values ){
-      if (new_detail.path == detail.path) return true;
+    foreach ( Dbusmenu.Menuitem item in this.current_playlists.values ){
+      var path = item.property_get (MENUITEM_PATH);
+      if (new_detail.path == path) return true;
     }
     return false;
   }
@@ -68,12 +85,12 @@ public class PlaylistsMenuitem : PlayerItem
   
   private void submenu_item_activated (int menu_item_id)
   {
-    if(!this.current_playlists.has_key(menu_item_id)){
+    if (!this.current_playlists.has_key(menu_item_id)) {
       warning( "item %i was activated but we don't have a corresponding playlist",
                menu_item_id );
       return;
     }
-    this.owner.mpris_bridge.activate_playlist ( this.current_playlists[menu_item_id].path );
+    this.owner.mpris_bridge.activate_playlist ( (GLib.ObjectPath)this.current_playlists[menu_item_id].property_get (MENUITEM_PATH) );
   }
   
   public static HashSet<string> attributes_format()
diff --git a/src/transport-menu-item.vala b/src/transport-menu-item.vala
index b8f55b5..b0009d9 100644
--- a/src/transport-menu-item.vala
+++ b/src/transport-menu-item.vala
@@ -42,8 +42,8 @@ public class TransportMenuitem : PlayerItem
 
   public void change_play_state(state update)
   {
-    debug("UPDATING THE TRANSPORT DBUSMENUITEM PLAY STATE WITH VALUE %i",
-          (int)update);
+    //debug("UPDATING THE TRANSPORT DBUSMENUITEM PLAY STATE WITH VALUE %i",
+    //      (int)update);
     int temp = (int)update;
     this.property_set_int(MENUITEM_PLAY_STATE, temp); 
   }
@@ -60,7 +60,7 @@ public class TransportMenuitem : PlayerItem
     }
     
     int32 input = v.get_int32();
-    debug("transport menu item -> handle_event with value %s", input.to_string());
+    //debug("transport menu item -> handle_event with value %s", input.to_string());
     //debug("transport owner name = %s", this.owner.app_info.get_name());
     this.owner.mpris_bridge.transport_update((action)input);
   } 
diff --git a/vapi/common-defs.vapi b/vapi/common-defs.vapi
index 2946d25..7b38e9c 100644
--- a/vapi/common-defs.vapi
+++ b/vapi/common-defs.vapi
@@ -53,4 +53,8 @@ namespace DbusmenuPlaylists{
         public const string MENUITEM_TYPE;
         public const string MENUITEM_TITLE;
         public const string MENUITEM_PLAYLISTS;
+}
+[CCode (cheader_filename = "common-defs.h")]
+namespace DbusmenuPlaylist{
+        public const string MENUITEM_PATH;
 }
\ No newline at end of file