[pulseaudio-discuss] [RFC next v0 02/11] bluetooth: Add basic backend->core notifications

[Tanu Kaskinen]

On Wed, 2013-05-15 at 10:46 +0200, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
> 
> Make a clear split to define an API implementing transport operations,
> and how these backends should report the events to the Bluetooth core.
> ---
>  src/modules/bluetooth/bluetooth-util.c | 143 +++++++++++++++++++++++----------
>  src/modules/bluetooth/bluetooth-util.h |   7 ++
>  2 files changed, 106 insertions(+), 44 deletions(-)
> 
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index b8fe450..2e3203e 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -588,7 +588,6 @@ static int parse_audio_property(pa_bluetooth_device *d, const char *interface, D
>  
>              if (pa_streq(key, "State")) {
>                  pa_bt_audio_state_t state = audio_state_from_string(value);
> -                pa_bluetooth_transport_state_t old_state;
>  
>                  pa_log_debug("Device %s interface %s property 'State' changed to value '%s'", d->path, interface, value);
>  
> @@ -607,16 +606,7 @@ static int parse_audio_property(pa_bluetooth_device *d, const char *interface, D
>                  if (!transport)
>                      break;
>  
> -                old_state = transport->state;
> -                transport->state = audio_state_to_transport_state(state);
> -
> -                if (transport->state != old_state) {
> -                    pa_log_debug("Transport %s (profile %s) changed state from %s to %s.", transport->path,
> -                                 pa_bt_profile_to_string(transport->profile), transport_state_to_string(old_state),
> -                                 transport_state_to_string(transport->state));
> -
> -                    pa_hook_fire(&d->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED], transport);
> -                }
> +                pa_bt_backend_notify_state(transport, audio_state_to_transport_state(state));

I would prefer moving all the D-Bus code to a separate bluez backend
file. So far it looks like you're putting both the bluetooth core and
the bluez backend to bluetooth-util.c - do you plan to keep things that
way?

> @@ -2313,3 +2276,95 @@ void pa_bt_backend_unregister(pa_bluetooth_discovery *y, pa_bluetooth_backend *b
>      y->profiles[p].backend = NULL;
>      y->profiles[p].backend_private = NULL;
>  }
> +
> +void pa_bt_backend_notify_transport_removed(pa_bluetooth_transport *t) {
> +    pa_bluetooth_discovery *y;
> +    pa_bluetooth_device *d;
> +    bool old_any_connected;
> +
> +    pa_assert(t);
> +    pa_assert_se(d = t->device);
> +    pa_assert_se(y = d->discovery);
> +
> +    old_any_connected = pa_bluetooth_device_any_audio_connected(d);
> +
> +    pa_log_debug("Removing transport %s profile %d", t->path, t->profile);
> +
> +    d->transports[t->profile] = NULL;
> +    pa_hashmap_remove(y->transports, t->path);

Perhaps pa_assert_se() would be appropriate here?

> diff --git a/src/modules/bluetooth/bluetooth-util.h b/src/modules/bluetooth/bluetooth-util.h
> index 969f489..3f54802 100644
> --- a/src/modules/bluetooth/bluetooth-util.h
> +++ b/src/modules/bluetooth/bluetooth-util.h
> @@ -187,4 +187,11 @@ struct pa_bluetooth_backend {
>  int pa_bt_backend_register(pa_bluetooth_discovery *y, pa_bluetooth_backend *b, enum profile p, void *bp);
>  void pa_bt_backend_unregister(pa_bluetooth_discovery *y, pa_bluetooth_backend *b, enum profile p);
>  
> +/* Reporting of events from backend to Bluetooth core */
> +void pa_bt_backend_notify_transport_removed(pa_bluetooth_transport *t);
> +void pa_bt_backend_notify_state(pa_bluetooth_transport *t, pa_bluetooth_transport_state_t state);
> +void pa_bt_backend_notify_nrec(pa_bluetooth_transport *t, bool nrec);
> +void pa_bt_backend_notify_microphone_gain(pa_bluetooth_transport *t, uint16_t value);
> +void pa_bt_backend_notify_speaker_gain(pa_bluetooth_transport *t, uint16_t value);

To me these seem transport methods, so I don't like the naming scheme. I
propose "pa_bluetooth_transport_transport_removed",
"pa_bluetooth_transport_state_changed" etc. One example of prior art
would be the pa_sink_volume_changed() function, which is called by the
sink implementation to notify when a volume change happens that is
initiated by the backend, not by PulseAudio.

-- 
Tanu