Merge lp:~cando/zeitgeist/storage-monitor into lp:~zeitgeist/zeitgeist/bluebird

[Stefano Candori]

In this branch i've ported the support for "net-storages" in the storage-monitor.

[Siegfried Gevatter (rainct)]

Hey,

Good job, thanks for working on this.

Some comments:

> (this.network != null) { ... }
What's this supposed to do? You've already unwatched everything in the "else { ... }", so you can just return.

> this.network.setup ();
I'm not too happy about this. Hm. Let's leave it like this, but at least add a comment to the abstract setup() document that it will call on_network_up/on_network_up with the initial state of the network.

> public interface NetworkMonitor: Object
Shouldn't this be private? It's not supposed to be used anywhere outside the extension.

> Write connectivity to the DB.
Not sure what this comment is supposed to mean.

> private void name_appeared_handler(DBusConnection connection, string name, string name_owner)
You're missing a space between "handler (".

> Checks whether there is a funtioning
Typo (funtioning -> functioning). I'd reword it to something like "Monitor the availability of working network connections using Network Manager (0.8 or later)".

> // ^^ There is a bug in some Connman versions causing it to not emit the
This comment is about the proxy.state_changed.connect, but the position where it's places make it look like it's proxy.get_state.

review: Needs Fixing

[lp:~cando/zeitgeist/storage-monitor]

341. By Stefano Candori on 2011-12-03

Fixes basing on RainCT's review.

=== modified file 'extensions/storage-monitor.vala'
--- extensions/storage-monitor.vala	2011-09-30 09:35:12 +0000
+++ extensions/storage-monitor.vala	2011-12-03 14:26:26 +0000
@@ -2,6 +2,7 @@
  *
  * Copyright © 2011 Collabora Ltd.
  *             By Siegfried-Angel Gevatter Pujals <[e-mail]>
+ * Copyright © 2011 Stefano Candori <[e-mail]>
  *
  * Based upon a Python implementation:
  *  Copyright © 2009 Mikkel Kamstrup Erlandsen <[e-mail]>
@@ -36,6 +37,16 @@
             [DBus (signature = "a{sv}")] Variant storage_description);
         public signal void storage_unavailable (string storage_id);
     }
+    
+    private interface NetworkMonitor: Object
+    { 
+        // This method emits the on_network_up/on_network_up signals
+        // basing on the initial state of the network.
+        public abstract void setup ();
+        
+        public signal void on_network_up ();
+        public signal void on_network_down ();
+    }
 
     namespace StorageMedia
     {
@@ -97,6 +108,10 @@
         private Sqlite.Statement store_storage_medium_stmt;
         private Sqlite.Statement insert_unavailable_medium_stmt;
         private Sqlite.Statement update_medium_state_stmt;
+        
+        private NetworkMonitor network;
+        private uint watch_connman;
+        private uint watch_nm;
 
         StorageMonitor ()
         {
@@ -136,9 +151,41 @@
                     volume.get_icon ().to_string (), volume.get_name ());
             }
 
-            // FIXME: ConnMan / NetworkManager D-Bus stuff...
-        }
+            // Dynamically decide whether to use Connman or NetworkManager
+            watch_connman = Bus.watch_name (BusType.SYSTEM, 
+                                      "net.connman", 
+                                      BusNameWatcherFlags.NONE,
+                                      name_appeared_handler,
+                                      null);
+            watch_nm = Bus.watch_name (BusType.SYSTEM, 
+                                      "org.freedesktop.NetworkManager", 
+                                      BusNameWatcherFlags.NONE,
+                                      name_appeared_handler,
+                                      null);
 
+        }
+        
+        private void name_appeared_handler (DBusConnection connection, string name, string name_owner)
+        {
+            if (this.network != null)
+                return;
+              
+            if (name == "net.connman")
+                this.network = new ConnmanNetworkMonitor ();
+            else if (name == "org.freedesktop.NetworkManager")
+                this.network = new NMNetworkMonitor ();
+                
+            this.network.on_network_up.connect (() => 
+                this.add_storage_medium ("net", "stock_internet", "Internet"));
+            this.network.on_network_down.connect (() => 
+                this.remove_storage_medium ("net"));
+                
+            this.network.setup ();
+                
+            Bus.unwatch_name (watch_connman);
+            Bus.unwatch_name (watch_nm);            
+        }
+        
         public override void unload ()
         {
             // FIXME: move all this D-Bus stuff to some shared
@@ -228,7 +275,7 @@
             return "unknown";
         }
 
-        private void on_volume_added (VolumeMonitor monitor, Volume volume)
+        private void on_volume_added (Volume volume)
         {
             debug ("volume added");
             Icon icon = volume.get_icon ();
@@ -240,7 +287,7 @@
                 volume.get_name ());
         }
 
-        private void on_volume_removed (VolumeMonitor monitor, Volume volume)
+        private void on_volume_removed (Volume volume)
         {
             debug ("Volume removed");
             remove_storage_medium (get_volume_id (volume));
@@ -329,6 +376,105 @@
 
     }
 
+    /*
+     * Monitor the availability of working network connections using Network Manager 
+     * (requires 0.8 or later).	
+     * See http://projects.gnome.org/NetworkManager/developers/spec-08.html
+     */
+    class NMNetworkMonitor : Object, NetworkMonitor
+    {
+        private const string NM_BUS_NAME = "org.freedesktop.NetworkManager";
+        private const string NM_IFACE = "org.freedesktop.NetworkManager";
+        private const string NM_OBJECT_PATH = "/org/freedesktop/NetworkManager";
+
+        //NM 0.9 broke API so we have to check for two possible values for the state
+        private const int NM_STATE_CONNECTED_PRE_09 = 3;
+        private const int NM_STATE_CONNECTED_POST_09 = 70;
+        
+        private NetworkManagerDBus proxy;
+        
+        public NMNetworkMonitor ()
+        {
+            Object ();
+        }
+        
+        public void setup ()
+        {
+            debug ("Creating NetworkManager network monitor");
+            try
+            {
+                proxy = Bus.get_proxy_sync<NetworkManagerDBus> (BusType.SYSTEM,
+                                            NM_BUS_NAME,
+                                            NM_OBJECT_PATH);
+                proxy.state_changed.connect (this.on_state_changed);
+            
+                uint32 state = proxy.state ();
+                this.on_state_changed (state);
+            }
+            catch (IOError e )
+            {
+                warning ("%s", e.message);
+            }
+        }
+        
+        private void on_state_changed(uint32 state)
+        {
+            debug ("NetworkManager network state: %u", state);
+            if (state == NMNetworkMonitor.NM_STATE_CONNECTED_PRE_09 || 
+                state == NMNetworkMonitor.NM_STATE_CONNECTED_POST_09)
+                on_network_up ();
+            else
+                on_network_down ();
+        }
+    }
+    
+    class ConnmanNetworkMonitor : Object, NetworkMonitor
+    {
+        private const string CM_BUS_NAME = "net.connman";
+        private const string CM_IFACE = "net.connman.Manager";
+        private const string CM_OBJECT_PATH = "/";
+        
+        private ConnmanManagerDBus proxy;
+        
+        public ConnmanNetworkMonitor ()
+        {
+            Object ();
+        }
+        
+        public void setup ()
+        {
+            debug ("Creating ConnmanNetworkManager network monitor");
+
+            try
+            {
+                proxy = Bus.get_proxy_sync<ConnmanManagerDBus> (BusType.SYSTEM,
+                                            CM_BUS_NAME,
+                                            CM_OBJECT_PATH);
+                
+                // There is a bug in some Connman versions causing it to not emit the
+                // net.connman.Manager.StateChanged signal. We take our chances this
+                // instance is working properly :-)     
+                proxy.state_changed.connect (this.on_state_changed);
+                
+                string state = proxy.get_state ();
+                this.on_state_changed (state);
+            }
+            catch (IOError e )
+            {
+                warning ("%s", e.message);
+            }
+        }
+        
+        private void on_state_changed(string state)
+        {
+            debug ("ConnmanNetworkMonitor network state: %s", state);
+            if (state == "online")
+                on_network_up ();
+            else
+                on_network_down ();
+        }
+    }
+
     [ModuleInit]
 #if BUILTIN_EXTENSIONS
     public static Type storage_monitor_init (TypeModule module)

=== modified file 'src/remote.vala'
--- src/remote.vala	2011-10-12 21:13:12 +0000
+++ src/remote.vala	2011-12-03 14:26:26 +0000
@@ -121,6 +121,23 @@
             [DBus (signature = "a(asaasay)")] Variant filter_templates,
             uint offset, uint count, uint result_type) throws Error;
     }
+    
+    /* FIXME: Remove this! Only here because of a bug in Vala (see ext-fts) */
+    [DBus (name = "org.freedesktop.NetworkManager")]
+    public interface NetworkManagerDBus : Object
+    {
+        [DBus (name = "state")]
+        public abstract uint32 state () throws IOError;
+        public signal void state_changed (uint32 state);
+    }
+    
+    /* FIXME: Remove this! Only here because of a bug in Vala (see ext-fts) */
+    [DBus (name = "net.connman.Manager")]
+    public interface ConnmanManagerDBus : Object
+    {
+        public abstract string get_state () throws IOError;
+        public signal void state_changed (string state);
+    }
 
 }