<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/121711/">https://git.reviewboard.kde.org/r/121711/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On December 29th, 2014, 5:12 a.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">apparle just commented on IRC that this change may affect battery life. It shouldn't have a big impact because the keepalive packet is handled by the OS instead of the app itself, but just in case, I will set it to a more conservative value, like 10 or 15 seconds.</p></pre>
 </blockquote>




 <p>On December 29th, 2014, 7:17 a.m. UTC, <b>Pramod Dematagoda</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't know if the difference between power usage would be significant either way, if we were looking at the power usage by the network adapter then the fact that we wait for a longer time before we start sending packets out to verify that the connection still exists would not result in much, if any, power being saved. Even if we were looking in terms of power usage based on the fact that no activity occurs, this still would not be true because the keep alive packets will eventually be sent out if the device has disconnected.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This also considering the gains we get in terms of kdeconnect being faster to detect a disconnected device which would be advantageous, especially for the screensaver inhibit plugin (shameless self plug there :P).</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 7:19 a.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I mean the battery life of the phone, that has to wake up to reply to the keepalive packages more often, not the computer.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 7:42 a.m. UTC, <b>Pramod Dematagoda</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well in that case the idle period still would not make a huge difference, all we might gain is about 10 seconds (with a 15 second idle) (and this assuming that kdeconnect is the only thing keeping the phone awake) extra where the phone has the opportunity to go to sleep before it has to wake back up for the keep alive packets.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I wouldn't mind increasing the idle time, but I just think it isn't worth it for what little extra battery time it might give us.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 7:47 a.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But for the same reason, it's not a big difference to have the phone incorrectly appearing as connected for 10 seconds instead of 5, and it would require half of the communication with the phone.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 7:53 a.m. UTC, <b>Pramod Dematagoda</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It would be the same amount of communication though, with the 10 second wait we just wait 5 seconds more before we decide to start sending the keep alive packets, the number of packets sent doesn't change. The only reason this 5 second extra wait might help is for usage patterns of active -> idle -> active over a long period of time (but even for this I wager the gain is almost negligible). Plus if the device was to actually be disconnected, then the device would not be woken up by these packets anyway.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 7:57 a.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If I understand correctly, when there is no activity it will send a packet every 5 seconds vs one every 10 seconds. So in 1 hour that would be 720 times that the phone will wake up vs 360 times.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 8:05 a.m. UTC, <b>Pramod Dematagoda</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No, that's the idle, it would start sending those packets after 5 seconds of the connection being idle in the current configuration, it would actually send a keep alive packet every 2 seconds (it was every 5 seconds under the previous configuration). But even if we extend the time between sending packets I would believe that the savings would be almost none. And yes, we could save on the number of packets we send out, but I don't believe that would necessarily translate to power savings directly.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 8:11 a.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Okay, sorry, then it's not a big deal to tune that parameter down, but I would not lower the interval to 2 seconds. There is no need to be so agresive with that: if the device is really gone, we have waited 5 seconds anyway for the keepalive to kick in. If the device is not gone (this is what will happen most of the times), we will be sending lots of unnecessary packets.</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 8:27 a.m. UTC, <b>Pramod Dematagoda</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There is a paper I found (http://www.slideshare.net/Sandra4211/tcp-wakeup-reducing-keepalive-traffic-in-mobile-ipv4-and) that supports my argument (page 4) that keep alives (on WLAN at least) are pretty inexpensive (to the point that there is approximately a 250mW saving from sending packets every 10 as opposed to 2 seconds). Keeping in mind that this was in 2008 and it is quite likely that hardware has become more efficient to reduce this difference now.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I realise that the paper is about an alternative to keep alive but still :P</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In the end though I am happy to increase the duration between packets (maybe the number of keep alives could be decreased to 2 to compensate). What number would you be happy with?</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 8:40 a.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The reason I'm being conservative here is because there are lots of users with different hardware and we want to be extra sure that we won't cause battery problems to any of them. Specially for a not-so-important reason as disconnecting the device early. It is useful for your screensaver-inhibit plugin, but in most of the cases is not a big deal. With the current parameters it takes over 70 seconds to disconnect a device, and I agree that it's too much, but there is no need to cut it down to 10 seconds either. Setting the values to 10 and 5 it will take 20 seconds to disconnect a device (10 for the initial packet + 5 for the second + 5 for the third), and I think that should be enough even for your screensaver inhibition use case. Are you okay with that?</p></pre>
 </blockquote>





 <p>On December 29th, 2014, 8:59 a.m. UTC, <b>Pramod Dematagoda</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Fair enough, you have your reasons and as you mentioned the difference between the different times is not that significant. I'll send a patch out with that, and the issues previously mentioned fixed up.</p></pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is it possible to quantify/benchmark all this? It's an odd conversation with too much guesses... :)</p></pre>
<br />










<p>- Aleix</p>


<br />
<p>On December 29th, 2014, 9:07 a.m. UTC, Pramod Dematagoda wrote:</p>









<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
 <tr>
  <td>

<div>Review request for kdeconnect and Albert Vaca Cintora.</div>
<div>By Pramod Dematagoda.</div>


<p style="color: grey;"><i>Updated Dec. 29, 2014, 9:07 a.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeconnect-kde
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Adjust the parameters used to set keep alive for the device's TCP socket to try and detect device disconnection faster.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">lanlinkprovider: Use the const qualifier for the keep alive parameters and indent the preprocessor directives correctly. Also adjust the keep alive parameters to reflect the discussion in review 121711.</p></pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Given that a phone is connected to kdeconnect.
When the phone's wireless is switched off.
And approximately 10 seconds pass.
Then kdeconnect detects that the device has disconnected and reflects this fact.</p></pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>core/backends/lan/lanlinkprovider.cpp <span style="color: grey">(39d30370ce225d1979f395fc20f26b23f0e791f1)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/121711/diff/" style="margin-left: 3em;">View Diff</a></p>






  </td>
 </tr>
</table>








  </div>
 </body>
</html>