From 1a398fb5cf8285e75d402e8a9b04244f50c560e4 Mon Sep 17 00:00:00 2001 From: Johannes Dewender Date: Sun, 29 Dec 2013 03:09:34 +0100 Subject: [PATCH 1/5] PICARD-123: actually use default drive on Mac This doesn't implement an automatic list of available drives on Mac, but adds the libdiscid default drive to the list of drive candidates. This should make disc lookups available on Mac without specifying the drive (as /dev/rdisk*) in the settings. Note that setting things like "1" or "2" as drives should also work with a recent libdiscid. (>= 0.6.0). --- picard/util/cdrom.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/picard/util/cdrom.py b/picard/util/cdrom.py index 64ae157d3..d7bf2acbe 100644 --- a/picard/util/cdrom.py +++ b/picard/util/cdrom.py @@ -95,6 +95,12 @@ else: def get_cdrom_drives(): from picard import config - # Need to filter out empty strings, particularly if the device list is empty - return filter(lambda string: (string != u''), - [d.strip() for d in config.setting["cd_lookup_device"].split(",")]) + + drives = list(DEFAULT_DRIVES) + # Need to filter out empty strings, + # particularly if the device list is empty + for device in config.setting["cd_lookup_device"].split(","): + if device.strip() != u'': + drives.append(device.strip()) + + return sorted(uniqify(drives)) From 971927529f73aee6e69532f2de848ea2f32415bc Mon Sep 17 00:00:00 2001 From: Johannes Dewender Date: Sun, 29 Dec 2013 17:00:27 +0100 Subject: [PATCH 2/5] use only one definition of get_cdrom_drives() --- picard/util/cdrom.py | 56 +++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/picard/util/cdrom.py b/picard/util/cdrom.py index d7bf2acbe..396bf0715 100644 --- a/picard/util/cdrom.py +++ b/picard/util/cdrom.py @@ -19,7 +19,13 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. import sys -from PyQt4.QtCore import QFile, QRegExp +import traceback +if sys.platform == 'win32': + from ctypes import windll + +from PyQt4.QtCore import QFile, QRegExp, QIODevice, QString + +from picard import config from picard.util import uniqify DEFAULT_DRIVES = [] @@ -36,36 +42,38 @@ try: if device: DEFAULT_DRIVES = [device] except: - import traceback print(traceback.format_exc()) LINUX_CDROM_INFO = '/proc/sys/dev/cdrom/info' +# if get_cdrom_drives() lists all drives available on the machine if sys.platform == 'win32': AUTO_DETECT_DRIVES = True - from ctypes import windll - GetLogicalDrives = windll.kernel32.GetLogicalDrives - GetDriveType = windll.kernel32.GetDriveTypeA - DRIVE_CDROM = 5 +elif sys.platform == 'linux2' and QFile.exists(LINUX_CDROM_INFO): + AUTO_DETECT_DRIVES = True +else: + # There might be more drives we couldn't detect + # setting uses a text field instead of a drop-down + AUTO_DETECT_DRIVES = False - def get_cdrom_drives(): - drives = list(DEFAULT_DRIVES) +def get_cdrom_drives(): + # add default drive from libdiscid to the list + drives = list(DEFAULT_DRIVES) + + if sys.platform == 'win32': + GetLogicalDrives = windll.kernel32.GetLogicalDrives + GetDriveType = windll.kernel32.GetDriveTypeA + DRIVE_CDROM = 5 mask = GetLogicalDrives() for i in range(26): if mask >> i & 1: drive = chr(i + ord("A")) + ":" if GetDriveType(drive) == DRIVE_CDROM: drives.append(drive) - return sorted(uniqify(drives)) -elif sys.platform == 'linux2' and QFile.exists(LINUX_CDROM_INFO): - AUTO_DETECT_DRIVES = True - from PyQt4.QtCore import QIODevice, QString - - # Read info from /proc/sys/dev/cdrom/info - def get_cdrom_drives(): - drives = list(DEFAULT_DRIVES) + elif sys.platform == 'linux2' and QFile.exists(LINUX_CDROM_INFO): + # Read info from /proc/sys/dev/cdrom/info cdinfo = QFile(LINUX_CDROM_INFO) if cdinfo.open(QIODevice.ReadOnly | QIODevice.Text): drive_names = [] @@ -88,19 +96,13 @@ elif sys.platform == 'linux2' and QFile.exists(LINUX_CDROM_INFO): if symlink_target != '': device = symlink_target drives.append(device) - return sorted(uniqify(drives)) -else: - AUTO_DETECT_DRIVES = False - - def get_cdrom_drives(): - from picard import config - - drives = list(DEFAULT_DRIVES) - # Need to filter out empty strings, - # particularly if the device list is empty + else: for device in config.setting["cd_lookup_device"].split(","): + # Need to filter out empty strings, + # particularly if the device list is empty if device.strip() != u'': drives.append(device.strip()) - return sorted(uniqify(drives)) + # make sure no drive is listed twice (given by multiple sources) + return sorted(uniqify(drives)) From e41bf6b054d0fd0dbf0cd6d6155c348c360491be Mon Sep 17 00:00:00 2001 From: Johannes Dewender Date: Sun, 29 Dec 2013 17:14:41 +0100 Subject: [PATCH 3/5] remove generic try-except If we do want to catch generic unexpected tracebacks, this shouldn't be done here. Note that `import discid` might raise OSError when the python module is found, but not libdiscid itself. We might want to catch that explicitely, but that is supposed to be prevented by whoever made `discid` available. --- picard/util/cdrom.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/picard/util/cdrom.py b/picard/util/cdrom.py index 396bf0715..afe56c48b 100644 --- a/picard/util/cdrom.py +++ b/picard/util/cdrom.py @@ -19,7 +19,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. import sys -import traceback if sys.platform == 'win32': from ctypes import windll @@ -28,23 +27,21 @@ from PyQt4.QtCore import QFile, QRegExp, QIODevice, QString from picard import config from picard.util import uniqify -DEFAULT_DRIVES = [] try: + from libdiscid.compat import discid +except ImportError: try: - from libdiscid.compat import discid + import discid except ImportError: - try: - import discid - except ImportError: - discid = None - if discid is not None: - device = discid.get_default_device() - if device: - DEFAULT_DRIVES = [device] -except: - print(traceback.format_exc()) + discid = None +DEFAULT_DRIVES = [] +if discid is not None: + device = discid.get_default_device() + if device: + DEFAULT_DRIVES.append(device) + LINUX_CDROM_INFO = '/proc/sys/dev/cdrom/info' # if get_cdrom_drives() lists all drives available on the machine From d2a3e3ad2df49829b058d94dcbf00c88d06adb1c Mon Sep 17 00:00:00 2001 From: Johannes Dewender Date: Sun, 29 Dec 2013 17:33:11 +0100 Subject: [PATCH 4/5] refactor cdrom code a bit --- picard/util/cdrom.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/picard/util/cdrom.py b/picard/util/cdrom.py index afe56c48b..80651dafe 100644 --- a/picard/util/cdrom.py +++ b/picard/util/cdrom.py @@ -44,7 +44,7 @@ if discid is not None: LINUX_CDROM_INFO = '/proc/sys/dev/cdrom/info' -# if get_cdrom_drives() lists all drives available on the machine +# if get_cdrom_drives() lists ALL drives available on the machine if sys.platform == 'win32': AUTO_DETECT_DRIVES = True elif sys.platform == 'linux2' and QFile.exists(LINUX_CDROM_INFO): @@ -54,7 +54,13 @@ else: # setting uses a text field instead of a drop-down AUTO_DETECT_DRIVES = False +def _split_values(s): + """split a space separated list""" + return QString(s).trimmed().split(QRegExp("\\s+"), QString.SkipEmptyParts) + def get_cdrom_drives(): + """List available disc drives on the machine + """ # add default drive from libdiscid to the list drives = list(DEFAULT_DRIVES) @@ -80,10 +86,10 @@ def get_cdrom_drives(): if line.indexOf(':') != -1: key, values = line.split(':') if key == 'drive name': - drive_names = QString(values).trimmed().split(QRegExp("\\s+"), QString.SkipEmptyParts) + drive_names = _split_values(values) elif key == 'Can play audio': - drive_audio_caps = [v == '1' for v in - QString(values).trimmed().split(QRegExp("\\s+"), QString.SkipEmptyParts)] + drive_audio_caps = [v == '1' + for v in _split_values(values)] line = cdinfo.readLine() # Show only drives that are capable of playing audio for drive in drive_names: From 5d5e8def93a5c50d01e9652eef2243f11e6537b9 Mon Sep 17 00:00:00 2001 From: Johannes Dewender Date: Mon, 30 Dec 2013 01:09:24 +0100 Subject: [PATCH 5/5] update NEWS for 123 --- NEWS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.txt b/NEWS.txt index 7bd9ba336..0946099ac 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -16,6 +16,7 @@ * Main window is now emitting a "selection_updated" signal, plugin api version bumps to 1.3.0 * Append system information to user-agent string * Compilation tag/variable aligned with iTunes, set only for Various Artists type compilations. + * autodetect the CD drive on Mac OS X (PICARD-123) Version 1.2 - 2013-03-30 * Picard now requires at least Python 2.6