GNOME Bugzilla – Bug 747424
[review] dcbw/supplicant-current-ap-bgo747424: use supplicant's CurrentBSS as the current AP
Last modified: 2015-04-10 21:59:08 UTC
Instead of trying to find the current AP ourselves from SSID, BSSID, frequency, etc, just use the supplicant's notion of the current AP.
Patches in dcbw/supplicant-current-ap-bgo747424
>> wifi: use a hash table to track access points seems most access to the APs still iterates over all the items. It's not clear to me, why using a hash table is better. Downside is, that now the order of iteration is unstable. That seems undesirable for ap_list_dump(), impl_device_get_access_points(). >> wifi: update AP properties from supplicant signals + if (g_variant_lookup (properties, "Signal", "n", &i16)) { + gint8 old_strength = nm_ap_get_strength (ap); + gint8 new_strength = nm_ap_utils_level_to_quality (i16); + + if (old_strength != new_strength) + nm_ap_set_strength (ap, new_strength); } nm_ap_set_strength() already checks whether the new value is different. Just if (g_variant_lookup (properties, "Signal", "n", &i16)) nm_ap_set_strength (ap, nm_ap_utils_level_to_quality (i16)); same for nm_ap_set_mode(), etc. pushed minor suggestions/fixup
(In reply to Thomas Haller from comment #2) > >> wifi: use a hash table to track access points > > seems most access to the APs still iterates over all the items. It's not > clear to me, why using a hash table is better. It should make some operations like check_connection_available(), complete_connection(), act_stage1_prepare(), AP removal, set_current_ap(), and new AP insertion faster, at least those will no longer be O(n). I guess I'm just partial to hash tables over lists that could be fairly large, partly insertion/lookup/removal is much shorter to write. > Downside is, that now the order of iteration is unstable. That seems > undesirable for ap_list_dump(), impl_device_get_access_points(). Yeah, this is true. I've added a fixup for this. > >> wifi: update AP properties from supplicant signals > > + if (g_variant_lookup (properties, "Signal", "n", &i16)) { > + gint8 old_strength = nm_ap_get_strength (ap); > + gint8 new_strength = nm_ap_utils_level_to_quality (i16); > + > + if (old_strength != new_strength) > + nm_ap_set_strength (ap, new_strength); > } > > nm_ap_set_strength() already checks whether the new value is different. Just > > if (g_variant_lookup (properties, "Signal", "n", &i16)) > nm_ap_set_strength (ap, nm_ap_utils_level_to_quality (i16)); > > same for nm_ap_set_mode(), etc. Fixed and repushed.
LGTM now
> wifi: fall back to band matching when frequency matching fails Would it be useful to put freq_to_band () to nm-utils.c? Otherwise, it looks good to me.
(In reply to Jiri Klimes from comment #5) > > wifi: fall back to band matching when frequency matching fails > > Would it be useful to put freq_to_band () to nm-utils.c? > > Otherwise, it looks good to me. Yeah, it might be, but I don't think it's good external API quite yet. Maybe later, if something else wants to use it? Merged to git master.