Differential D26037 Diff 73619 common/orientation_sensor.h

Changeset View

Standalone View

View Options

common/orientation_sensor.h

  • This file was added.
1​/********************************************************************
2​Copyright © 2019 Roman Gilg <subdiff@gmail.com>
3​
4​This program is free software; you can redistribute it and/or modify
5​it under the terms of the GNU General Public License as published by
6​the Free Software Foundation; either version 2 of the License, or
7​(at your option) any later version.
8​
9​This program is distributed in the hope that it will be useful,
10​but WITHOUT ANY WARRANTY; without even the implied warranty of
11​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12​GNU General Public License for more details.
13​
14​You should have received a copy of the GNU General Public License
15​along with this program. If not, see <http://www.gnu.org/licenses/>.
16​*********************************************************************/
17​#pragma once
18​
19​#include <QObject>
20​#include <QOrientationReading>
21​
22​class OrientationSensor : public QObject
23​{
24​ Q_OBJECT
25​public:
26​ explicit OrientationSensor(QObject *parent = nullptr);
27​ ~OrientationSensor() override final;
28​
29​ QOrientationReading::Orientation value() const;
30​ bool available() const;
31​ bool enabled() const;
32​
33​ void setEnabled(bool enable);
34​
35​Q_SIGNALS:
36​ void valueChanged(QOrientationReading::Orientation orientation);
37​ void availableChanged(bool available);
38​ void enabledChanged(bool enabled);
davidedmundsonUnsubmitted
Done

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.

davidedmundson: IMHO this would be clearer if we abstract to a screen orientation inside this class, rather…
romanggAuthorUnsubmitted
Done

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.

romangg: I left FaceUp and FaceDown in there in case at some point we also want to react to these states.
davidedmundsonUnsubmitted
Done

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

davidedmundson: I suspect some apps will need that info, but it's just as easy for them to get that via…
romanggAuthorUnsubmitted
Done

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.

romangg: Looking at it again I don't think it is advisable to remove these two values from the class…
davidedmundsonUnsubmitted
Done

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)

davidedmundson: I don't really understand. The only reason to wrap QOrientationSensor in a wrapper class is to…
romanggAuthorUnsubmitted
Done

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

romangg: Yea, the "just use QOrientationSensor enum" argument makes sense. I will try to use this…
39​
40​private:
41​ void refresh();
42​ void updateState();
43​
44​ QOrientationSensor *m_sensor;
45​ QOrientationReading::Orientation m_value = QOrientationReading::Undefined;
46​ bool m_enabled = false;
47​};