From c34fead27404ea494c8642bf94a9f5c6f85fd8b9 Mon Sep 17 00:00:00 2001 From: Alex Merry Date: Thu, 3 Nov 2011 22:51:50 +0000 Subject: Fixes for the Player interface testing code --- metadatamodel.cpp | 11 ++++- mpris2/interfacetest.cpp | 16 ++++--- mpris2/interfacetest.h | 2 + mpris2/playerinterfacetest.cpp | 106 ++++++++++++++++++++++++++++++----------- mpris2/playertestwidget.cpp | 9 +++- window.cpp | 2 + 6 files changed, 109 insertions(+), 37 deletions(-) diff --git a/metadatamodel.cpp b/metadatamodel.cpp index 3484ad7..513e3d0 100644 --- a/metadatamodel.cpp +++ b/metadatamodel.cpp @@ -15,6 +15,8 @@ * along with this program. If not, see . */ #include "metadatamodel.h" +#include +#include MetadataModel::MetadataModel(QObject* parent) : QAbstractTableModel(parent) @@ -49,7 +51,14 @@ QVariant MetadataModel::data(const QModelIndex& index, int role) const if (index.column() == 0) { return key; } else { - return m_metadata[key]; + if (m_metadata[key].canConvert()) { + qDebug() << "Unmarshalled type in metadata map for entry" << key; + return QVariant(); + } else if (m_metadata[key].canConvert()) { + return m_metadata[key].value().path(); + } else { + return m_metadata[key]; + } } } } diff --git a/mpris2/interfacetest.cpp b/mpris2/interfacetest.cpp index 8422921..11fdd84 100644 --- a/mpris2/interfacetest.cpp +++ b/mpris2/interfacetest.cpp @@ -135,11 +135,15 @@ bool InterfaceTest::checkPropValid(const QString& propName, QVariant::Type expTy return false; } else if (oldProps.contains(propName)) { // FIXME: QVariant equality only works for builtin types - if (props[propName] != oldProps[propName]) { - outOfDateProperties.insert(propName, props[propName]); - props[propName] = oldProps[propName]; - // don't check right now - return false; + if (props[propName].type() < QVariant::UserType) { + if (props[propName] != oldProps[propName]) { + outOfDateProperties.insert(propName, props[propName]); + props[propName] = oldProps[propName]; + // don't check right now + return false; + } + } else { + qDebug() << "Could not check equality for" << propName; } } return true; @@ -167,7 +171,7 @@ void InterfaceTest::initialTest() iface->service(), iface->path(), DBUS_PROPS_IFACE, - "propertiesChanged", /* signature, */ + "PropertiesChanged", /* signature, */ this, SLOT( _m_propertiesChanged(QString,QVariantMap,QStringList,QDBusMessage))); connectSignals(); diff --git a/mpris2/interfacetest.h b/mpris2/interfacetest.h index 8d62b2c..cfc174c 100644 --- a/mpris2/interfacetest.h +++ b/mpris2/interfacetest.h @@ -25,6 +25,7 @@ class QDBusInterface; class QDBusMessage; class QTimer; #include +#include #include namespace Mpris2 @@ -130,6 +131,7 @@ namespace Mpris2 QDBusInterface* iface; QVariantMap props; QVariantMap outOfDateProperties; // prop name -> new value + QStringList propsNotUpdated; private: QDBusInterface* propsIface; diff --git a/mpris2/playerinterfacetest.cpp b/mpris2/playerinterfacetest.cpp index d663809..88e55cf 100644 --- a/mpris2/playerinterfacetest.cpp +++ b/mpris2/playerinterfacetest.cpp @@ -29,6 +29,7 @@ PlayerInterfaceTest::PlayerInterfaceTest(const QString& service, QObject* parent { m_pos = -1; m_currentRate = 0.0; + propsNotUpdated << "Position"; } PlayerInterfaceTest::~PlayerInterfaceTest() @@ -81,6 +82,10 @@ void PlayerInterfaceTest::checkUpdatedProperty(const QString& propName) void PlayerInterfaceTest::checkProps(const QVariantMap& oldProps) { + // position is time-sensitive; check it first + checkPosition(oldProps); + checkPredictedPosition(); + checkPropValid("CanControl", QVariant::Bool, oldProps); checkControlProp("CanGoNext", oldProps); checkControlProp("CanGoPrevious", oldProps); @@ -105,7 +110,6 @@ void PlayerInterfaceTest::checkProps(const QVariantMap& oldProps) checkMaximumRate(oldProps); checkPropValid("Rate", QVariant::Double, oldProps); checkMetadata(oldProps); - checkPosition(oldProps); checkConsistency(); } @@ -156,9 +160,9 @@ void PlayerInterfaceTest::checkPlaybackStatus(const QVariantMap& oldProps) if (!checkPropValid("PlaybackStatus", QVariant::String, oldProps)) return; QString playbackStatus = properties().value("PlaybackStatus").toString(); - if (playbackStatus != "None" && - playbackStatus != "Track" && - playbackStatus != "Playlist") + if (playbackStatus != "Playing" && + playbackStatus != "Paused" && + playbackStatus != "Stopped") { emit interfaceError(Property, "PlaybackStatus", "Invalid value: '" + playbackStatus + "'"); @@ -202,31 +206,68 @@ void PlayerInterfaceTest::checkRate(const QVariantMap& oldProps) } } +static bool compare(const QVariantMap& one, const QVariantMap& other) +{ + if (one.size() != other.size()) + return false; + + QVariantMap::const_iterator it1 = one.begin(); + QVariantMap::const_iterator it2 = other.begin(); + + while (it1 != one.end()) { + if (it1.value().userType() != it2.value().userType()) + return false; + if (!(it1.value() == it2.value())) { + if (it1.value().userType() == qMetaTypeId()) { + if (!(it1.value().value() == it2.value().value())) + return false; + } + } + if (qMapLessThanKey(it1.key(), it2.key()) || qMapLessThanKey(it2.key(), it1.key())) + return false; + ++it2; + ++it1; + } + return true; +} + void PlayerInterfaceTest::checkMetadata(const QVariantMap& oldProps) { - if (!properties().contains("Metadata")) { + if (!props.contains("Metadata")) { emit interfaceError(Property, "Metadata", "Property Metadata is missing"); return; } - if (!properties().value("Metadata").canConvert()) { + if (props["Metadata"].type() != QVariant::Map && + !props.value("Metadata").canConvert()) + { const char * gotTypeCh = QDBusMetaType::typeToSignature(props["Metadata"].userType()); QString gotType = gotTypeCh ? QString::fromAscii(gotTypeCh) : ""; emit interfaceError(Property, "Metadata", "Property Metadata has type " + gotType + ", but should be type a{sv}"); return; } QVariantMap metadata; - QDBusArgument arg = properties().value("Metadata").value(); - arg >> metadata; + if (props["Metadata"].type() == QVariant::Map) { + metadata = props["Metadata"].toMap(); + } else { + QDBusArgument arg = props["Metadata"].value(); + arg >> metadata; + // replace the entry in the properties array + props["Metadata"] = metadata; + } if (oldProps.contains("Metadata") && - oldProps.value("Metadata").canConvert()) + oldProps.value("Metadata").canConvert(QVariant::Map)) { - QVariantMap oldMetadata; - oldProps.value("Metadata").value() >> oldMetadata; - if (metadata != oldMetadata) { - outOfDateProperties.insert("Metadata", props["Metadata"]); + QVariantMap oldMetadata = oldProps.value("Metadata").toMap(); + + // custom compare fn as we're expecting a QDBusObjectPath entry + if (!compare(metadata, oldMetadata)) { + outOfDateProperties["Metadata"] = props["Metadata"]; props["Metadata"] = oldProps["Metadata"]; return; + } else { + // same as before; don't re-run checks + return; } } if (metadata.isEmpty()) { @@ -240,8 +281,8 @@ void PlayerInterfaceTest::checkMetadata(const QVariantMap& oldProps) "No mpris:trackid entry for the current track"); } else if (metadata.value("mpris:trackid").userType() != qMetaTypeId()) { emit interfaceError(Property, "Metadata", - "mpris:trackid entry is not a D-Bus object path"); - } else if (metadata.value("mpris:trackid").toString().isEmpty()) { + "mpris:trackid entry was not sent as a D-Bus object path (D-Bus type 'o')"); + } else if (metadata.value("mpris:trackid").value().path().isEmpty()) { emit interfaceError(Property, "Metadata", "mpris:trackid entry is an empty path"); } @@ -261,7 +302,7 @@ void PlayerInterfaceTest::checkMetadata(const QVariantMap& oldProps) "mpris:artUrl references a file that does not exist"); } } - // FIXME: check network files + // TODO: check network files } } @@ -389,7 +430,6 @@ void PlayerInterfaceTest::checkConsistency(const QVariantMap& oldProps) { checkRateConsistency(); checkPositionConsistency(); - checkPredictedPosition(); } void PlayerInterfaceTest::checkPositionConsistency(const QVariantMap& oldProps) @@ -511,23 +551,33 @@ void PlayerInterfaceTest::checkPredictedPosition() m_posLastUpdated = QTime::currentTime(); updateCurrentRate(); + qint64 diffMillis = (position - predictedPos) / 1000; // allow 1s of error - const qint64 allowance = 1000000; - if (position - predictedPos > allowance || - position - predictedPos < -allowance) + const qint64 allowanceMillis = 1000; + if (diffMillis > allowanceMillis || + diffMillis < -allowanceMillis) { - qint64 diffMillis = (position - predictedPos) / 1000; - qreal diffSecs = (qreal)diffMillis / 1000.0; + qreal diffSecs = ((qreal)diffMillis) / 1000.0; + qreal predictedPosSecs = ((qreal)(predictedPos/1000)) / 1000.0; + qreal positionSecs = ((qreal)(position/1000)) / 1000.0; if (diffMillis > 0) { emit interfaceWarning(Property, "Position", - "Position is " + - QString::number(diffSecs, 'g', 2) + - "ms ahead of what was predicted from Rate"); + "Position (" + + QString::number(positionSecs, 'f', 2) + + "s) is " + + QString::number(diffSecs, 'f', 2) + + "s ahead of what was predicted from Rate (" + + QString::number(predictedPosSecs, 'f', 2) + + "s)"); } else { emit interfaceWarning(Property, "Position", - "Position is " + QString::number(-diffMillis) + - QString::number(-diffSecs, 'g', 2) + - "ms behind of what was predicted from Rate"); + "Position (" + + QString::number(positionSecs, 'f', 2) + + "s) is " + + QString::number(-diffSecs, 'f', 2) + + "s ahead of what was predicted from Rate (" + + QString::number(predictedPosSecs, 'f', 2) + + "s)"); } } } diff --git a/mpris2/playertestwidget.cpp b/mpris2/playertestwidget.cpp index 8ed42fe..c35ee5c 100644 --- a/mpris2/playertestwidget.cpp +++ b/mpris2/playertestwidget.cpp @@ -20,8 +20,9 @@ #include "playertestwidget.h" #include "playerinterfacetest.h" -#include -#include +#include +#include +#include using namespace Mpris2; @@ -141,7 +142,11 @@ void PlayerTestWidget::propertiesChanged(const QStringList& properties) estPosTimer->start(); } if (test->properties().contains("Metadata")) { + if (test->properties().value("Metadata").type() != QVariant::Map) { + qDebug() << "Metadata map was wrong type"; + } metadataModel->setMetadata(test->properties().value("Metadata").toMap()); + ui.metadataTableView->setEnabled(true); } } diff --git a/window.cpp b/window.cpp index 368d80a..7aed710 100644 --- a/window.cpp +++ b/window.cpp @@ -138,6 +138,8 @@ void Window::setPlayer(const QString& dbusAddress) { clear(); + m_currentPlayer = dbusAddress; + qDebug() << "Connecting to player" << dbusAddress; m_tabWidget = new QTabWidget(this); -- cgit v1.2.3