Hi Tanu,
On Mon, Nov 18, 2013 at 9:36 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
>>
>> This makes module-bluetooth-policy to rank profiles based on their
>> available flag and only consider their priority in case profiles have
>> the same rank.
>
> The old code already ranks profiles based on their available flag and
> only considers their priority in case profiles have the same rank, so
> the commit message doesn't really justify the patch.
Yep, in a very odd fashion that is probably why I thought they didn't, still...
> The only behaviour change (apart from the probably unintentional change
> of preferring "no" over "unknown") that I can see is that the "off"
> profile is now preferred over profiles that have their availability set
> to "unknown", because the "off" profile has availability "yes". If that
> is intended, please explain why you are doing that change.
Profile with higher availability should be considered first in the
following order: yes > unknown > no, or perhaps Im missing something.
This means whenever a Bluetooth profile is disconnected, yes ->
unknown, it should switch to 'off' if there are no profile with
available set to yes.
>> ---
>> src/modules/bluetooth/module-bluetooth-policy.c | 17 +++++++++--------
>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/modules/bluetooth/module-bluetooth-policy.c b/src/modules/bluetooth/module-bluetooth-policy.c
>> index 4a90db7..864d10d 100644
>> --- a/src/modules/bluetooth/module-bluetooth-policy.c
>> +++ b/src/modules/bluetooth/module-bluetooth-policy.c
>> @@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card *card) {
>> void *state;
>> pa_card_profile *profile;
>> pa_card_profile *result = card->active_profile;
>> - pa_card_profile *off;
>> -
>> - pa_assert_se(off = pa_hashmap_get(card->profiles, "off"));
>>
>> PA_HASHMAP_FOREACH(profile, card->profiles, state) {
>> - if (profile->available == PA_AVAILABLE_NO || profile == off)
>> + if (result == profile)
>> + continue;
>> +
>> + if (profile->available > result->available) {
>
> "Unknown" is defined as 0 and "no" is defined as 1, so "no" will be
> preferred over "unknown", which is probably not what you intended.
Now I figure why the checks for available was done in such way, still
there is no point in disregard 'off' profile like the was doing as in
99% of the time it is the best/most available profile in the
circumstances where find_best_profile is called.
--
Luiz Augusto von Dentz
