Merge lp://staging/~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals into lp://staging/~network-manager/network-manager/ubuntu
Status: | Merged |
---|---|
Approved by: | Mathieu Trudel-Lapierre |
Approved revision: | 997 |
Merged at revision: | 998 |
Proposed branch: | lp://staging/~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals |
Merge into: | lp://staging/~network-manager/network-manager/ubuntu |
Diff against target: |
202 lines (+121/-33) 2 files modified
debian/changelog (+11/-0) debian/patches/0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch (+110/-33) |
To merge this branch: | bzr merge lp://staging/~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mathieu Trudel-Lapierre | Needs Information | ||
Manuel de la Peña (community) | Approve | ||
Review via email: mp+271575@code.staging.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='
...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.
Have you discussed this with the upstream developers?
The suggested changes look reasonable, but there may be unforseen consequences to changing the code this way. Have you made sure that scan were still running in a reasonable fashion, even if a scan request failed, for example? It seems to me (from memory) that this would be one case where we might expect beacons to possibly schedule a new scan.
I much less intrusive change would be to simply remove the last-seen update on bss-updated -- it still makes sense to update last-seen for every scan results, otherwise it defeats its purpose... Still, there can be cases where scans fail, or when that value won't be up to date enough.