davidedmundson added inline comments.

INLINE COMMENTS

> romangg wrote in orientation_sensor.cpp:29
> 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.

From what I can tell, we can call

if (!sensor->connectToBackend()) {
 m_available = false;
}

in the constructor

and then we have the option to call start/stop whenever.

> romangg wrote in orientation_sensor.h:38
> 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:
> 
> > 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.

I don't really understand.

The only reason to wrap QOrientationSensor in a wrapper class is to try and 
encapsulate the details of the sensor into something domain specific.

If we just forward everything 1:1, what does this wrapper provide over just 
having the other code use QOrietnationSensor directly.

(but whatever this isn't a topic I'm particularly passionate about, so whatever)

REPOSITORY
  R104 KScreen

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

To: romangg, #plasma
Cc: 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

Reply via email to