[pulseaudio-discuss] [PATCH 2/3] bluetooth: Add pa_bluetooth_transport_set_state

[Tanu Kaskinen]

On Mon, 2014-09-08 at 12:14 +0300, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
> 
> ---
>  src/modules/bluetooth/bluez5-util.c | 12 +++++++-----
>  src/modules/bluetooth/bluez5-util.h |  1 +
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index 818790d..3c0d6e1 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -167,7 +167,7 @@ static const char *transport_state_to_string(pa_bluetooth_transport_state_t stat
>      return "invalid";
>  }
>  
> -static void transport_state_changed(pa_bluetooth_transport *t, pa_bluetooth_transport_state_t state) {
> +void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_transport_state_t state) {
>      bool old_any_connected;
>  
>      pa_assert(t);
> @@ -183,6 +183,8 @@ static void transport_state_changed(pa_bluetooth_transport *t, pa_bluetooth_tran
>      t->state = state;
>      if (state == PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
>          t->device->transports[t->profile] = NULL;
> +    else if (!t->device->transports[t->profile])
> +         t->device->transports[t->profile] = t;
>  
>      pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED], t);
>  
> @@ -191,7 +193,7 @@ static void transport_state_changed(pa_bluetooth_transport *t, pa_bluetooth_tran
>  }
>  
>  void pa_bluetooth_transport_put(pa_bluetooth_transport *t) {
> -    transport_state_changed(t, PA_BLUETOOTH_TRANSPORT_STATE_IDLE);
> +    pa_bluetooth_transport_set_state(t, PA_BLUETOOTH_TRANSPORT_STATE_IDLE);
>  }
>  
>  void pa_bluetooth_transport_free(pa_bluetooth_transport *t) {
> @@ -328,7 +330,7 @@ static void parse_transport_property(pa_bluetooth_transport *t, DBusMessageIter
>                      return;
>                  }
>  
> -                transport_state_changed(t, state);
> +                pa_bluetooth_transport_set_state(t, state);
>              }
>  
>              break;
> @@ -412,7 +414,7 @@ static void device_free(pa_bluetooth_device *d) {
>          if (!(t = d->transports[i]))
>              continue;
>  
> -        transport_state_changed(t, PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED);
> +        pa_bluetooth_transport_set_state(t, PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED);
>          pa_bluetooth_transport_free(t);
>      }
>  
> @@ -1436,7 +1438,7 @@ static DBusMessage *endpoint_clear_configuration(DBusConnection *conn, DBusMessa
>  
>      if ((t = pa_hashmap_get(y->transports, path))) {
>          pa_log_debug("Clearing transport %s profile %s", t->path, pa_bluetooth_profile_to_string(t->profile));
> -        transport_state_changed(t, PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED);
> +        pa_bluetooth_transport_set_state(t, PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED);
>          pa_bluetooth_transport_free(t);
>      }
>  
> diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
> index 67377e9..5bcab16 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -112,6 +112,7 @@ void pa_bluetooth_backend_free(pa_bluetooth_backend *b);
>  pa_bluetooth_transport *pa_bluetooth_transport_new(pa_bluetooth_device *d, const char *owner, const char *path,
>                                                     pa_bluetooth_profile_t p, const uint8_t *config, size_t size);
>  
> +void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_transport_state_t state);
>  void pa_bluetooth_transport_put(pa_bluetooth_transport *t);
>  void pa_bluetooth_transport_free(pa_bluetooth_transport *t);
>  

Thanks! I applied this with a couple of minor tweaks, described by this
diff:

@@ -183,8 +183,8 @@ void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_tr
     t->state = state;
     if (state == PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
         t->device->transports[t->profile] = NULL;
-    else if (!t->device->transports[t->profile])
-         t->device->transports[t->profile] = t;
+    else
+        t->device->transports[t->profile] = t;
 
     pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED], t);
 
@@ -1272,7 +1272,7 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
     pa_assert_se(dbus_connection_send(pa_dbus_connection_get(y->connection), r, NULL));
     dbus_message_unref(r);
 
-    d->transports[p] = t = pa_bluetooth_transport_new(d, sender, path, p, config, size);
+    t = pa_bluetooth_transport_new(d, sender, path, p, config, size);
     t->acquire = bluez5_transport_acquire_cb;
     t->release = bluez5_transport_release_cb;
     pa_bluetooth_transport_put(t);

I now regret the first change, but I already pushed the patch... It
doesn't make any difference in behaviour, but having the extra "if" that
was in the original patch made sense, because the assignment needs to be
done only when calling pa_bluetooth_transport_set_state() for the first
time. Removing the extra "if" was a brain fart - I somehow thought that
the function would be called only when creating or destroying the
transport.

-- 
Tanu