conduit r1787 - in trunk: . conduit conduit/dataproviders conduit/modules/iPodModule

[jstowers svn gnome org]

Author: jstowers
Date: Thu Nov  6 14:36:00 2008
New Revision: 1787
URL: http://svn.gnome.org/viewvc/conduit?rev=1787&view=rev

Log:
2008-11-06  John Stowers  <john stowers gmail com>

	* conduit/Conduit.py: Dont hang at exit if we cannot unitialize
    a dataprovider

	* conduit/SyncSet.py:
	* conduit/dataproviders/HalFactory.py:
	* conduit/dataproviders/VolumeFactory.py:
	* conduit/modules/iPodModule/iPodModule.py: Add a gobject timeout
    handler which checks if a volume has been mounted for VolumeFactory. This
    is necessary to work around a race condition where HAL gets some volume.*
    properties before it gets the volume.mound_point property.
    Fixes #557676 and #557669

2008-10-28  John Stowers  <john stowers gmail com>


Modified:
   trunk/ChangeLog
   trunk/conduit/Conduit.py
   trunk/conduit/SyncSet.py
   trunk/conduit/dataproviders/HalFactory.py
   trunk/conduit/dataproviders/VolumeFactory.py
   trunk/conduit/modules/iPodModule/iPodModule.py

Modified: trunk/conduit/Conduit.py
==============================================================================
--- trunk/conduit/Conduit.py	(original)
+++ trunk/conduit/Conduit.py	Thu Nov  6 14:36:00 2008
@@ -214,7 +214,10 @@
         
         #needed to close the db in file dataproviders
         if dataprovider.module != None:
-            dataprovider.module.uninitialize()
+            try:
+                dataprovider.module.uninitialize()
+            except Exception:
+                log.warn("Could not uninitialize %s" % dataprovider, exc_info=True)
 
         #Sources and sinks are stored seperately so must be deleted from different
         #places. Lucky there is only one source or this would be harder....

Modified: trunk/conduit/SyncSet.py
==============================================================================
--- trunk/conduit/SyncSet.py	(original)
+++ trunk/conduit/SyncSet.py	Thu Nov  6 14:36:00 2008
@@ -50,7 +50,10 @@
     def _unitialize_dataproviders(self, cond):
         for dp in cond.get_all_dataproviders():
             if dp.module:
-                dp.module.uninitialize()
+                try:
+                    dp.module.uninitialize()
+                except Exception:
+                    log.warn("Could not uninitialize %s" % dp, exc_info=True)
                 
     def _restore_dataprovider(self, cond, wrapperKey, dpName="", dpxml="", trySourceFirst=True):
         """

Modified: trunk/conduit/dataproviders/HalFactory.py
==============================================================================
--- trunk/conduit/dataproviders/HalFactory.py	(original)
+++ trunk/conduit/dataproviders/HalFactory.py	Thu Nov  6 14:36:00 2008
@@ -19,16 +19,17 @@
         self.hal.connect_to_signal("DeviceRemoved", self._device_removed)
         self.hal.connect_to_signal("NewCapability", self._new_capability)
 
-    def _device_added(self, device_udi, *args):
+    def _maybe_new(self, device_udi):
         props = self._get_properties(device_udi)
         if self.is_interesting(device_udi, props):
             self.item_added(device_udi, **props)
 
+    def _device_added(self, device_udi, *args):
+        self._maybe_new(device_udi)
+
     def _new_capability(self, device_udi, *args):
         if not device_udi in self.items.keys():
-            props = self._get_properties(device_udi)
-            if self.is_interesting(device_udi, props):
-                self.item_added(device_udi, **props)
+            self._maybe_new(device_udi)
 
     def _device_removed(self, device_udi):
         self.item_removed(device_udi)
@@ -40,19 +41,18 @@
             for x, y in device_dbus_obj.GetAllProperties(dbus_interface="org.freedesktop.Hal.Device").items():
                 #DBus *still* does not marshal dbus.String to str correctly,
                 #so we force it to
-                buf[str(x)] = str(y)
+                buf[str(x)] = y
         except:
             log.warn("Could not get HAL properties for %s" % device_udi)
         return buf
 
     def probe(self):
-        """ Enumerate HAL for any entries of interest """
+        """
+        Enumerate HAL for any entries of interest
+        """
         devices = self.hal.GetAllDevices()
         for device in self.hal.GetAllDevices():
-            device = str(device)
-            props = self._get_properties(device)
-            if self.is_interesting(device, props):
-                 self.item_added(device, **props)
+            self._maybe_new(str(device))
 
     def get_args(self, udi, **kwargs):
         return (udi,)

Modified: trunk/conduit/dataproviders/VolumeFactory.py
==============================================================================
--- trunk/conduit/dataproviders/VolumeFactory.py	(original)
+++ trunk/conduit/dataproviders/VolumeFactory.py	Thu Nov  6 14:36:00 2008
@@ -1,3 +1,4 @@
+import gobject
 import logging
 log = logging.getLogger("dataproviders.VolumeFactory")
 
@@ -5,30 +6,53 @@
 import conduit.dataproviders.HalFactory as HalFactory
 
 class VolumeFactory(HalFactory.HalFactory):
-    """ 
-    Generic factory for dataproviders that are removable file system based, or
+    """  file system based, or
     more technically, that have the volume capability defined in HAL. This 
     usually results in them being mounted as removable volumes.
+
+    We jump through some extra hoops here because there is a race condition
+    where we receive notification (the device has capability ("volume"))
     """
 
+    def _wait_for_mount(self, udi, props):
+        props.update(self._get_properties(udi))
+
+        if not props.has_key("volume.is_mounted"):
+            log.info("Still waiting for HAL to notice: %s" % udi)
+            return True
+        else:
+            try:
+                mounted = int(props["volume.is_mounted"])
+                if mounted == 1 and self.is_interesting(udi, props):
+                    self.item_added(udi, **props)
+            except ValueError:
+                log.warn("Could not determine if volume was mounted")
+
+            return False
+
+    def _maybe_new(self, device_udi):
+        props = self._get_properties(device_udi)
+        if "volume" in [str(c) for c in props.get("info.capabilities", [])]:
+            #this basically checks if the volume mounting procedure has finished
+            if str(props.get("volume.mount_point", "")) == "" or props.has_key("volume.is_mounted") == False:
+                log.info("Waiting for HAL to attempt mount")
+                gobject.timeout_add(1000, self._wait_for_mount, device_udi, props)
+            else:
+                if self.is_interesting(device_udi, props):
+                    self.item_added(device_udi, **props)
+
     def probe(self):
         """
         Called after VolumeFactory is initialised to detect already connected volumes
         """
-        for device_udi in self.hal.FindDeviceByCapability("volume"):
-            props = self._get_properties(device_udi)
-            
-            #convert the mountpoint to a string uri because that is what 
-            #all the folder dataproviders work on
-            if props.get("volume.mount_point"):
-                props["mount"] = "file://" + str(props["volume.mount_point"])
-                log.debug("Adjusted mount: %s", props["mount"])
-
-            if self.is_interesting(device_udi, props):
-                self.item_added(device_udi, **kwargs)
+        for device in self.hal.FindDeviceByCapability("volume"):
+            self._maybe_new(str(device))
 
     def get_args(self, udi, **kwargs):
-        """ VolumeFactory passes mount point and udi to dataproviders """
+        """
+        VolumeFactory passes mount point and udi to dataproviders
+        """
+        kwargs["mount"] = "file://" + str(kwargs["volume.mount_point"])
         return (kwargs['mount'], udi,)
 
 

Modified: trunk/conduit/modules/iPodModule/iPodModule.py
==============================================================================
--- trunk/conduit/modules/iPodModule/iPodModule.py	(original)
+++ trunk/conduit/modules/iPodModule/iPodModule.py	Thu Nov  6 14:36:00 2008
@@ -18,6 +18,7 @@
 import locale
 import weakref
 import threading
+import gobject
 log = logging.getLogger("modules.iPod")
 
 import conduit
@@ -62,31 +63,43 @@
     return temp.get_rid()
 
 class iPodFactory(VolumeFactory.VolumeFactory):
+
+    def _get_mount_path(self, props):
+        return str(props["volume.mount_point"])
+
     def is_interesting(self, udi, props):
-        if props.has_key("info.parent") and props.has_key("info.parent")!="":
-            prop2 = self._get_properties(props["info.parent"])
-            if prop2.has_key("storage.model") and prop2["storage.model"]=="iPod":
+        if props.get("info.parent"):
+            parent = self._get_properties(props["info.parent"])
+            if parent.get("storage.model") == "iPod":
+                props.update(parent)
                 return True
         return False
 
     def get_category(self, udi, **kwargs):
         return DataProviderCategory.DataProviderCategory(
-                    kwargs['label'],
+                    kwargs['volume.label'],
                     "multimedia-player-ipod-standard-color",
-                    kwargs['mount'])
+                    self._get_mount_path(kwargs))
 
     def get_dataproviders(self, udi, **kwargs):
         #Read information about the ipod, like if it supports
         #photos or not
         d = gpod.itdb_device_new()
-        gpod.itdb_device_set_mountpoint(d,kwargs['mount'])
+        gpod.itdb_device_set_mountpoint(d,self._get_mount_path(kwargs))
         supportsPhotos = gpod.itdb_device_supports_photo(d)
         gpod.itdb_device_free(d)
         if supportsPhotos:
             return [IPodMusicTwoWay, IPodVideoTwoWay, IPodNoteTwoWay, IPodContactsTwoWay, IPodCalendarTwoWay, IPodPhotoSink]
         else:
+            log.info("iPod does not report photo support")
             return [IPodMusicTwoWay, IPodVideoTwoWay, IPodNoteTwoWay, IPodContactsTwoWay, IPodCalendarTwoWay]
 
+    def get_args(self, udi, **kwargs):
+        """
+        iPod needs a local path to the DB, not a URI
+        """
+        kwargs["mount_path"] = self._get_mount_path(kwargs)
+        return (kwargs['mount_path'], udi)
 
 class IPodBase(DataProvider.TwoWay):
     def __init__(self, *args):
@@ -95,6 +108,8 @@
         self.uid = args[1]
         self.objects = None
 
+        log.debug("Created ipod %s at %s" % (self.__class__.__name__, self.mountPoint))
+
     def refresh(self):
         DataProvider.TwoWay.refresh(self)
         self.objects = []