[pulseaudio-discuss] [PATCH 31/56] bluetooth: Implement org.bluez.MediaEndpoint1.ClearConfiguration()

[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 | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index d8cf1df..762d1e4 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -631,11 +631,41 @@ fail:
>  }
>  
>  static DBusMessage *endpoint_clear_configuration(DBusConnection *conn, DBusMessage *m, void *userdata) {
> +    pa_bluetooth_discovery *y = userdata;
> +    pa_bluetooth_transport *t;
>      DBusMessage *r;
> +    DBusError err;
> +    const char *path;
>  
> -    pa_assert_se(r = dbus_message_new_error(m, BLUEZ_MEDIA_ENDPOINT_INTERFACE ".Error.NotImplemented",
> -                                            "Method not implemented"));
> +    dbus_error_init(&err);
>  
> +    if (!dbus_message_get_args(m, &err, DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_INVALID)) {
> +        pa_log_error("Endpoint ClearConfiguration(): %s", err.message);
> +        dbus_error_free(&err);
> +        goto fail;
> +    }
> +
> +    if ((t = pa_hashmap_get(y->transports, path))) {
> +        bool old_any_connected = pa_bluetooth_device_any_transport_connected(t->device);
> +
> +        pa_log_debug("Clearing transport %s profile %d", t->path, t->profile);

Human-readable profile, please.

> +        t->device->transports[t->profile] = NULL;
> +        t->state = PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED;
> +        pa_hook_fire(&y->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED], t);

Copied from earlier mails:

"Can the transport state be DISCONNECTED already before we set it here?
If it can, please fire the hook only if the state actually changes."

"Also, I'd like a log message (debug level) whenever t->state changes.
There could be set_state() helper function that checks whether the new
state is different than the old state, and if it is, then sets t->state,
logs a message (including the old state, "state changed from <old> to
<new>") and fires the STATE_CHANGED hook.

This is just a wish, so I won't continue arguing if you for some reason
don't think this is a good idea."

> +
> +        if (old_any_connected != pa_bluetooth_device_any_transport_connected(t->device)) {
> +            pa_hook_fire(&y->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED], t->device);
> +        }

Redundant braces.

-- 
Tanu