[pulseaudio-discuss] [PATCH 3/5] bluetooth: Fix notifying about new devices

[Tanu Kaskinen]

On Tue, 2013-10-01 at 10:34 -0300, João Paulo Rechi Vita wrote:
> On Sun, Sep 29, 2013 at 12:49 PM, Tanu Kaskinen
> <tanu.kaskinen at linux.intel.com> wrote:
> > Normally devices are created and their properties are parsed before
> > creating any transports for the device, and the
> > DEVICE_CONNECTION_CHANGED hook is fired when the first transport is
> > created. It's possible, however, that a transport is created before
> > the device that it belongs to, and in this case
> > DEVICE_CONNECTION_CHANGED should be fired when the properties have
> > been successfully parsed for the device. The old code didn't fire the
> > hook at this situation, and this patch fixes that.
> > ---
> >  src/modules/bluetooth/bluez5-util.c | 43 +++++++++++++++++++++++++++++--------
> >  1 file changed, 34 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> > index eaff4b1..c7e934e 100644
> > --- a/src/modules/bluetooth/bluez5-util.c
> > +++ b/src/modules/bluetooth/bluez5-util.c
> > @@ -438,14 +438,29 @@ static void device_remove(pa_bluetooth_discovery *y, const char *path) {
> >      }
> >  }
> >
> > +static void set_device_info_valid(pa_bluetooth_device *device, int valid) {
> > +    bool old_any_connected;
> > +
> > +    pa_assert(device);
> > +    pa_assert(valid == -1 || valid == 0 || valid == 1);
> > +
> > +    if (valid == device->device_info_valid)
> > +        return;
> > +
> > +    old_any_connected = pa_bluetooth_device_any_transport_connected(device);
> > +    device->device_info_valid = valid;
> > +
> > +    if (pa_bluetooth_device_any_transport_connected(device) != old_any_connected)
> > +        pa_hook_fire(&device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED], device);
> > +}
> > +
> 
> I like the idea of having a helper function here. But wouldn't be
> better to factor the occurences of "device->device_info_valid = X"
> into set_device_info_valid() in one commit and the
> DEVICE_CONNECTION_CHANGED hook fire, together with the respective
> checks, in a separate commit? I think they are two different
> modifications, so the would belong to separate commits, making the
> history cleaner and easier to read. If for some reason you don't want
> to separate them it's ok for me (just please explain me why you think
> so).

Good point, I'll update the patches.

-- 
Tanu