[pulseaudio-discuss] [PATCH 39/56] bluetooth: Handle PropertiesChanged for org.bluez.MediaTransport1

[Tanu Kaskinen]

On Fri, 2013-07-12 at 15:06 -0300, jprvita at gmail.com wrote:
> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> 
> ---
>  src/modules/bluetooth/bluez5-util.c | 84 +++++++++++++++++++++++++++++++++++++
>  src/modules/bluetooth/bluez5-util.h |  1 +
>  2 files changed, 85 insertions(+)
> 
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index e29a19a..45052a2 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -253,6 +253,79 @@ bool pa_bluetooth_device_any_transport_connected(const pa_bluetooth_device *d) {
>      return false;
>  }
>  
> +static pa_bluetooth_transport_state_t transport_state_from_string(const char* value, pa_bluetooth_transport_state_t *state) {
> +    pa_assert(value);
> +    pa_assert(state);
> +
> +    if (pa_streq(value, "idle"))
> +        *state = PA_BLUETOOTH_TRANSPORT_STATE_IDLE;
> +    else if (pa_streq(value, "pending") || pa_streq(value, "active"))
> +        *state = PA_BLUETOOTH_TRANSPORT_STATE_PLAYING;
> +    else
> +        return PA_BLUETOOTH_TRANSPORT_STATE_INVALID;

Here you return an enum value...

> +
> +    return 0;

...and here you return an int.

It seems that the only purpose of PA_BLUETOOTH_TRANSPORT_STATE_INVALID
is to signal error from transport_state_from_string(). Since returning
-1 is the normal way to signal error, I think it's best to not have the
INVALID state at all, and just change the return value of
transport_state_from_string() to int.

> +}
> +
> +static void parse_transport_property(pa_bluetooth_transport *t, DBusMessageIter *i) {
> +    const char *key;
> +    DBusMessageIter variant_i;
> +
> +    key = pa_bluetooth_dbus_check_variant_property(i);
> +    if (key == NULL)
> +        return;
> +
> +    dbus_message_iter_recurse(i, &variant_i);
> +
> +    switch (dbus_message_iter_get_arg_type(&variant_i)) {
> +
> +        case DBUS_TYPE_STRING: {
> +
> +            const char *value;
> +            dbus_message_iter_get_basic(&variant_i, &value);
> +
> +            if (pa_streq(key, "State")) {
> +                bool old_any_connected = pa_bluetooth_device_any_transport_connected(t->device);
> +
> +                if (transport_state_from_string(value, &t->state) == PA_BLUETOOTH_TRANSPORT_STATE_INVALID) {
> +                    pa_log_error("Transport %s has an invalid state: '%s'", t->path, value);
> +                    return;
> +                }
> +
> +                pa_log_debug("Transport %s state changed to '%s'", t->path, value);
> +                pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED], t);
> +
> +                if (old_any_connected != pa_bluetooth_device_any_transport_connected(t->device)) {
> +                    t->device->dead = old_any_connected;
> +                    pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED], t->device);

If the device is dead, I suppose it should be freed after firing the
hook.

-- 
Tanu