Merge lp://staging/~phablet-team/network-manager/lp1480877-wifi-rm-dup-scan-signals into lp://staging/~network-manager/network-manager/ubuntu

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.

My changes have no effect on whether or not scans are scheduled.

For the gory details see:

https://bugs.launchpad.net/ubuntu-rtm/+source/network-manager/+bug/1480877/comments/41

I checked, and the change that introduced DUP signals in first place was a location services fix ( see: 0002-wifi-cull-the-scan-list-before-signalling-ScanDone-b.patch ), which hasn't been up-streamed, so my changes aren't really directly applicable.

I've test three different phones, and a laptop running wily, and the only time bss_updated_cb is ever fired is after a scan completes. As scan_done_cb always calls cull_scan_list directly, which in turn updates all of the last-seen properties of the know AccessPoints, the removal of bss_updated_cb is basically a no-op.

The removal of the schedule_scan_list call from new_bss_cb again is also predicated on the fact that new_bss_cb only ever gets triggered when a scan happens. As it was the last function that used schedule_scanlist_cull, I removed it and the associated functions/priv member.

This should not affect the location service and fixes the redundant signals.

Have you made sure that this did not break the signal level change notifications, and that we can still see signal level changes in the indicator on desktop?

If you mean, signal changes for the currently connected AP ( AccessPoint/0 ), then yes, I've verified that this doesn't break signal strength changes for the current AP.

These signal strength updates are handled by periodic_update_cb, which fires every six seconds when connected to an AP. This callback in turn calls a function periodic_update which reads the signal strength of the connected AP via NMPlaform and then sets the AP property, which fires a DBus signal.

That said, I've only verified this behavior directly on a phone (krillin stable #25) running the 0.9.10 version of this patch. If you want me to verify it on a wily desktop before we build the silo, I will do so...

Just tested on my Thinkpad T410s running wily. It has an Intel WiFi adapter and is running iwlwifi.

Confirmed that DBus signal strength PropertiesChanged signals for AccessPoint/0 are still fired periodically when associated with an access point. Also confirmed that the indicator sees these signals and responds by changing the WiFi icon according to signal strength.

Testing with version 1.0.4-0ubuntu4~awe3 from my PPA:

https://launchpad.net/~awe/+archive/ubuntu/ppa/+packages