Merge lp:~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals-vivid into lp:~phablet-team/network-manager/vivid-phone-overlay

Proposed by Tony Espy on 2015-10-01
Status: Merged
Approved by: Alfonso Sanchez-Beato on 2015-10-02
Approved revision: 969
Merged at revision: 969
Proposed branch: lp:~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals-vivid
Merge into: lp:~phablet-team/network-manager/vivid-phone-overlay
Diff against target: 192 lines (+115/-30)
2 files modified
debian/changelog (+8/-0)
debian/patches/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch (+107/-30)
To merge this branch: bzr merge lp:~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals-vivid
Reviewer Review Type Date Requested Status
Mathieu Trudel-Lapierre 2015-10-01 Approve on 2015-10-02
Simon Fels 2015-10-02 Pending
Review via email: mp+273137@code.launchpad.net

Description of the change

This change fixes a problem where duplicate (2-3x) AccessPoint 'LastSeen' PropertiesChanged signals are generated each time a WiFi scan completes. This can be seen by running:

dbus-monitor --system --profile "type='signal',sender='org.freedesktop.NetworkManager'"

...and waiting for a scan to occur. You'll see 2-3 PropsChanged for each AccessPoint after a scan. From then on, you'll only see changes for AccessPoint/0 until the next scan happens.

The change makes cull_scan_list the function responsible for updating an AP's last-seen property, and ensures that scan_done_cb is the only function that calls cull_scan_list. It also removes the bss_updated_cb function, as its only purpose was to update the last-seen property and schedule yet another scanlist cull.

To post a comment you must log in.
969. By Tony Espy on 2015-10-01

d/p/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch:
Fix duplicate 'LastSeen' PropertiesChanged signals being emitted
after scanning (LP: #1480877).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2015-08-13 15:26:03 +0000
+++ debian/changelog 2015-10-01 20:47:33 +0000
@@ -1,3 +1,11 @@
1network-manager (0.9.10.0-4ubuntu15.1.8) UNRELEASED; urgency=medium
2
3 * d/p/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch:
4 Fix duplicate 'LastSeen' PropertiesChanged signals being emitted
5 after scanning (LP: #1480877).
6
7 -- Tony Espy <espy@canonical.com> Thu, 01 Oct 2015 16:30:19 -0400
8
1network-manager (0.9.10.0-4ubuntu15.1.7) vivid; urgency=medium9network-manager (0.9.10.0-4ubuntu15.1.7) vivid; urgency=medium
210
3 * d/p/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch:11 * d/p/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch:
412
=== modified file 'debian/patches/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch'
--- debian/patches/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch 2015-08-05 16:23:18 +0000
+++ debian/patches/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch 2015-10-01 20:47:33 +0000
@@ -18,29 +18,66 @@
18===================================================================18===================================================================
19--- network-manager-0.9.10.0.orig/src/devices/wifi/nm-device-wifi.c19--- network-manager-0.9.10.0.orig/src/devices/wifi/nm-device-wifi.c
20+++ network-manager-0.9.10.0/src/devices/wifi/nm-device-wifi.c20+++ network-manager-0.9.10.0/src/devices/wifi/nm-device-wifi.c
21@@ -183,6 +183,10 @@ static void supplicant_iface_notify_scan21@@ -129,7 +129,6 @@ struct _NMDeviceWifiPrivate {
22 gint32 scheduled_scan_time;
23 guint8 scan_interval; /* seconds */
24 guint pending_scan_id;
25- guint scanlist_cull_id;
26 gboolean requested_scan;
27
28 NMSupplicantManager *sup_mgr;
29@@ -166,11 +165,6 @@ static void supplicant_iface_new_bss_cb
30 GHashTable *properties,
31 NMDeviceWifi * self);
32
33-static void supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface,
34- const char *object_path,
35- GHashTable *properties,
36- NMDeviceWifi *self);
37-
38 static void supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface,
39 const char *object_path,
40 NMDeviceWifi *self);
41@@ -183,7 +177,7 @@ static void supplicant_iface_notify_scan
22 GParamSpec * pspec,42 GParamSpec * pspec,
23 NMDeviceWifi * self);43 NMDeviceWifi * self);
24 44
25+static void remove_outstanding_scanlist_cull(NMDeviceWifi *self);45-static void schedule_scanlist_cull (NMDeviceWifi *self);
26+
27+static gboolean cull_scan_list(NMDeviceWifi *self);46+static gboolean cull_scan_list(NMDeviceWifi *self);
28+
29 static void schedule_scanlist_cull (NMDeviceWifi *self);
30 47
31 static gboolean request_wireless_scan (gpointer user_data);48 static gboolean request_wireless_scan (gpointer user_data);
32@@ -1609,14 +1613,22 @@ supplicant_iface_scan_done_cb (NMSupplic49
50@@ -278,10 +272,6 @@ supplicant_interface_acquire (NMDeviceWi
51 G_CALLBACK (supplicant_iface_new_bss_cb),
52 self);
53 g_signal_connect (priv->sup_iface,
54- NM_SUPPLICANT_INTERFACE_BSS_UPDATED,
55- G_CALLBACK (supplicant_iface_bss_updated_cb),
56- self);
57- g_signal_connect (priv->sup_iface,
58 NM_SUPPLICANT_INTERFACE_BSS_REMOVED,
59 G_CALLBACK (supplicant_iface_bss_removed_cb),
60 self);
61@@ -314,11 +304,6 @@ supplicant_interface_release (NMDeviceWi
62 nm_device_get_iface (NM_DEVICE (self)),
63 priv->scan_interval);
64
65- if (priv->scanlist_cull_id) {
66- g_source_remove (priv->scanlist_cull_id);
67- priv->scanlist_cull_id = 0;
68- }
69-
70 if (priv->sup_iface) {
71 remove_supplicant_interface_error_handler (self);
72
73@@ -1609,14 +1594,17 @@ supplicant_iface_scan_done_cb (NMSupplic
33 nm_device_get_iface (NM_DEVICE (self)),74 nm_device_get_iface (NM_DEVICE (self)),
34 success ? "successful" : "failed");75 success ? "successful" : "failed");
35 76
36- g_signal_emit (self, signals[SCAN_DONE], 0, NULL);77- g_signal_emit (self, signals[SCAN_DONE], 0, NULL);
37-78-
38- schedule_scan (self, success);79- schedule_scan (self, success);
39+ /* Make sure that scheduled and not yet run scanlist cull80-
40+ * requests are aborted.
41+ */
42+ remove_outstanding_scanlist_cull (self);
43
44 /* Ensure that old APs get removed, which otherwise only81 /* Ensure that old APs get removed, which otherwise only
45 * happens when there are new BSSes.82 * happens when there are new BSSes.
46 */83 */
@@ -56,7 +93,16 @@
56 93
57 if (priv->requested_scan) {94 if (priv->requested_scan) {
58 priv->requested_scan = FALSE;95 priv->requested_scan = FALSE;
59@@ -1793,8 +1805,10 @@ cull_scan_list (NMDeviceWifi *self)96@@ -1767,8 +1755,6 @@ cull_scan_list (NMDeviceWifi *self)
97 GSList *elt;
98 guint32 removed = 0, total = 0;
99
100- priv->scanlist_cull_id = 0;
101-
102 nm_log_dbg (LOGD_WIFI_SCAN, "(%s): checking scan list for outdated APs",
103 nm_device_get_iface (NM_DEVICE (self)));
104
105@@ -1793,8 +1779,10 @@ cull_scan_list (NMDeviceWifi *self)
60 * supplicant in the last scan.106 * supplicant in the last scan.
61 */107 */
62 if ( nm_ap_get_supplicant_path (ap)108 if ( nm_ap_get_supplicant_path (ap)
@@ -68,27 +114,58 @@
68 114
69 last_seen = nm_ap_get_last_seen (ap);115 last_seen = nm_ap_get_last_seen (ap);
70 if (!last_seen || last_seen + prune_interval_s < boottime_now)116 if (!last_seen || last_seen + prune_interval_s < boottime_now)
71@@ -1836,13 +1850,22 @@ cull_scan_list (NMDeviceWifi *self)117@@ -1836,17 +1824,6 @@ cull_scan_list (NMDeviceWifi *self)
72 }118 }
73 119
74 static void120 static void
75-schedule_scanlist_cull (NMDeviceWifi *self)121-schedule_scanlist_cull (NMDeviceWifi *self)
76+remove_outstanding_scanlist_cull (NMDeviceWifi *self)122-{
77 {123- NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
78 NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);124-
79 125- /* Cull the scan list after the last request for it has come in */
80 /* Cull the scan list after the last request for it has come in */126- if (priv->scanlist_cull_id)
81 if (priv->scanlist_cull_id)127- g_source_remove (priv->scanlist_cull_id);
82 g_source_remove (priv->scanlist_cull_id);128- priv->scanlist_cull_id = g_timeout_add_seconds (4, (GSourceFunc) cull_scan_list, self);
83+}129-}
84+130-
85+static void131-static void
86+schedule_scanlist_cull (NMDeviceWifi *self)132 supplicant_iface_new_bss_cb (NMSupplicantInterface *iface,
87+{133 const char *object_path,
88+ NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);134 GHashTable *properties,
89+135@@ -1877,36 +1854,6 @@ supplicant_iface_new_bss_cb (NMSupplican
90+ remove_outstanding_scanlist_cull(self);136 nm_log_warn (LOGD_WIFI_SCAN, "(%s): invalid AP properties received",
91+137 nm_device_get_iface (NM_DEVICE (self)));
92 priv->scanlist_cull_id = g_timeout_add_seconds (4, (GSourceFunc) cull_scan_list, self);138 }
139-
140- /* Remove outdated access points */
141- schedule_scanlist_cull (self);
142-}
143-
144-static void
145-supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface,
146- const char *object_path,
147- GHashTable *properties,
148- NMDeviceWifi *self)
149-{
150- NMDeviceState state;
151- NMAccessPoint *ap;
152-
153- g_return_if_fail (self != NULL);
154- g_return_if_fail (object_path != NULL);
155- g_return_if_fail (properties != NULL);
156-
157- /* Ignore new APs when unavailable or unamnaged */
158- state = nm_device_get_state (NM_DEVICE (self));
159- if (state <= NM_DEVICE_STATE_UNAVAILABLE)
160- return;
161-
162- /* Update the AP's last-seen property */
163- ap = get_ap_by_supplicant_path (self, object_path);
164- if (ap)
165- nm_ap_update_last_seen (ap);
166-
167- /* Remove outdated access points */
168- schedule_scanlist_cull (self);
93 }169 }
94 170
171 static void

Subscribers

People subscribed via source and target branches

to all changes: