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
