Do we need to set mode to QSensor::FixedOrientation

it's not clear what the default is in the docs

It's very wasteful to have this always on when we're not in autorotate mode.

From a mobile POV, that's an entire sensor being active and causing wakeups. We also want it off when the screen is on dpms.

IMHO this would be clearer if we abstract to a screen orientation inside this class, rather than exposing a version with FaceUp and FaceDown events only to ignore them.

It says the default is fixed orientation: https://doc.qt.io/qt-5/qsensor.html#orientation

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.

I left FaceUp and FaceDown in there in case at some point we also want to react to these states. But it's not really necessary and I will change it to use KScreen::Output::Rotation directly instead.

I suspect some apps will need that info, but it's just as easy for them to get that via QSensors directly.

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.

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.

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)

Yea, I overlooked this getter. Will replace.

Yea, the "just use QOrientationSensor enum" argument makes sense. I will try to use this instead.