Re: [Tracker] [patch] split TrackerHal into TrackerStorage and TrackerPower and re-organise into backend

[Jürg Billeter <j bitron ch>]

On Fri, 2009-05-15 at 12:52 +0100, Richard Hughes wrote:
> On Wed, May 13, 2009 at 1:33 PM, JÃrg Billeter <j bitron ch> wrote:
> > On Thu, 2009-04-02 at 10:26 +0100, Richard Hughes wrote:
> > > The attached patch splits TrackerHal into TrackerStorage and
> > > TrackerPower to better split the functionalities for future work. This
> > > is needed for future patches to add DeviceKit support. I've also created
> > > the files tracker-storage-hal and tracker-power-hal so that the power
> > > and storage code can be compiled into a HAL or DeviceKit backends in the
> > > future.
> > >
> > > The diffstat looks huge, but it's really just splitting the GObject and
> > > reorganising the filenames, there are no real logic changes. I've done a
> > > quick test of trackerd, but it probably needs someone familiar with the
> > > self tests to check this before it's committed.
> > >
> > > [...]
> > >
> > > Feedback please. I'll submit further patches when this patch is
> > > accepted. I have commit on gnome svn if this helps. Thanks.
> >
> > Thanks for the patch. I've updated/ported it for use with master (0.7)
> > and applied it.
>
> Excellent, thanks. I've now attached another patch against master
> which fixes the overlapping policy and (and suspicious logic) to
> create two properties, on-battery and on-low-battery. This simplifies
> the logic, and now the code can do either policy decision without
> reimplement the logic and previous bugs.

Thanks, the changes make sense to me. A few comments:

On line 488 in tracker-indexer.c there is one occurrence of
tracker_power_get_battery_in_use left, needs to be fixed to not break
the build.

+       return (priv->battery_percentage > BATTERY_LOW_THRESHOLD);

This should be less than, not greater than, I assume.

The hal_battery_notify function needs to check whether the percentage
change crosses the BATTERY_LOW_THRESHOLD and call g_object_notify if
appropriate.

> Helpfully, the DeviceKit-power backend also exposes on-battery and
> on-low-battery on the DBus interface. This makes it trivial to add a
> DeviceKit-power backend, which you can expect when this patch is
> approved, and when I get back from my honeymoon :-)

Enjoy your honeymoon!

Thanks,
JÃrg