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
