D7652: Better broadcast/neighbor networking

[Albert Vaca Cintora]

albertvaka added a comment.


  I'm trying to review this, but it's huge, so I will need some help. What are the three code paths you mention?
  
  Also, if you want people to test if it works on their devices, maybe you can add an UI with some debug info, so people who find this doesn't work can come back with useful info.

INLINE COMMENTS

> BackgroundService.java:58-65
> +    private static final ConcurrentMap<String, DeviceListChangedCallback>
> +            deviceListChangedCallbacks = new ConcurrentHashMap<>();
>  
> -    private final ArrayList<BaseLinkProvider> linkProviders = new ArrayList<>();
> +    private static final List<BaseLinkProvider> linkProviders = new ArrayList<>();
>  
> -    private final ConcurrentHashMap<String, Device> devices = new ConcurrentHashMap<>();
> +    private static final ConcurrentMap<String, Device> devices = new ConcurrentHashMap<>();
>  

Why making them static?

> BackgroundService.java:248
> +        /*
> +         * Refreshing Network may be costly, so don't do it unless necessary.
> +         *

Why is it costly?

> NetworkHelper.java:125-127
> +                if (!(name.startsWith("rndis") // USB Tethering
> +                        || name.equals("bt-pan") // Bluetooth Tethering
> +                        || name.startsWith("wlan"))) { // Regular WiFi / WiFi Tethering

How do we know every USB tethering will start with 'rndis', etc? Is this documented anywhere?

> DeviceFragment.java:341
> +
> +                    if (!reachable) {
> +                        return;

Unrelated change?

REPOSITORY
  R225 KDE Connect - Android application

REVISION DETAIL
  https://phabricator.kde.org/D7652

To: daniel.z.tg, #kde_connect
Cc: albertvaka, daniel.z.tg, jeanv, tfella, aboudhar, seebauer, bugzy, progwolff, MayeulC, menasshock, ach, apol, hkaelberer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20170903/9d27d80d/attachment.html>