On Mon, Jul 22, 2013 at 11:04 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> 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.
>
Ok
>> +}
>> +
>> +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.
>
I was reviewing the dead logic and I found that it was only useful in
the BlueZ 4 modules, so I'll completely remove it for the next series.
--
Jo?o Paulo Rechi Vita
http://about.me/jprvita
