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

Subscribers

People subscribed via source and target branches

to all changes: