<table><tr><td style="">daniel.z.tg added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D7652" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>The 3 codepaths can be found in <tt style="background: #ebebeb; font-size: 13px;">NetworkHelper</tt>. They were the following (but now changed):</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">"255.255.255.255"</li>
<li class="remarkup-list-item">Broadcast addresses from <tt style="background: #ebebeb; font-size: 13px;">NetworkInterface</tt>s. A list of these can be generated on Linux Java 8 using:</li>
</ul>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">Collections.list(NetworkInterface.getNetworkInterfaces()).stream()
                .filter(x -> isInterfaceNameCompatible(x.getName()))
                .map(NetworkInterface::getInterfaceAddresses).flatMap(List::stream)
                .map(InterfaceAddress::getBroadcast).filter(Objects::nonNull)
                .collect(Collectors.toList());</pre></div>

<ul class="remarkup-list">
<li class="remarkup-list-item">Network neighbors, (from ARP, or other places). On Android/Linux shell:</li>
</ul>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="console" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);"><span style="color: #000080">$ cat /proc/net/arp</span>
<span style="color: #000080">$ ip n</span>
<span style="color: #000080">$ arp</span></pre></div>

<p>The ARP codepath has been removed in the latest change, and exactly one of the other two can be active at a time. Previously, enabling multiple codepaths would cause things to randomly and silently break.</p>

<p>Also, getting rid of <tt style="background: #ebebeb; font-size: 13px;">isInterfaceNameCompatible()</tt> does not have any effect on my phone. My mobile data interface, <tt style="background: #ebebeb; font-size: 13px;">rmnet_data0</tt>, does not have a broadcast address, so it does not get included. My virtual interfaces like loopback don't have broadcast addresses either. Is this guaranteed for all devices? If so, we can fully remove <tt style="background: #ebebeb; font-size: 13px;">isInterfaceNameCompatible()</tt>. But if this is not the case, removing it might cause a routing loop if things get sent to loopback interfaces.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7652#inline-31458" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">albertvaka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">BackgroundService.java:58-65</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Why making them static?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><ol class="remarkup-list">
<li class="remarkup-list-item">They were already effectively static, since only 1 instance of <tt style="background: #ebebeb; font-size: 13px;">BackgroundService</tt> gets created, and that instance is stored is a static field, and all users use that static object to access instance fields & methods.</li>
<li class="remarkup-list-item"><tt style="background: #ebebeb; font-size: 13px;">BackgroundService</tt> needed to be called everywhere, and to do this either the instance needed to be passed around, or <tt style="background: #ebebeb; font-size: 13px;">getInstance()</tt> needed to be called. It also looks cleaner, and is more efficient to avoid calling <tt style="background: #ebebeb; font-size: 13px;">getInstace()</tt> every time.</li>
<li class="remarkup-list-item"><tt style="background: #ebebeb; font-size: 13px;">KdeConnectBroadcastReceiver</tt> needs <tt style="background: #ebebeb; font-size: 13px;">BackgroundService.refreshNetwork()</tt>. <tt style="background: #ebebeb; font-size: 13px;">refreshNetwork()</tt> in turn needs to use the static locks. It also needs to use <tt style="background: #ebebeb; font-size: 13px;">onNetworkChange()</tt>. Mixing instance and static when they could be the same looks wrong, so I changed <tt style="background: #ebebeb; font-size: 13px;">onNetworkChange()</tt> to static. It needed <tt style="background: #ebebeb; font-size: 13px;">linkProviders</tt> to be static, so I changed that as well. For consistency, I changed the other collections, <tt style="background: #ebebeb; font-size: 13px;">devices</tt> and <tt style="background: #ebebeb; font-size: 13px;">discoverymodeAcquisitions</tt> to static, along with their accessor methods. In the end, nothing except the initialization-related methods needed the <tt style="background: #ebebeb; font-size: 13px;">BackgroundService</tt> instance anymore, so I stopped passing it around in <tt style="background: #ebebeb; font-size: 13px;">onServiceStart()</tt>.</li>
</ol>

<p style="padding: 0; margin: 8px;">They don't need to be static, but it just simplifies things to make them that way. They will still work if changed back to instance fields/methods and chained with <tt style="background: #ebebeb; font-size: 13px;">getInstance()</tt>.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7652#inline-31459" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">albertvaka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">BackgroundService.java:248</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Why is it costly?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It's not anymore (only 5ms). It used to be costly earlier, when a lot of broadcast receivers were added, and debug messages were printed everywhere.</p>

<p style="padding: 0; margin: 8px;">Now, the filtering is only used to help with connection problems. If the network is refreshed twice before a neighbor can respond, the connection to that neighbor might require another refresh to get working. This is made worse by more broadcast receivers being added, and them actually refreshing the network every time it changes (before, they didn't always get registered).</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7652#inline-31460" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">albertvaka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">NetworkHelper.java:125-127</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">How do we know every USB tethering will start with 'rndis', etc? Is this documented anywhere?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yes, it is documented, and is standard in AOSP.</p>

<p style="padding: 0; margin: 8px;"><a href="https://github.com/android/platform_frameworks_base/blob/b09cf3fbf1c7e340bc06e8aba06461d4a0bae457/core/java/android/hardware/usb/UsbManager.java#L64-L65" class="remarkup-link" target="_blank" rel="noreferrer">https://github.com/android/platform_frameworks_base/blob/b09cf3fbf1c7e340bc06e8aba06461d4a0bae457/core/java/android/hardware/usb/UsbManager.java#L64-L65</a><br />
<a href="https://github.com/android/platform_frameworks_base/blob/b09cf3fbf1c7e340bc06e8aba06461d4a0bae457/services/core/java/com/android/server/connectivity/Tethering.java#L194" class="remarkup-link" target="_blank" rel="noreferrer">https://github.com/android/platform_frameworks_base/blob/b09cf3fbf1c7e340bc06e8aba06461d4a0bae457/services/core/java/com/android/server/connectivity/Tethering.java#L194</a><br />
<a href="https://stackoverflow.com/questions/7509924/detect-usb-tethering-on-android/43916414#43916414" class="remarkup-link" target="_blank" rel="noreferrer">https://stackoverflow.com/questions/7509924/detect-usb-tethering-on-android/43916414#43916414</a></p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7652#inline-31461" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">albertvaka</span> wrote in <span style="color: #4b4d51; font-weight: bold;">DeviceFragment.java:341</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Unrelated change?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't know the side effects of creating the plugin list, so I just skip it when it's invisible and all that should show is an error message. Probably doesn't make a difference.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R225 KDE Connect - Android application</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7652" rel="noreferrer">https://phabricator.kde.org/D7652</a></div></div><br /><div><strong>To: </strong>daniel.z.tg, KDE Connect<br /><strong>Cc: </strong>albertvaka, daniel.z.tg, jeanv, tfella, aboudhar, seebauer, bugzy, progwolff, MayeulC, menasshock, ach, apol<br /></div>