Rework autodetection to better handle ambiguous cases.

The old detection code simply assumed only one player to be connected and threw
all information it could find together, causing wrong results in various cases.
Rewrite it to better handle this.

- Don't expect only a single player to be attached. Return a list of players
  found instead. The configuration dialog can then show the user a list to
  select from. This is not implemented yet, the configuration dialog will only
  show the devices found and tell the user to disconnect all but one.

- Handle ambiguous cases better. A player found that is ambiguous is treated as
  such, and if refining the result didn't lead to a single match for each
  possible player a single entry is created that indicates it being ambiguous.

The current code needs a bit of additional cleanup (the result exposes internal
data to the caller it shouldn't) to be done later.

Change-Id: I22dc2f73cdd6a9d90c34f3a0703aa9e0a2d2087a

2 files changed, 260 insertions, 148 deletions

diff --git a/rbutil/rbutilqt/base/autodetection.cpp b/rbutil/rbutilqt/base/autodetection.cpp
index de3765ffb1..2d7476502a 100644
--- a/rbutil/rbutilqt/base/autodetection.cpp
+++ b/rbutil/rbutilqt/base/autodetection.cpp
@@ -33,20 +33,101 @@ Autodetection::Autodetection(QObject* parent): QObject(parent)
 {
 }
 
-bool Autodetection::detect()
+
+bool Autodetection::detect(void)
 {
-    m_device = "";
-    m_mountpoint = "";
-    m_errdev = "";
-    m_usberr = "";
+    QMap<PlayerStatus, QString> states;
+    states[PlayerOk] = "Ok";
+    states[PlayerAmbiguous] = "Ambiguous";
+    states[PlayerError] = "Error";
+    states[PlayerIncompatible] = "Incompatible";
+    states[PlayerMtpMode] = "MtpMode";
+
+    // clear detection state
+    m_detected.clear();
 
     detectUsb();
+    mergeMounted();
+    mergePatcher();
+    // if any entry with usbdevices containing a value is left that entry
+    // hasn't been merged later. This indicates a problem during detection
+    // (ambiguous player but refining it failed). In this case create an entry
+    // for eacho of those so the user can select.
+    for(int i = 0; i < m_detected.size(); ++i) {
+        int j = m_detected.at(i).usbdevices.size();
+        if(j > 0) {
+            struct Detected entry = m_detected.takeAt(i);
+            while(j--) {
+                struct Detected d;
+                d.device = entry.usbdevices.at(j);
+                d.mountpoint = entry.mountpoint;
+                d.status = PlayerAmbiguous;
+                m_detected.append(d);
+            }
+        }
+    }
+    for(int i = 0; i < m_detected.size(); ++i) {
+        qDebug() << "[Autodetect] Detected player:" << m_detected.at(i).device
+                 << "at" << m_detected.at(i).mountpoint << states[m_detected.at(i).status];
+    }
+
+    return m_detected.size() > 0;
+}
+
+
+/** @brief detect devices based on usb pid / vid.
+ */
+void Autodetection::detectUsb()
+{
+    // usbids holds the mapping in the form
+    // ((VID<<16)|(PID)), targetname
+    // the ini file needs to hold the IDs as hex values.
+    QMap<int, QStringList> usbids = SystemInfo::usbIdMap(SystemInfo::MapDevice);
+    QMap<int, QStringList> usberror = SystemInfo::usbIdMap(SystemInfo::MapError);
+    QMap<int, QStringList> usbincompat = SystemInfo::usbIdMap(SystemInfo::MapIncompatible);
+
+    // usb pid detection
+    QList<uint32_t> attached;
+    attached = System::listUsbIds();
 
-    // Try detection via rockbox.info / rbutil.log
+    int i = attached.size();
+    while(i--) {
+        if(usbids.contains(attached.at(i))) {
+            // we found a USB device that might be ambiguous.
+            struct Detected d;
+            d.status = PlayerOk;
+            d.usbdevices = usbids.value(attached.at(i));
+            m_detected.append(d);
+            qDebug() << "[USB] detected supported player" << d.usbdevices;
+        }
+        if(usberror.contains(attached.at(i))) {
+            struct Detected d;
+            d.status = PlayerMtpMode;
+            d.device = usbids.value(attached.at(i)).at(0);
+            m_detected.append(d);
+            qDebug() << "[USB] detected problem with player" << d.device;
+        }
+        QString idstring = QString("%1").arg(attached.at(i), 8, 16, QChar('0'));
+        if(!SystemInfo::platformValue(idstring, SystemInfo::CurName).toString().isEmpty()) {
+            struct Detected d;
+            d.status = PlayerIncompatible;
+            d.device = idstring;
+            m_detected.append(d);
+            qDebug() << "[USB] detected incompatible player" << d.device;
+        }
+    }
+}
+
+
+// Merge players detected by checking mounted filesystems for known files:
+// - rockbox-info.txt / rbutil.log
+// - player specific files
+void Autodetection::mergeMounted(void)
+{
     QStringList mounts = Utils::mountpoints(Utils::MountpointsSupported);
     qDebug() << "[Autodetect] paths to check:" << mounts;
 
-    for(int i=0; i< mounts.size();i++)
+    for(int i = 0; i < mounts.size(); i++)
     {
         // do the file checking
         QDir dir(mounts.at(i));
@@ -57,11 +138,20 @@ bool Autodetection::detect()
                 QSettings log(mounts.at(i) + "/.rockbox/rbutil.log",
                               QSettings::IniFormat, this);
                 if(!log.value("platform").toString().isEmpty()) {
-                    if(m_device.isEmpty())
-                        m_device = log.value("platform").toString();
-                    m_mountpoint = mounts.at(i);
-                    qDebug() << "[Autodetect] rbutil.log detected:" << m_device << m_mountpoint;
-                    return true;
+                    int index = findDetectedDevice(log.value("platform").toString());
+                    struct Detected d;
+                    d.device = log.value("platform").toString();
+                    d.mountpoint = mounts.at(i);
+                    d.status = PlayerOk;
+                    if(index < 0) {
+                        m_detected.append(d);
+                    }
+                    else {
+                        m_detected.takeAt(index);
+                        m_detected.append(d);
+                    }
+                    qDebug() << "[Autodetect] rbutil.log detected:"
+                             << log.value("platform").toString() << mounts.at(i);
                 }
             }
 
@@ -69,14 +159,20 @@ bool Autodetection::detect()
             RockboxInfo info(mounts.at(i));
             if(info.success())
             {
-                if(m_device.isEmpty())
-                {
-                    m_device = info.target();
+                int index = findDetectedDevice(info.target());
+                struct Detected d;
+                d.device = info.target();
+                d.mountpoint = mounts.at(i);
+                d.status = PlayerOk;
+                if(index < 0) {
+                    m_detected.append(d);
+                }
+                else {
+                    m_detected.takeAt(index);
+                    m_detected.append(d);
                 }
-                m_mountpoint = mounts.at(i);
                 qDebug() << "[Autodetect] rockbox-info.txt detected:"
-                         << m_device << m_mountpoint;
-                return true;
+                         << info.target() << mounts.at(i);
             }
 
             // check for some specific files in root folder
@@ -85,31 +181,61 @@ bool Autodetection::detect()
             if(rootentries.contains("archos.mod", Qt::CaseInsensitive))
             {
                 // archos.mod in root folder -> Archos Player
-                m_device = "player";
-                m_mountpoint = mounts.at(i);
-                return true;
+                struct Detected d;
+                d.device = "player";
+                d.mountpoint = mounts.at(i);
+                d.status = PlayerOk;
+                m_detected.append(d);
             }
             if(rootentries.contains("ONDIOST.BIN", Qt::CaseInsensitive))
             {
                 // ONDIOST.BIN in root -> Ondio FM
-                m_device = "ondiofm";
-                m_mountpoint = mounts.at(i);
-                return true;
+                struct Detected d;
+                d.device = "ondiofm";
+                d.mountpoint = mounts.at(i);
+                d.status = PlayerOk;
+                int index = findDetectedDevice("ondiofm");
+                if(index < 0) {
+                    m_detected.append(d);
+                }
+                else {
+                    m_detected.takeAt(index);
+                    m_detected.append(d);
+                }
             }
             if(rootentries.contains("ONDIOSP.BIN", Qt::CaseInsensitive))
             {
                 // ONDIOSP.BIN in root -> Ondio SP
-                m_device = "ondiosp";
-                m_mountpoint = mounts.at(i);
-                return true;
+                struct Detected d;
+                d.device = "ondiosp";
+                d.mountpoint = mounts.at(i);
+                d.status = PlayerOk;
+                int index = findDetectedDevice("ondiosp");
+                if(index < 0) {
+                    m_detected.append(d);
+                }
+                else {
+                    m_detected.takeAt(index);
+                    m_detected.append(d);
+                }
             }
             if(rootentries.contains("ajbrec.ajz", Qt::CaseInsensitive))
             {
                 qDebug() << "[Autodetect] ajbrec.ajz found. Trying detectAjbrec()";
-                if(detectAjbrec(mounts.at(i))) {
-                    m_mountpoint = mounts.at(i);
-                    qDebug() << "[Autodetect]" << m_device;
-                    return true;
+                struct Detected d;
+                d.device = detectAjbrec(mounts.at(i));
+                d.mountpoint = mounts.at(i);
+                d.status = PlayerOk;
+                if(!d.device.isEmpty()) {
+                    qDebug() << "[Autodetect]" << d.device;
+                    int index = findDetectedDevice("ondiosp");
+                    if(index < 0) {
+                        m_detected.append(d);
+                    }
+                    else {
+                        m_detected.takeAt(index);
+                        m_detected.append(d);
+                    }
                 }
             }
             // detection based on player specific folders
@@ -118,23 +244,50 @@ bool Autodetection::detect()
             if(rootfolders.contains("GBSYSTEM", Qt::CaseInsensitive))
             {
                 // GBSYSTEM folder -> Gigabeat
-                m_device = "gigabeatf";
-                m_mountpoint = mounts.at(i);
-                return true;
+                struct Detected d;
+                d.device = "gigabeatf";
+                d.mountpoint = mounts.at(i);
+                int index = findDetectedDevice("ondiosp");
+                if(index < 0) {
+                    m_detected.append(d);
+                }
+                else {
+                    m_detected.takeAt(index);
+                    m_detected.append(d);
+                }
             }
-#if defined(Q_OS_WIN32)
-            // on windows, try to detect the drive letter of an Ipod
-            if(rootfolders.contains("iPod_Control", Qt::CaseInsensitive))
-            {
-                // iPod_Control folder -> Ipod found
-                // detecting of the Ipod type is done below using ipodpatcher
-                m_mountpoint = mounts.at(i);
+        }
+    }
+#if 0
+    // Ipods have a folder "iPod_Control" in the root.
+    for(int i = 0; i < m_detected.size(); ++i) {
+        struct Detected entry = m_detected.at(i);
+        for(int j = 0; j < entry.usbdevices.size(); ++j) {
+            // limit this to Ipods only.
+            if(!entry.usbdevices.at(j).startsWith("ipod")
+                    && !entry.device.startsWith("ipod")) {
+                continue;
+            }
+            // look for iPod_Control on all supported volumes.
+            for(int k = 0; k < mounts.size(); k++) {
+                QDir root(mounts.at(k));
+                QStringList rootfolders = root.entryList(QDir::Dirs
+                        | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System);
+                if(rootfolders.contains("iPod_Control", Qt::CaseInsensitive)) {
+                    entry.mountpoint = mounts.at(k);
+                    m_detected.takeAt(i);
+                    m_detected.append(entry);
+                }
             }
-#endif
         }
-
     }
+#endif
 
+}
+
+
+void Autodetection::mergePatcher(void)
+{
     int n;
     // try ipodpatcher
     // initialize sector buffer. Needed.
@@ -142,13 +295,9 @@ bool Autodetection::detect()
     ipod.sectorbuf = NULL;
     ipod_alloc_buffer(&ipod, BUFFER_SIZE);
     n = ipod_scan(&ipod);
+    // FIXME: handle more than one Ipod connected in ipodpatcher.
     if(n == 1) {
         qDebug() << "[Autodetect] Ipod found:" << ipod.modelstr << "at" << ipod.diskname;
-        // if the found ipod is a macpod also notice it as device with problem.
-        if(ipod.macpod)
-            m_errdev = ipod.targetname;
-        else
-            m_device = ipod.targetname;
         // since resolveMountPoint is doing exact matches we need to select
         // the correct partition.
         QString mp(ipod.diskname);
@@ -158,8 +307,22 @@ bool Autodetection::detect()
 #ifdef Q_OS_MACX
         mp.append("s2");
 #endif
-        m_mountpoint = Utils::resolveMountPoint(mp);
-        return true;
+        struct Detected d;
+        d.device = ipod.targetname;
+        d.mountpoint = Utils::resolveMountPoint(mp);
+        // if the found ipod is a macpod also notice it as device with problem.
+        if(ipod.macpod)
+            d.status = PlayerWrongFilesystem;
+        else
+            d.status = PlayerOk;
+        int index = findDetectedDevice(ipod.targetname);
+        if(index < 0) {
+            m_detected.append(d);
+        }
+        else {
+            m_detected.takeAt(index);
+            m_detected.append(d);
+        }
     }
     else {
         qDebug() << "[Autodetect] ipodpatcher: no Ipod found." << n;
@@ -172,8 +335,8 @@ bool Autodetection::detect()
     sansa_alloc_buffer(&sansa, BUFFER_SIZE);
     n = sansa_scan(&sansa);
     if(n == 1) {
-        qDebug() << "[Autodetect] Sansa found:" << sansa.targetname << "at" << sansa.diskname;
-        m_device = QString("sansa%1").arg(sansa.targetname);
+        qDebug() << "[Autodetect] Sansa found:"
+                 << sansa.targetname << "at" << sansa.diskname;
         QString mp(sansa.diskname);
 #ifdef Q_OS_LINUX
         mp.append("1");
@@ -181,97 +344,33 @@ bool Autodetection::detect()
 #ifdef Q_OS_MACX
         mp.append("s1");
 #endif
-        m_mountpoint = Utils::resolveMountPoint(mp);
-        return true;
+        struct Detected d;
+        d.device = QString("sansa%1").arg(sansa.targetname);
+        d.mountpoint = Utils::resolveMountPoint(mp);
+        d.status = PlayerOk;
+        int index = findDetectedDevice(d.device);
+        if(index < 0) {
+            m_detected.append(d);
+        }
+        else {
+            m_detected.takeAt(index);
+            m_detected.append(d);
+        }
     }
     else {
         qDebug() << "[Autodetect] sansapatcher: no Sansa found." << n;
     }
     sansa_dealloc_buffer(&sansa);
-
-    if(m_mountpoint.isEmpty() && m_device.isEmpty()
-            && m_errdev.isEmpty() && m_incompat.isEmpty())
-        return false;
-    return true;
-}
-
-
-/** @brief detect devices based on usb pid / vid.
- *  @return true upon success, false otherwise.
- */
-bool Autodetection::detectUsb()
-{
-    // usbids holds the mapping in the form
-    // ((VID<<16)|(PID)), targetname
-    // the ini file needs to hold the IDs as hex values.
-    QMap<int, QStringList> usbids = SystemInfo::usbIdMap(SystemInfo::MapDevice);
-    QMap<int, QStringList> usberror = SystemInfo::usbIdMap(SystemInfo::MapError);
-    QMap<int, QStringList> usbincompat = SystemInfo::usbIdMap(SystemInfo::MapIncompatible);
-
-    // usb pid detection
-    QList<uint32_t> attached;
-    attached = System::listUsbIds();
-
-    int i = attached.size();
-    while(i--) {
-        if(usbids.contains(attached.at(i))) {
-            m_device = usbids.value(attached.at(i)).at(0);
-            qDebug() << "[USB] detected supported player" << m_device;
-            return true;
-        }
-        if(usberror.contains(attached.at(i))) {
-            m_usberr = usberror.value(attached.at(i)).at(0);
-            qDebug() << "[USB] detected problem with player" << m_usberr;
-            return true;
-        }
-        QString idstring = QString("%1").arg(attached.at(i), 8, 16, QChar('0'));
-        if(!SystemInfo::platformValue(idstring, SystemInfo::CurName).toString().isEmpty()) {
-            m_incompat = idstring;
-            qDebug() << "[USB] detected incompatible player" << m_incompat;
-            return true;
-        }
-    }
-    return false;
 }
 
 
-QList<struct Autodetection::Detected> Autodetection::detected(void)
-{
-    struct Detected d;
-
-    m_detected.clear();
-    if(!m_device.isEmpty()) {
-        d.device = m_device;
-        d.mountpoint = m_mountpoint;
-        d.status = PlayerOk;
-        m_detected.append(d);
-    }
-    else if(!m_errdev.isEmpty()) {
-        d.device = m_errdev;
-        d.status = PlayerWrongFilesystem;
-        m_detected.append(d);
-    }
-    else if(!m_usberr.isEmpty()) {
-        d.device = m_usberr;
-        d.status = PlayerMtpMode;
-        m_detected.append(d);
-    }
-    else if(!m_incompat.isEmpty()) {
-        d.device = m_incompat;
-        d.status = PlayerIncompatible;
-        m_detected.append(d);
-    }
-
-    return m_detected;
-}
-
-
-bool Autodetection::detectAjbrec(QString root)
+QString Autodetection::detectAjbrec(QString root)
 {
     QFile f(root + "/ajbrec.ajz");
     char header[24];
     f.open(QIODevice::ReadOnly);
-    if(!f.read(header, 24)) return false;
+    if(!f.read(header, 24)) return QString();
+    f.close();
 
     // check the header of the file.
     // recorder v1 had a 6 bytes sized header
@@ -283,32 +382,44 @@ bool Autodetection::detectAjbrec(QString root)
     qDebug() << "[Autodetect] ABJREC possible bin length:" << len
              << "file len:" << f.size();
     if((f.size() - 6) == len)
-        m_device = "recorder";
+        return "recorder";
 
     // size didn't match, now we need to assume we have a headerlength of 24.
     switch(header[11]) {
         case 2:
-            m_device = "recorderv2";
+            return "recorderv2";
             break;
 
         case 4:
-            m_device = "fmrecorder";
+            return "fmrecorder";
             break;
 
         case 8:
-            m_device = "ondiofm";
+            return "ondiofm";
             break;
 
         case 16:
-            m_device = "ondiosp";
+            return "ondiosp";
             break;
 
         default:
             break;
     }
-    f.close();
-
-    if(m_device.isEmpty()) return false;
-    return true;
+    return QString();
 }
 
+
+int Autodetection::findDetectedDevice(QString device)
+{
+    int i = m_detected.size();
+    while(i--) {
+        if(m_detected.at(i).usbdevices.contains(device))
+            return i;
+    }
+    i = m_detected.size();
+    while(i--) {
+        if(m_detected.at(i).device == device)
+            return i;
+    }
+    return -1;
+}
diff --git a/rbutil/rbutilqt/base/autodetection.h b/rbutil/rbutilqt/base/autodetection.h
index 65a24abb8f..c3c7fc9168 100644
--- a/rbutil/rbutilqt/base/autodetection.h
+++ b/rbutil/rbutilqt/base/autodetection.h
@@ -25,6 +25,7 @@
 #include <QObject>
 #include <QString>
 #include <QList>
+#include <QStringList>
 
 class Autodetection :public QObject
 {
@@ -39,29 +40,29 @@ public:
         PlayerMtpMode,
         PlayerWrongFilesystem,
         PlayerError,
+        PlayerAmbiguous,
     };
 
     struct Detected {
         QString device;
+        QStringList usbdevices;
         QString mountpoint;
         enum PlayerStatus status;
     };
 
     bool detect();
 
-    QList<struct Detected> detected(void);
+    QList<struct Detected> detected(void) { return m_detected; }
 
 private:
     QString resolveMountPoint(QString);
-    bool detectUsb(void);
-    bool detectAjbrec(QString);
+    void detectUsb(void);
+    void mergeMounted(void);
+    void mergePatcher(void);
+    QString detectAjbrec(QString);
+    int findDetectedDevice(QString device);
 
     QList<struct Detected> m_detected;
-    QString m_device;
-    QString m_mountpoint;
-    QString m_errdev;
-    QString m_usberr;
-    QString m_incompat;
     QList<int> m_usbconid;
 };