Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent location state changes during emergency calls #27

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rpm/nemo-qml-plugin-systemsettings.spec
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ BuildRequires: pkgconfig(Qt5XmlPatterns)
BuildRequires: pkgconfig(timed-qt5)
BuildRequires: pkgconfig(profile)
BuildRequires: pkgconfig(mce) >= 1.21.0
BuildRequires: pkgconfig(mce-qt5) >= 1.4.2
BuildRequires: pkgconfig(mlite5) >= 0.3.6
BuildRequires: pkgconfig(usb-moded-qt5)
BuildRequires: pkgconfig(blkid)
Expand Down
68 changes: 68 additions & 0 deletions src/locationsettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ LocationSettingsPrivate::LocationSettingsPrivate(LocationSettings::Mode mode, Lo
"net.connman",
"/net/connman/technology/gps",
"net.connman.Technology"))
, m_mceCallState(new QMceCallState(this))
, m_canToggleLocation(LocationSettings::ToggleEnabled)
{
loadProviders();

Expand Down Expand Up @@ -233,6 +235,16 @@ LocationSettingsPrivate::LocationSettingsPrivate(LocationSettings::Mode mode, Lo
this, &LocationSettingsPrivate::findGpsTech);
findGpsTech();
}

connect(m_mceCallState, &QMceCallState::validChanged,
this, &LocationSettingsPrivate::onCallStateChanged);
connect(m_mceCallState, &QMceCallState::stateChanged,
this, &LocationSettingsPrivate::onCallStateChanged);
connect(m_mceCallState, &QMceCallState::typeChanged,
this, &LocationSettingsPrivate::onCallStateChanged);

/* To check the current status of the call if it is valid */
onCallStateChanged();
}

LocationSettingsPrivate::~LocationSettingsPrivate()
Expand Down Expand Up @@ -466,13 +478,25 @@ bool LocationSettings::locationEnabled() const
void LocationSettings::setLocationEnabled(bool enabled)
{
Q_D(LocationSettings);

if (d->m_canToggleLocation) {
qWarning() << "Cannot allow to change location enabled status when emergency call is active";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering whether we want debug logging for any attempt to enable/disable, or just for the ones that would actually lead to changes i.e. should this be withing the below "if it differs from current" conditional block?

Also, is it okay for qml property setter not to set the property? I'd guess yes, but does somebody know for sure?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort of related, if there were properties like "canToggleLocationEnabled" or stte -> those could perhaps be used in (qml) ui to disable/hide those element that could lead to attempts to make such changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good option I think, to have a canToggleLocation visible upwards to UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some with 517f7bd -> naming needs to be rethought. On Friday evening brain just shortcircuits.

return;
}

if (enabled != d->m_locationEnabled) {
d->m_locationEnabled = enabled;
d->writeSettings();
emit locationEnabledChanged();
}
}

LocationSettings::LocationCanToggle LocationSettings::locationCanToggle() const
{
Q_D(const LocationSettings);
return d->m_canToggleLocation;
}

bool LocationSettings::gpsEnabled() const
{
Q_D(const LocationSettings);
Expand All @@ -482,6 +506,20 @@ bool LocationSettings::gpsEnabled() const
void LocationSettings::setGpsEnabled(bool enabled)
{
Q_D(LocationSettings);

switch (d->m_canToggleLocation) {
case LocationSettings::ToggleDisabled:
qDebug() << "Cannot allow to change GPS status when emergency call is active";
return;
case LocationSettings::CanOnlyEnable:
if (!enabled) {
qDebug() << "Cannot allow to disable GPS when emergency call is active";
return;
}
case LocationSettings::ToggleEnabled:
break;
}

if (enabled != d->m_gpsEnabled) {
d->m_gpsEnabled = enabled;
d->writeSettings();
Expand Down Expand Up @@ -529,6 +567,12 @@ bool LocationSettings::gpsAvailable() const
return QFile::exists(QStringLiteral("/usr/libexec/geoclue-hybris"));
}

LocationSettings::LocationCanToggle LocationSettings::gpsCanToggle() const
{
Q_D(const LocationSettings);
return d->m_canToggleLocation;
}

QStringList LocationSettings::locationProviders() const
{
Q_D(const LocationSettings);
Expand Down Expand Up @@ -814,3 +858,27 @@ void LocationSettingsPrivate::writeSettings()
}
}
}

void LocationSettingsPrivate::onCallStateChanged()
{
LocationSettings::LocationCanToggle canToggleLocation;

/*
* During emergency call the location is set by AML.
* TODO: We'd most likely need a specific type for AML enabled emergency
* call since AML is not always enabled everywhere as it is not supported
* in every region yet.
*/
if (m_mceCallState->valid() && m_mceCallState->state() == QMceCallState::Active &&
m_mceCallState->type() == QMceCallState::Emergency) {
canToggleLocation = LocationSettings::ToggleDisabled;
} else {
canToggleLocation = LocationSettings::ToggleEnabled;
}

if (m_canToggleLocation != canToggleLocation) {
m_canToggleLocation = canToggleLocation;
emit q->locationCanToggleChanged();
emit q->gpsCanToggleChanged();
}
}
14 changes: 14 additions & 0 deletions src/locationsettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ class SYSTEMSETTINGS_EXPORT LocationSettings : public QObject
Q_OBJECT

Q_PROPERTY(bool locationEnabled READ locationEnabled WRITE setLocationEnabled NOTIFY locationEnabledChanged)
Q_PROPERTY(LocationCanToggle locationCanToggle READ locationCanToggle NOTIFY locationCanToggleChanged)
Q_PROPERTY(LocationMode locationMode READ locationMode WRITE setLocationMode NOTIFY locationModeChanged)
Q_PROPERTY(QStringList pendingAgreements READ pendingAgreements NOTIFY pendingAgreementsChanged)
Q_PROPERTY(DataSources allowedDataSources READ allowedDataSources WRITE setAllowedDataSources NOTIFY allowedDataSourcesChanged)
Q_PROPERTY(bool gpsAvailable READ gpsAvailable CONSTANT)
Q_PROPERTY(bool gpsEnabled READ gpsEnabled WRITE setGpsEnabled NOTIFY gpsEnabledChanged)
Q_PROPERTY(LocationCanToggle gpsCanToggle READ gpsCanToggle NOTIFY gpsCanToggleChanged)
Q_PROPERTY(bool gpsFlightMode READ gpsFlightMode WRITE setGpsFlightMode NOTIFY gpsFlightModeChanged)
Q_PROPERTY(QStringList locationProviders READ locationProviders CONSTANT)

Expand All @@ -80,6 +82,7 @@ class SYSTEMSETTINGS_EXPORT LocationSettings : public QObject

Q_ENUMS(OnlineAGpsState)
Q_ENUMS(LocationMode)
Q_ENUMS(LocationCanToggle)

public:
enum Mode {
Expand All @@ -100,6 +103,12 @@ class SYSTEMSETTINGS_EXPORT LocationSettings : public QObject
CustomMode
};

enum LocationCanToggle {
ToggleEnabled,
CanOnlyEnable,
ToggleDisabled
};

// Data sources are grouped roughly by type,
// with gaps left for future expansion.
enum DataSource {
Expand Down Expand Up @@ -137,6 +146,9 @@ class SYSTEMSETTINGS_EXPORT LocationSettings : public QObject
void setGpsFlightMode(bool flightMode);
bool gpsAvailable() const;

LocationCanToggle locationCanToggle() const;
LocationCanToggle gpsCanToggle() const;

QStringList locationProviders() const;
LocationProvider providerInfo(const QString &name) const;
bool updateLocationProvider(const QString &name, const LocationProvider &providerState);
Expand Down Expand Up @@ -165,7 +177,9 @@ class SYSTEMSETTINGS_EXPORT LocationSettings : public QObject

signals:
void locationEnabledChanged();
void locationCanToggleChanged();
void gpsEnabledChanged();
void gpsCanToggleChanged();
void gpsFlightModeChanged();
void locationModeChanged();
void pendingAgreementsChanged();
Expand Down
4 changes: 4 additions & 0 deletions src/locationsettings_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <QHash>

#include <nemo-dbus/interface.h>
#include <mce-qt5/qmcecallstate.h>

#include <sailfishkeyprovider_processmutex.h>

Expand Down Expand Up @@ -79,11 +80,14 @@ class LocationSettingsPrivate : public QObject
NetworkManager *m_connMan;
NetworkTechnology *m_gpsTech;
NemoDBus::Interface *m_gpsTechInterface;
QMceCallState *m_mceCallState;
LocationSettings::LocationCanToggle m_canToggleLocation;

private slots:
void readSettings();
void findGpsTech();
void gpsTechPropertyChanged(const QString &propertyName, const QDBusVariant &value);
void onCallStateChanged();
};

// TODO: replace this with DBus calls to a central settings service...
Expand Down