<table><tr><td style="">davidedmundson added inline comments.
</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/D26037">View Revision</a></tr></table><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/D26037#inline-146757">View Inline</a><span style="color: #4b4d51; font-weight: bold;">romangg</span> wrote in <span style="color: #4b4d51; font-weight: bold;">orientation_sensor.cpp:29</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Yea, we need to start it in the beginning to see if a sensor is available. But we could cache the result and then shut it off again in case auto rotation or screen is off. Needs some more logic though.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">From what I can tell, we can call</p>

<p style="padding: 0; margin: 8px;">if (!sensor->connectToBackend()) {<br />
 m_available = false;<br />
}</p>

<p style="padding: 0; margin: 8px;">in the constructor</p>

<p style="padding: 0; margin: 8px;">and then we have the option to call start/stop whenever.</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/D26037#inline-147717">View Inline</a><span style="color: #4b4d51; font-weight: bold;">romangg</span> wrote in <span style="color: #4b4d51; font-weight: bold;">orientation_sensor.h:38</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Looking at it again I don't think it is advisable to remove these two values from the class interface. Instead having them in there for later and ignoring them in the meantime in the implementation in the KScreenDaemon is fine. See the comment there:</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">We currently don't do anything with FaceUp/FaceDown, but in the future we could use them to shut off and switch on again a display when display is facing downwards/upwards.</p></blockquote></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't really understand.</p>

<p style="padding: 0; margin: 8px;">The only reason to wrap QOrientationSensor in a wrapper class is to try and encapsulate the details of the sensor into something domain specific.</p>

<p style="padding: 0; margin: 8px;">If we just forward everything 1:1, what does this wrapper provide over just having the other code use QOrietnationSensor directly.</p>

<p style="padding: 0; margin: 8px;">(but whatever this isn't a topic I'm particularly passionate about, so whatever)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R104 KScreen</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D26037">https://phabricator.kde.org/D26037</a></div></div><br /><div><strong>To: </strong>romangg, Plasma<br /><strong>Cc: </strong>plasma-devel, davidedmundson, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart<br /></div>