<table><tr><td style="">albertvaka 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>I'm trying to review this, but it's huge, so I will need some help. What are the three code paths you mention?</p>
<p>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.</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;">BackgroundService.java:58-65</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">private</span> <span class="bright"></span><span style="color: #aa4000"><span class="bright">final</span></span><span class="bright"> </span><span class="n"><span class="bright">ConcurrentHashMap</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"></span><span class="n"><span class="bright">String</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">DeviceListChangedCallback</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">></span></span><span class="bright"> </span><span class="n"><span class="bright">deviceListChangedCallbacks</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">new</span></span><span class="bright"> </span><span class="n"><span class="bright">ConcurrentHashMap</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><>();</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">private</span> <span class="bright"></span><span style="color: #aa4000"><span class="bright">static</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">final</span></span><span class="bright"> </span><span class="n"><span class="bright">ConcurrentMap</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"></span><span class="n"><span class="bright">String</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">DeviceListChangedCallback</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">></span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">deviceListChangedCallbacks</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">ConcurrentHashMap</span><span style="color: #aa2211"><>();</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">private</span> <span style="color: #aa4000">final</span> <span class="bright"></span><span class="n"><span class="bright">Array</span>List</span><span style="color: #aa2211"><</span><span class="n">BaseLinkProvider</span><span style="color: #aa2211">></span> <span class="n">linkProviders</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">ArrayList</span><span style="color: #aa2211"><>();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">private</span> <span class="bright"></span><span style="color: #aa4000"><span class="bright">static</span></span><span class="bright"> </span><span style="color: #aa4000">final</span> <span class="n">List</span><span style="color: #aa2211"><</span><span class="n">BaseLinkProvider</span><span style="color: #aa2211">></span> <span class="n">linkProviders</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">ArrayList</span><span style="color: #aa2211"><>();</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">private</span> <span style="color: #aa4000">final</span> <span class="n">Concurrent<span class="bright">Hash</span>Map</span><span style="color: #aa2211"><</span><span class="n">String</span><span style="color: #aa2211">,</span> <span class="n">Device</span><span style="color: #aa2211">></span> <span class="n">devices</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">ConcurrentHashMap</span><span style="color: #aa2211"><>();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">private</span> <span class="bright"></span><span style="color: #aa4000"><span class="bright">static</span></span><span class="bright"> </span><span style="color: #aa4000">final</span> <span class="n">ConcurrentMap</span><span style="color: #aa2211"><</span><span class="n">String</span><span style="color: #aa2211">,</span> <span class="n">Device</span><span style="color: #aa2211">></span> <span class="n">devices</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">ConcurrentHashMap</span><span style="color: #aa2211"><>();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Why making them static?</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;">BackgroundService.java:248</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">/*</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * Refreshing Network may be costly, so don't do it unless necessary.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Why is it costly?</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;">NetworkHelper.java:125-127</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span style="color: #aa2211">(<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">info</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">getType</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">()</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">==</span></span><span class="bright"> </span><span class="n"><span class="bright">ConnectivityManager</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">TYPE_MOBILE</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">mobile</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span class="n"><span class="bright">info</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">isConnected</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">();</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span style="color: #aa2211">(<span class="bright">!(</span></span><span class="bright"></span><span class="n"><span class="bright">name</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">startsWith</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">(</span></span><span class="bright"></span><span style="color: #766510"><span class="bright">"rndis"</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// USB Tethering</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"> </span><span style="color: #aa2211"><span class="bright">||</span></span><span class="bright"> </span><span class="n"><span class="bright">name</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">equals</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">(</span></span><span class="bright"></span><span style="color: #766510"><span class="bright">"bt-pan"</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// Bluetooth Tethering</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa2211">||</span> <span class="n">name</span><span style="color: #aa2211">.</span><span style="color: #354bb3">startsWith</span><span style="color: #aa2211">(</span><span style="color: #766510">"wlan"</span><span style="color: #aa2211">)))</span> <span style="color: #aa2211">{</span> <span style="color: #74777d">// Regular WiFi / WiFi Tethering</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><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><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;">DeviceFragment.java:341</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">rootView</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">findViewById</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">R</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">id</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">pairing_buttons</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">).</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">setVisibility</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">paired</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">?</span></span><span class="bright"> </span><span class="n"><span class="bright">View</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">GONE</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">:</span></span><span class="bright"> </span><span class="n"><span class="bright">View</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">VISIBLE</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span class="n"><span class="bright">rootView</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">findViewById</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">R</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">id</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">not_reachable_message</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">).</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">setVisibility</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">((</span></span><span class="bright"></span><span class="n"><span class="bright">paired</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&&</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">!</span></span><span class="bright"></span><span class="n"><span class="bright">reachable</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&&</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">!</span></span><span class="bright"></span><span class="n"><span class="bright">onData</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">?</span></span><span class="bright"> </span><span class="n"><span class="bright">View</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">VISIBLE</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">:</span></span><span class="bright"> </span><span class="n"><span class="bright">View</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">.</span></span><span class="bright"></span><span style="color: #354bb3"><span class="bright">GONE</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">)</span>;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">(!</span></span><span class="bright"></span><span class="n"><span class="bright">reachable</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"> </span><span style="color: #aa4000"><span class="bright">return</span></span><span style="color: #aa2211">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Unrelated change?</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, hkaelberer<br /></div>