Merge lp:~donadigo/switchboard-plug-networking/wifi-radiobuttons into lp:~elementary-pantheon/switchboard-plug-networking/trunk

[Adam Bieńkowski]

* Add new radiobuttons to WiFi list instead of setting the text on the item.
* get_strenth_image replaced with get_strength_icon
* Strength boundaries are now taken from Indicator Network
* Update strength when it changes
* Add spinner that signals that the connection is in progress
* RSN flags now also for checking if the entry is secured.

* Add new radiobuttons to WiFi list instead of setting the text on the item.
* get_strenth_image replaced with get_strength_icon
* Strength boundaries are now taken from Indicator Network
* Update strength when it changes
* Add spinner that signals that the connection is in progress
* RSN flags now also for checking if the entry is secured.

[Danielle Foré (danrabbit)]

Hm, it seems I'm now unable to connect to networks that require a password.

Also the spinner only shows while the password dialog is open, but then disappears and the connection never happens. Doesn't even seem like it tries to connect.

review: Needs Fixing

[xapantu (xapantu)]

There is definitively a problem with the spinner, it does not show as often as the spinner in the indicator.

I also have the problem with the password protected networks, a password is asked even if it is already registered.

review: Needs Fixing

[xapantu (xapantu)]

So, I wrote more inline comments. Some are maybe arguable. You don't need to fix every item marked as "not important", but that would be better if the other were :)

Other than that, it is great to see that this plug is quickly progressing :)

review: Needs Fixing

[Adam Bieńkowski (donadigo)]

xapantu: Okay, I fixed some problems with this branch, there is only one dumb_btn created in the wifi page as you pointed out. I've also borrowed some of yours code to check if entry is connecting and showing a spinner :) I updated those casts, but I dunno about removing "private" stuff, is there anything about this in elementary code style or we are free with those? I will make separate branch for code style updates soon.

[Adam Bieńkowski (donadigo)]

Ah, I forgot, I think the problem with connecting to secured entry is also gone now by these changes.

[xapantu (xapantu)]

About the "private" keyword thing, that is just an opinion, I don't really care :)

[xapantu (xapantu)]

The spinner is now shown too often: if my computer attempts to connect to a network, and then to another, two spinner are displayed. Connecting to a secured network is not fixed (however I did not check wether it worked before your branch or not).

[xapantu (xapantu)]

Okay, I just tested, this bug is not introduced by this branch, don't worry about then.

[xapantu (xapantu)]

After some discussions we decided to merge some code between the indicator and the plug, so I just merged this branch to lp:~elementary-apps/switchboard-plug-networking/merge-with-indicator where these radio buttons are going to be merged with the indicator ones.

[Adam Bieńkowski (donadigo)]

The branch will no longer be merged to the trunk as of the new branch that has merged this one.

Unmerged revisions

=== modified file 'src/Widgets/WiFi/WiFiEntry.vala'
--- src/Widgets/WiFi/WiFiEntry.vala	2015-07-23 20:01:49 +0000
+++ src/Widgets/WiFi/WiFiEntry.vala	2015-08-01 18:42:17 +0000
@@ -22,65 +22,72 @@
 
 namespace Network.Widgets {
     public class WiFiEntry : Gtk.ListBoxRow {
-        public NM.AccessPoint? ap;
-        public string ssid;
+        public NM.AccessPoint ap;
+        public Gtk.RadioButton radio_btn;
+        public string ssid_str;
         public bool is_secured = false;
         public uint strength;
 
+        private Gtk.Box hbox;
+        private Gtk.Spinner spinner;
+
         private string bssid;
 
-        private Gtk.Label title;
-
-        public WiFiEntry.from_access_point (NM.AccessPoint? point) {
+        public WiFiEntry (NM.AccessPoint point, Gtk.RadioButton? previous_btn = null) {
             ap = point;
 
-            this.ssid = NM.Utils.ssid_to_utf8 (ap.get_ssid ());
+            this.ssid_str = NM.Utils.ssid_to_utf8 (ap.get_ssid ());
             this.bssid = ap.get_bssid ();
             this.strength = ap.get_strength ();
 
-            title = new Gtk.Label (ssid);
-            title.halign = Gtk.Align.START;
-
-            var hbox = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0);
-            hbox.add (title);
-
-            hbox.pack_end (get_strength_image (), false, false, 7);
-            if (ap.get_wpa_flags () != [e-mail]) {
+            radio_btn = new Gtk.RadioButton.with_label_from_widget (previous_btn, ssid_str);
+            radio_btn.halign = Gtk.Align.START;
+
+            hbox = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0);
+            hbox.add (radio_btn);
+
+            var strength_image = new Gtk.Image.from_icon_name (get_strength_icon (), Gtk.IconSize.SMALL_TOOLBAR);
+            ap.notify["strength"].connect (() => {
+               strength_image.icon_name = get_strength_icon ();
+            });
+
+            hbox.pack_end (strength_image, false, false, 7);
+            if (ap.get_wpa_flags () != [e-mail] || ap.get_rsn_flags () != [e-mail]) {
                 is_secured = true;
 
                 var lock_img = new Gtk.Image.from_icon_name ("channel-secure-symbolic", Gtk.IconSize.SMALL_TOOLBAR);
                 hbox.pack_end (lock_img, false, false, 0);
             }
 
+            spinner = new Gtk.Spinner ();
+            spinner.no_show_all = true;
+            spinner.start ();
+            hbox.pack_end (spinner, false, false, 0);
+
+            set_connection_in_progress (false);
+
             this.add (hbox);
             this.show_all ();
         }
 
-        public void set_status_point (bool connected, bool in_process) {
-            if (connected || in_process) {
-                string status = Utils.state_to_string (NM.DeviceState.ACTIVATED);
-                if (in_process)
-                    status = Utils.state_to_string (NM.DeviceState.CONFIG);
-                title.label = title.get_label () + SUFFIX + "(" + status + ")";
+        public void set_active (bool connected) {
+            radio_btn.active = connected;
+        }
+
+        public void set_connection_in_progress (bool progress) {
+            spinner.visible = progress;
+        }
+
+        private string get_strength_icon () {
+            if (strength < 30) {
+                return "network-wireless-signal-weak-symbolic";
+            } else if (strength < 55) {
+                return "network-wireless-signal-ok-symbolic";
+            } else if (strength < 80) {
+                return "network-wireless-signal-good-symbolic";
             } else {
-                title.label = ssid;
-            }
-        }
-
-        private Gtk.Image get_strength_image () {
-            var image = new Gtk.Image.from_icon_name ("network-wireless-offline-symbolic", Gtk.IconSize.SMALL_TOOLBAR);
-
-            if (strength == 0 || strength <= 25) {
-                image.icon_name = "network-wireless-signal-weak-symbolic";
-            } else if (strength > 25 && strength <= 50) {
-                image.icon_name = "network-wireless-signal-ok-symbolic";
-            } else if (strength > 50 && strength <= 75) {
-                image.icon_name = "network-wireless-signal-good-symbolic";
-            } else if (strength > 75) {
-                image.icon_name = "network-wireless-signal-excellent-symbolic";
-            }
-
-            return image;
+                return "network-wireless-signal-excellent-symbolic";
+            }
         }
     }
 }

=== modified file 'src/Widgets/WiFi/WiFiPage.vala'
--- src/Widgets/WiFi/WiFiPage.vala	2015-07-24 17:42:48 +0000
+++ src/Widgets/WiFi/WiFiPage.vala	2015-08-01 18:42:17 +0000
@@ -25,6 +25,8 @@
         public new NM.DeviceWifi device;
         private Gtk.ListBox wifi_list;
         private List<WiFiEntry> entries;
+        private Gtk.RadioButton dumb_btn;
+        private Gtk.RadioButton previous_btn;
         private const string[] BLACKLISTED = { "Free Public WiFi" };
         
         private WiFiEntry? current_connecting_entry = null;
@@ -40,6 +42,8 @@
             this.init (wifidevice, info_box);
 
             entries = new List<WiFiEntry> ();
+            dumb_btn = new Gtk.RadioButton (null);
+            previous_btn = dumb_btn;
 
             wifi_list = new Gtk.ListBox ();
             wifi_list.selection_mode = Gtk.SelectionMode.SINGLE;
@@ -56,7 +60,9 @@
             disconnect_btn.sensitive = (device.get_state () == NM.DeviceState.ACTIVATED);
             disconnect_btn.get_style_context ().add_class ("destructive-action");
             disconnect_btn.clicked.connect (() => {
-                device.disconnect (null);
+                device.disconnect (((_device, _error) => {
+                    update_points ();
+                }));
             });
 
             var advanced_btn = Utils.get_advanced_button_from_device (device);
@@ -65,6 +71,7 @@
                 bool sensitive = (device.get_state () == NM.DeviceState.ACTIVATED);
                 disconnect_btn.sensitive = sensitive;
                 advanced_btn.sensitive = sensitive;
+                update_points ();
             });
 
             var hidden_btn = new Gtk.Button.with_label (_("Connect to Hidden Network…"));
@@ -84,10 +91,12 @@
             button_box.pack_start (hidden_btn, false, false, 0);
             button_box.pack_end (end_btn_box, false, false, 0);
 
+            device.notify["active-access-point"].connect (update_points);
             device.access_point_added.connect (add_access_point);
             device.access_point_removed.connect (remove_access_point);
 
             update (info_box);
+            update_points ();
 
             this.add_switch_title (_("Wireless:"));
             this.add (scrolled);
@@ -99,11 +108,11 @@
         private void on_row_activated (Gtk.ListBoxRow row) {
             if (device != null) {  
                 /* Do not activate connection if it is already activated */
-                if (device.get_active_access_point () != (row as WiFiEntry).ap) {
+                if (device.get_active_access_point () != ((WiFiEntry) row).ap) {
                     var setting_wireless = new NM.SettingWireless ();
-                    if (setting_wireless.add_seen_bssid ((row as WiFiEntry).ap.get_bssid ())) {
-                        current_connecting_entry = row as WiFiEntry;
-                        if ((row as WiFiEntry).is_secured) {
+                    if (setting_wireless.add_seen_bssid (((WiFiEntry) row).ap.get_bssid ())) {
+                        current_connecting_entry = ((WiFiEntry) row);
+                        if (((WiFiEntry) row).is_secured) {
                             var remote_settings = new NM.RemoteSettings (null);
 
                             var connection = new NM.Connection ();
@@ -112,62 +121,79 @@
                             connection.add_setting (s_con);
 
                             var s_wifi = new NM.SettingWireless ();
-                            s_wifi.@set (NM.SettingWireless.SSID, (row as WiFiEntry).ap.get_ssid ());
+                            s_wifi.@set (NM.SettingWireless.SSID, ((WiFiEntry) row).ap.get_ssid ());
                             connection.add_setting (s_wifi);
 
                             var s_wsec = new NM.SettingWirelessSecurity ();
-                            s_wsec.@set (NM.SettingWirelessSecurity.KEY_MGMT, "wpa-eap");
+                            s_wsec.@set (NM.SettingWirelessSecurity.KEY_MGMT, "wpa-psk");
                             connection.add_setting (s_wsec);
 
-                            var s_8021x = new NM.Setting8021x ();
-                            s_8021x.add_eap_method ("ttls");
-                            s_8021x.@set (NM.Setting8021x.PHASE2_AUTH, "mschapv2");
-                            connection.add_setting (s_8021x);
-                                            
-                            var dialog = NMGtk.new_wifi_dialog (client,
-                                                                remote_settings,
-                                                                connection,
-                                                                device,
-                                                                (row as WiFiEntry).ap,
-                                                                false);
-                            dialog.run ();
+                            var dialog = new NMAWifiDialog (client,
+                                                            remote_settings,
+                                                            connection,
+                                                            device,
+                                                            ((WiFiEntry) row).ap,
+                                                            false);
+                            
+                            dialog.response.connect ((response) => {
+                                if (response != Gtk.ResponseType.OK) {
+                                    return;
+                                }
+
+                                NM.Device dialog_device;
+                                NM.AccessPoint dialog_ap;
+                                var dialog_connection = dialog.get_connection (out dialog_device, out dialog_ap);
+
+                                if (get_connection_available (dialog_connection, dialog_device)) {
+                                    client.activate_connection (dialog_connection,
+                                                                dialog_device,
+                                                                dialog_ap.get_path (),
+                                                                null);                                    
+                                } else {
+                                    client.add_and_activate_connection (dialog_connection,
+                                                                        dialog_device,
+                                                                        dialog_ap.get_path (),
+                                                                        finish_connection_callback);
+                                }
+                            }); 
+
+                            dialog.run ();  
+                            dialog.destroy ();
                         } else {
-                            (row as WiFiEntry).set_status_point (false, true);
                             client.add_and_activate_connection (new NM.Connection (),
                                                                 device,
-                                                                (row as WiFiEntry).ap.get_path (),
+                                                                ((WiFiEntry) row).ap.get_path (),
                                                                 finish_connection_callback);
-                        }
+                        }                            
                     }
                 }
 
-                /* Check if we are successfully connected to the requested point */
-                if (device.get_active_access_point () == (row as WiFiEntry).ap) {
-                    entries.@foreach ((entry) => {
-                        entry.set_status_point (false, false);
-                    });
-                    
-                    (row as WiFiEntry).set_status_point (true, false);
-                }
+                update_points ();
             }
         }
 
+        private bool get_connection_available (NM.Connection connection, NM.Device _device) {
+            bool retval = false;
+            _device.get_available_connections ().@foreach ((_connection) => {
+                if (_connection == connection) {
+                    retval = true;
+                } 
+            });
+
+            return retval;
+        }
+
         private void finish_connection_callback (NM.Client _client,
                                                 NM.ActiveConnection connection,
                                                 string new_connection_path,
                                                 Error error) {
             bool success = false;
             _client.get_active_connections ().@foreach ((c) => {
-                if (c == connection)
+                if (c == connection) 
                     success = true;
             });
 
-            if (success) {
-                current_connecting_entry.set_status_point (true, false);
-            } else {
-                current_connecting_entry.set_status_point (false, false);
-            }
-
+            current_connecting_entry.set_active (success);
             current_connecting_entry = null;
         }
 
@@ -185,20 +211,25 @@
         }
         
         private void add_access_point (Object ap) {
-            var row = new WiFiEntry.from_access_point (ap as NM.AccessPoint);
-            if (!(row.ssid in BLACKLISTED) && row.ap.get_ssid () != null) {
+            var row = new WiFiEntry (((NM.AccessPoint) ap), previous_btn);
+            previous_btn = row.radio_btn;
+
+            if (!(row.ssid_str in BLACKLISTED) && row.ap.get_ssid () != null) {
                 if (insert_on_top) {
                     wifi_list.insert (row, 0);
                 } else {
                     wifi_list.add (row);
                 }
 
-                entries.append (row as WiFiEntry);
-            }
-
-            if ((ap as NM.AccessPoint) == device.get_active_access_point ()) {
-                row.set_status_point (true, false);
-            }
+                row.radio_btn.button_release_event.connect (() => {
+                    this.on_row_activated (row);
+                    return false;
+                });
+
+                entries.append (row);
+            }
+
+            update_points ();
         }
         
         private void remove_access_point (Object ap_removed) {
@@ -207,7 +238,9 @@
                     entries.remove (entry);
                     entry.destroy ();
                 }
-            });          
+            }); 
+
+            update_points ();         
         }
 
         private int sort_func (Gtk.ListBoxRow r1, Gtk.ListBoxRow r2) {
@@ -222,6 +255,36 @@
             } else {
                 return 0;
             }
-        }        
+        }
+
+        private void update_points () {
+            var active_point = device.get_active_access_point ();
+            if (active_point == null) {
+                dumb_btn.active = true;
+                return;
+            }
+
+            bool in_progress = false;
+            switch (device.get_state ()) {
+                case NM.DeviceState.PREPARE:
+                case NM.DeviceState.CONFIG:
+                case NM.DeviceState.NEED_AUTH:
+                case NM.DeviceState.IP_CONFIG:
+                case NM.DeviceState.IP_CHECK:
+                case NM.DeviceState.SECONDARIES:
+                    in_progress = true;        
+                    break;
+                default:
+                    break;    
+            }
+
+            entries.@foreach ((entry) => {
+                entry.set_connection_in_progress (false);
+                if (entry.ap == active_point) {
+                    entry.set_connection_in_progress (in_progress);
+                    entry.set_active (true);
+                }
+            });   
+        }                 
     }
 }
\ No newline at end of file

=== modified file 'src/libnm-gtk.vapi'
--- src/libnm-gtk.vapi	2015-05-23 18:30:03 +0000
+++ src/libnm-gtk.vapi	2015-08-01 18:42:17 +0000
@@ -1,13 +1,12 @@
+[CCode (cheader_filename = "nm-wifi-dialog.h")]
+class NMAWifiDialog : Gtk.Dialog {
+    public NMAWifiDialog (NM.Client client, NM.RemoteSettings settings, NM.Connection connection, NM.Device device, NM.AccessPoint ap, bool secrets_only);
+    public NM.Connection get_connection (out NM.Device device, out NM.AccessPoint ap);
+}        
+
+
 [CCode (cprefix = "NMGtk", gir_namespace = "NMGtk", gir_version = "1.0", lower_case_cprefix = "nmgtk_")]
 namespace NMGtk {
-    [CCode (cheader_filename = "libnm-gtk/nm-wifi-dialog.h", cname = "nma_wifi_dialog_new")] 
-    public Gtk.Dialog new_wifi_dialog (NM.Client client,
-                                       NM.RemoteSettings settings,
-                                       NM.Connection connection,
-                                       NM.Device device,
-                                       NM.AccessPoint ap,
-                                       bool secrets_only);
-                                       
     [CCode (cheader_filename = "libnm-gtk/nm-wifi-dialog.h", cname = "nma_wifi_dialog_new_for_other")]                                    
     public Gtk.Dialog new_wifi_dialog_for_hidden (NM.Client client, NM.RemoteSettings settings);                                                                           
 }