Merge lp:~bladernr/checkbox/953160-mmc-not-sdio into lp:checkbox

[Jeff Lane ]

Changes to the scripts to allow us to specify a list of busses to watch/test. This handles those pesky MMC readers that appear as SDIO, USB or SCSI devices :/

[Brendan Donegan (brendan-donegan)]

Not quite sure why it was necessary to mess around with the way imports were done but otherwise it looks fine. I'm going to give it a test now and see how it works.

[Jeff Lane  (bladernr)]

Ummm... I'll just say it seemed like a good idea at the time... I think at one point I changed that because I was using more than just ArgumentParser, but then removed those changes and forgot to reset the import.

[TienFu Chen (ctf)]

Looks good,
I ran the command as below, but got the message "Expected device usb inserted" for twice.
-------------------------------------
sudo ./removable_storage_watcher insert usb sdio scsi
Expected device usb inserted
Expected device usb inserted
-------------------------------------
I examined the parameters of function job_change_detected: devices, job_in_progress, job_id, job_num_tasks, job_cur_task_id, job_cur_task_percentage, from the first and second run of job_change_detected. Those parameters are the same. So it implies when the usb is inserted, the usb is being mounted for twice?

---------------------
/org/freedesktop/UDisks/devices/sdb1
1
FilesystemMount
1000
0
-1.0
Expected device usb inserted
/org/freedesktop/UDisks/devices/sdb1
1
FilesystemMount
1000
0
-1.0
---------------------

review: Approve

[Brendan Donegan (brendan-donegan)]

This is fine. There's no point adding code to get around this - as long as the script detects one mount at least then it will pass.

[Daniel Manrique (roadmr)]

Merging this based on Tim's approval. Fixed changelog while I was at it. Thanks!

=== modified file 'debian/changelog'
--- debian/changelog	2012-03-15 14:05:42 +0000
+++ debian/changelog	2012-03-15 15:18:21 +0000
@@ -11,12 +11,19 @@
   [Jeff Lane]
   * Reset default checkbox log level to INFO from DEBUG to make logs less
     confusing and verbose. (LP: #949745) 
+<<<<<<< TREE
   * Removed dependency on bluetooth/detect-output on the 
     suspend/suspend_advanced job. (LP: #955375)
+=======
+  * Modified removable_storage_watcher and removable_storage_test to accept
+    list of busses to watch to resolve problems on systems with MMC readers
+    that present themselves as USB devices rather than SDIO (LP: #953160)
+>>>>>>> MERGE-SOURCE
 
   [Marc Tardif]
   * Linted qt_interface which had a few syntax errors (LP: #949957)
 
+<<<<<<< TREE
   [Daniel Manrique]
   * Bumpted revision number to 0.13.5 in trunk
   * jobs/keys.txt.in: Fix definition for keys/media-keys test which failed to
@@ -25,6 +32,9 @@
     undesirable secondary effects.
 
  -- Jeff Lane <[e-mail]>  Wed, 14 Mar 2012 18:10:57 -0400
+=======
+ -- Jeff Lane <[e-mail]>  Tue, 13 Mar 2012 18:18:04 -0400
+>>>>>>> MERGE-SOURCE
 
 checkbox (0.13.4) precise; urgency=low
 

=== modified file 'jobs/mediacard.txt.in'
--- jobs/mediacard.txt.in	2012-02-05 13:07:59 +0000
+++ jobs/mediacard.txt.in	2012-03-15 15:18:21 +0000
@@ -1,6 +1,6 @@
 plugin: manual
 name: mediacard/mmc-insert
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -24,7 +24,7 @@
 plugin: manual
 name: mediacard/mmc-remove
 depends: mediacard/mmc-storage
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects 
@@ -39,7 +39,7 @@
 plugin: manual
 name: mediacard/mmc-insert-after-suspend
 depends: suspend/suspend_advanced
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -63,7 +63,7 @@
 plugin: manual
 name: mediacard/mmc-remove-after-suspend
 depends: mediacard/mmc-storage-after-suspend
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects the removal
@@ -77,7 +77,7 @@
 
 plugin: manual
 name: mediacard/sd-insert
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -101,7 +101,7 @@
 plugin: manual
 name: mediacard/sd-remove
 depends: mediacard/sd-storage
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects 
@@ -116,7 +116,7 @@
 plugin: manual
 name: mediacard/sd-insert-after-suspend
 depends: suspend/suspend_advanced
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -140,7 +140,7 @@
 plugin: manual
 name: mediacard/sd-remove-after-suspend
 depends: mediacard/sd-storage-after-suspend
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects 
@@ -154,7 +154,7 @@
 
 plugin: manual
 name: mediacard/sdhc-insert
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -178,7 +178,7 @@
 plugin: manual
 name: mediacard/sdhc-remove
 depends: mediacard/sdhc-storage
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects 
@@ -193,7 +193,7 @@
 plugin: manual
 name: mediacard/sdhc-insert-after-suspend
 depends: suspend/suspend_advanced
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -217,7 +217,7 @@
 plugin: manual
 name: mediacard/sdhc-remove-after-suspend
 depends: mediacard/sdhc-storage-after-suspend
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects the removal
@@ -231,7 +231,7 @@
 
 plugin: manual
 name: mediacard/cf-insert
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -255,7 +255,7 @@
 plugin: manual
 name: mediacard/cf-remove
 depends: mediacard/cf-storage
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects 
@@ -270,7 +270,7 @@
 plugin: manual
 name: mediacard/cf-insert-after-suspend
 depends: suspend/suspend_advanced
-command: removable_storage_watcher insert sdio
+command: removable_storage_watcher insert sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the systems media card reader can
@@ -294,7 +294,7 @@
 plugin: manual
 name: mediacard/cf-remove-after-suspend
 depends: mediacard/cf-storage-after-suspend
-command: removable_storage_watcher remove sdio
+command: removable_storage_watcher remove sdio usb scsi
 _description:
  PURPOSE:
      This test will check that the system correctly detects the removal

=== modified file 'scripts/removable_storage_test'
--- scripts/removable_storage_test	2012-03-05 08:23:50 +0000
+++ scripts/removable_storage_test	2012-03-15 15:18:21 +0000
@@ -79,7 +79,7 @@
             device_obj = bus.get_object("org.freedesktop.UDisks", dev)
             device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
             if not device_props.Get('org.freedesktop.UDisks.Device',"DeviceIsDrive"):
-                if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") == device:
+                if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") in device:
                     dev_file = str(device_props.Get('org.freedesktop.UDisks.Device',"DeviceFile"))
 
                     if len(device_props.Get('org.freedesktop.UDisks.Device',"DeviceMountPaths")) > 0:
@@ -168,7 +168,8 @@
 def main():
     parser = argparse.ArgumentParser()
     parser.add_argument('device',
-                        choices=['usb','firewire','sdio'],
+                        choices=['usb','firewire','sdio', 'scsi'],
+                        nargs = '+',
                         help="The type of removable media (usb, firewire, sdio) to test.")
     parser.add_argument('-l','--list',
                         action='store_true',

=== modified file 'scripts/removable_storage_watcher'
--- scripts/removable_storage_watcher	2012-01-23 15:04:52 +0000
+++ scripts/removable_storage_watcher	2012-03-15 15:18:21 +0000
@@ -3,16 +3,15 @@
 import sys
 import gobject
 import dbus
+import argparse
 
-from argparse import ArgumentParser
 from dbus.mainloop.glib import DBusGMainLoop
 
-
 class StorageDeviceListener(object):
 
-    def __init__(self, action, device):
+    def __init__(self, action, devices):
         self._action = action
-        self._device = device
+        self._devices = devices
         self._bus = dbus.SystemBus(mainloop=DBusGMainLoop())
         self._loop = gobject.MainLoop()
         self._error = False
@@ -33,7 +32,7 @@
 
         return self._error
 
-    def job_change_detected(self, device, job_in_progress, job_id, job_num_tasks, job_cur_task_id, job_cur_task_percentage):
+    def job_change_detected(self, devices, job_in_progress, job_id, job_num_tasks, job_cur_task_id, job_cur_task_percentage):
         if job_id == "FilesystemMount" and self.is_device_inserted():
             print "Expected device %s inserted" % self._device
             self._loop.quit()
@@ -43,20 +42,21 @@
 
     def remove_detected(self, removed_path):
         if not self.is_device_inserted():
-            print "Expected device %s has been removed" % self._device
-
+            print "Removable storage device has been removed"
+        #TODO: figure out a way to get the DriveConnectionInterface of the
+        #device that was just removed.
         self._loop.quit()
 
     def is_device_inserted(self):
         ud_manager_obj = self._bus.get_object("org.freedesktop.UDisks", "/org/freedesktop/UDisks")
         ud_manager = dbus.Interface(ud_manager_obj, 'org.freedesktop.UDisks')
-
         for dev in ud_manager.EnumerateDevices():
             try:
                 device_obj = self._bus.get_object("org.freedesktop.UDisks", dev)
                 device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
+                self._device = device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface")
                 if not device_props.Get('org.freedesktop.UDisks.Device',"DeviceIsDrive"):
-                    if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") == self._device:
+                    if self._device in self._devices:
                         return True
             except dbus.DBusException:
                 pass
@@ -64,9 +64,9 @@
         return False
 
 def main():
-    parser = ArgumentParser(description="Wait for the specified device to be inserted or removed.")
+    parser = argparse.ArgumentParser(description="Wait for the specified device to be inserted or removed.")
     parser.add_argument('action', choices=['insert','remove'])
-    parser.add_argument('device', choices=['usb','firewire','sdio'])
+    parser.add_argument('device', choices=['usb','sdio','firewire','scsi'],nargs=argparse.REMAINDER)
     parser.add_argument('--timeout', type=int, default=10)
     args = parser.parse_args()