[Utopia] Fix gnome-volume-manager PTP camera detection

[Martin Pitt]

Hi Jeffrey!

(crossposting to the hal list to get some better coordination).

Jeffrey Stedfast [2005-07-29 15:39 -0400]:
> e.g. is camera.access_method only reported as "ptp" if it's unsupported
> by libgphoto2 (or at least no fdi file for the camera)? or is the access
> method only "libgphoto2" for USB Mass-Storage?

I think it's even more complicated: There are basically three ways to
talk to a camera:

  * usb mass-storage
  * standardized PTP protocol (supported by gphoto)
  * proprietary model specific protocols (most of them supported by
    gphoto)

Some cameras (like my Canon PowerShot A70) speak both PTP and the
Canon-specific protocol, so earlier hal versions (which used gphoto's
UBS hotplug map) actually reported *two* "camera" interfaces which
caused g-v-m to ask for photo import twice. Nowadays (hal 0.5.3) seems
to only check for PTP interfaces, and it almost seems that support for
non-PTP cameras, which gphoto supports with a custom protocol has been
dropped. David, can you confirm this?

So access method "libgphoto2" is not USB mass storage; g-v-m needs to
check for all three access method and then chose a prefered one (I
guess the order I enumerated them would be fine).

> If the latter, I might have to change some logic again... because we get
> the "device added" event from HAL for the physical camera device before
> the volume is added, but we don't actually want to do anything until we
> get the volume added event for USB MAss-Storage camera devices (so that
> we can both mount the camera volume and also pass the mount point to
> f-spot or whatever)

Uh, indeed, that makes it even more complicated. There might actually
be cameras around which support both mass storage and PTP, especially
since the latter is very popular with directly connecting your camera
to a printer.

> >  I noticed that hal does not
> > use gphoto's hotplug map any more, so I guess right now it only
> > supports PTP cameras.
> > 
> > > So... I think the correct solution to this would be to make sure the
> > > property exists before using the value returned by get_property_bool() -
> > > this way, for people without the patch, it continues to work. 
> > 
> > Sounds good, thank you! :-)
> > 
> > > I might split the function in 2 such that the logic is clearer:
> > > 
> > > gvm_udi_is_camera()
> > > 
> > > gvm_udi_is_ptp_camera()
> > 
> > I think the function is simple enough already, but as you wish.
> 
> it turns out the case for USB Mass-Storage Cameras was wrong and
> required a bit more work to detect properly. I needed to check the
> physical device for the "camera" property, rather than the volume udi
> (duh, should have been obvious).
> 
> I didn't catch this originally because the post-mount code was detecting
> a DCIM directory and so things magically worked that way...
> 
> > 
> > > > --- gnome-volume-manager-1.3.2-old/src/manager.c        2005-07-29 17:02:13.000000000 +0200
> > > > +++ gnome-volume-manager-1.3.2/src/manager.c    2005-07-29 17:02:13.000000000 +0200
> > > > @@ -614,15 +614,13 @@
> > > >  static gboolean
> > > >  gvm_udi_is_camera (const char *udi, gboolean check_libgphoto2)
> > > >  {
> > > > -       if (!libhal_device_query_capability (hal_ctx, udi, "camera", NULL))
> > > > -               return FALSE;
> > > > -
> > > > -       if (check_libgphoto2 && !libhal_device_get_property_bool (hal_ctx, udi, "camera.libgphoto2_support", NULL))
> > > > -               return FALSE;
> > > > -
> > > > -       dbg ("Camera detected: %s\n", udi);
> > > > +       if (libhal_device_query_capability (hal_ctx, udi, "camera", NULL) ||
> > > > +            (check_libgphoto2 && libhal_device_get_property_bool (hal_ctx, udi, "camera.libgphoto2_support", NULL))) {
> > > > +                dbg ("Camera detected: %s\n", udi);
> > > > +                return TRUE;
> > > > +        }
> > > 
> > > the new logic is such that it will still return TRUE even if
> > > check_libgphoto2 is TRUE and libgphoto2_support is FALSE (assuming the
> > > camera property is on the device). This should not happen.
> > > 
> > > Might as well not even bother checking libgphoto2 :)
> > 
> > I don't understand---with my patch, hal will return FALSE if
> > camera.libgphoto2_support exists and is false. So what do you mean?
> 
> if (TRUE || (TRUE && FALSE))
> 
> shortcut that logic :)

Ah, now I know what you mean. I originally thought that there is
_either_ the "camera" capability (since it is only added by hal if the
camera supports PTP) or the camera.libgphoto2_support property (if
vendor and product ID are in the gphoto hotplug map. And of course
both if the camera supports both. So if a camera would only support
gphoto, but not PTP (like older Canon PowerShot models), then my "or"
logic would indeed be correct.

Thanks,

Martin

-- 
Martin Pitt              http://www.piware.de
Ubuntu Developer   http://www.ubuntulinux.org
Debian Developer        http://www.debian.org
_______________________________________________
hal mailing list
hal at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/hal