Hi Jeffrey!
Jeffrey Stedfast [2005-07-29 11:21 -0400]:
> On Fri, 2005-07-29 at 17:08 +0200, Martin Pitt wrote:
> > Hi!
> >
> > gnome-volume-manager 1.3.2 broke PTP camera detection due to a logic
> > flaw in gvm_udi_is_camera(). Even if a device had the "camera"
> > capability, the subsequent check for libgphoto2 support made the
> > function return FALSE.
>
> I believe the logic is correct, but it sounds like it's failing for you
> because your HAL isn't patched to ad the libgphoto2_support property.
Maybe, but it should also work with the upstream version. Will the
libgphoto2_support go upstream as well? 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.
> > --- 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?
Thanks,
Martin
--
Martin Pitt http://www.piware.de
Ubuntu Developer http://www.ubuntulinux.org
Debian Developer http://www.debian.org
Attachment:
signature.asc
Description: Digital signature