Differential D26037 Diff 73146 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 "globals.h"
20​
21​#include <QObject>
22​
23​class QOrientationSensor;
24​
25​using namespace Globals;
26​
27​class OrientationSensor : public QObject
28​{
29​ Q_OBJECT
30​public:
31​ explicit OrientationSensor(QObject *parent = nullptr);
32​ ~OrientationSensor() override final;
33​
34​ DeviceOrientation value() const;
35​ bool available() const;
36​ bool enabled() const;
37​
38​ void setEnabled(bool enable);
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​Q_SIGNALS:
41​ void valueChanged(DeviceOrientation orientation);
42​ void availableChanged(bool available);
43​ void enabledChanged(bool enabled);
44​
45​private:
46​ void refresh();
47​ void updateState();
48​
49​ QOrientationSensor *m_sensor;
50​ DeviceOrientation m_value = DeviceOrientation::Undefined;
51​ bool m_available = false;
52​ bool m_enabled = false;
53​};