Re: Read default link policy from main.conf - draft change for comments

Re: Read default link policy from main.conf - draft change for comments

[Liang Bao]

I've added a configuration item DisableParkState in main.conf and let
the bluetoothd check this. If it's set to true, the
main_opts.link_policy will be HCI_LP_RSWITCH | HCI_LP_SNIFF |
HCI_LP_HOLD, otherwise, it will be original one -
HCI_LP_RSWITCH | HCI_LP_SNIFF | HCI_LP_HOLD | HCI_LP_PARK. Any comment
on the code change or the way to configure? I just want to see if
further improvement is necessary before I send out the patch. Thanks.

diff --git a/data/main.conf b/data/main.conf
index 8153eb2..b9b8fef 100644
--- a/data/main.conf
+++ b/data/main.conf
@@ -57,3 +57,8 @@ ReverseServiceDiscovery = true
 # Enable name resolving after inquiry. Set it to 'false' if you don't need
 # remote devices name and want shorter discovery cycle. Defaults to 'true'.
 NameResolving = true
+
+# Disable park state support helps some bluetooth device stably connected, e.g.
+# Motorola HF850 car-kit will lose connection or fail to setup SCO when it's
+# in park state. Defaults to 'false'.
+DisableParkState = true

diff --git a/src/main.c b/src/main.c
index ff423a4..e0ec7ce 100644
--- a/src/main.c
+++ b/src/main.c
@@ -203,8 +203,15 @@ static void parse_config(GKeyFile *config)

        main_opts.link_mode = HCI_LM_ACCEPT;

        main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
-                                               HCI_LP_HOLD | HCI_LP_PARK;
+                                               HCI_LP_HOLD ;
+        boolean = g_key_file_get_boolean(config, "General",
+                                                "DisableParkState", &err);
+        if (err)
+                g_clear_error(&err);
+        else if (!boolean)
+               main_opts.link_policy |= HCI_LP_PARK;
 }




2010/2/10 Liang Bao <tim.bao@gmail.com>:
> Anyone could advice? Thanks.
>
> 2010/2/8 Liang Bao <tim.bao@gmail.com>:
>> Hi,
>>
>> We noticed that currently BlueZ doesn't read link policy settings from
>> main.conf as it does for other things such as General.Name or
>> General.Class. It just enable everything in parse_config()
>> (src/main.c). Is this just due to lack of use case or BlueZ does this
>> intentionally for some reason? Thanks.
>>
>>
>> 86 static void parse_config(GKeyFile *config)
>> 87 {
>> .....
>> 206         main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
>> 207                                                 HCI_LP_HOLD | HCI_LP_PARK;
>> 208 }
>>
>

Re: Read default link policy from main.conf - draft change for comments

[Liang Bao]

[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]

Here's the patch file. I move the assignment of default value to
init_defaults() and just remove HCI_LP_PARK if the configuration is
set to "true". By default, this configuation in main.conf is written
as "DisableParkState = false"

2010/2/24 Liang Bao <tim.bao@gmail.com>:
> I've added a configuration item DisableParkState in main.conf and let
> the bluetoothd check this. If it's set to true, the
> main_opts.link_policy will be HCI_LP_RSWITCH | HCI_LP_SNIFF |
> HCI_LP_HOLD, otherwise, it will be original one -
> HCI_LP_RSWITCH | HCI_LP_SNIFF | HCI_LP_HOLD | HCI_LP_PARK. Any comment
> on the code change or the way to configure? I just want to see if
> further improvement is necessary before I send out the patch. Thanks.
>
> diff --git a/data/main.conf b/data/main.conf
> index 8153eb2..b9b8fef 100644
> --- a/data/main.conf
> +++ b/data/main.conf
> @@ -57,3 +57,8 @@ ReverseServiceDiscovery = true
>  # Enable name resolving after inquiry. Set it to 'false' if you don't need
>  # remote devices name and want shorter discovery cycle. Defaults to 'true'.
>  NameResolving = true
> +
> +# Disable park state support helps some bluetooth device stably connected, e.g.
> +# Motorola HF850 car-kit will lose connection or fail to setup SCO when it's
> +# in park state. Defaults to 'false'.
> +DisableParkState = true
>
> diff --git a/src/main.c b/src/main.c
> index ff423a4..e0ec7ce 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -203,8 +203,15 @@ static void parse_config(GKeyFile *config)
>
>        main_opts.link_mode = HCI_LM_ACCEPT;
>
>        main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
> -                                               HCI_LP_HOLD | HCI_LP_PARK;
> +                                               HCI_LP_HOLD ;
> +        boolean = g_key_file_get_boolean(config, "General",
> +                                                "DisableParkState", &err);
> +        if (err)
> +                g_clear_error(&err);
> +        else if (!boolean)
> +               main_opts.link_policy |= HCI_LP_PARK;
>  }
>
>
>
>
> 2010/2/10 Liang Bao <tim.bao@gmail.com>:
>> Anyone could advice? Thanks.
>>
>> 2010/2/8 Liang Bao <tim.bao@gmail.com>:
>>> Hi,
>>>
>>> We noticed that currently BlueZ doesn't read link policy settings from
>>> main.conf as it does for other things such as General.Name or
>>> General.Class. It just enable everything in parse_config()
>>> (src/main.c). Is this just due to lack of use case or BlueZ does this
>>> intentionally for some reason? Thanks.
>>>
>>>
>>> 86 static void parse_config(GKeyFile *config)
>>> 87 {
>>> .....
>>> 206         main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
>>> 207                                                 HCI_LP_HOLD | HCI_LP_PARK;
>>> 208 }
>>>
>>
>

[-- Attachment #2: 0001-Allow-disable-park-state-in-main.conf.patch --]
[-- Type: text/x-diff, Size: 2182 bytes --]

From 331b120153c04694c73556c0287456a11874251f Mon Sep 17 00:00:00 2001
From: Tim Bao <tim.bao@gmail.com>
Date: Wed, 24 Feb 2010 20:48:29 +0800
Subject: [PATCH] Allow disable park state in main.conf

Currently BlueZ is hard-coding the default link policy to include
hold mode, role switch, sniff mode and park state. However, some
device will have problem to maintain the connection or setup SCO
if they are in park state. Making the park state configurable in
main.conf improves flexibility so it can be disabled when needed.

Signed-off-by: Tim Bao <tim.bao@gmail.com>
---
 src/main.c    |   13 +++++++++----
 src/main.conf |    5 +++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/main.c b/src/main.c
index 014d8b6..977c87b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -201,10 +201,12 @@ static void parse_config(GKeyFile *config)
 	else
 		main_opts.name_resolv = boolean;
 
-	main_opts.link_mode = HCI_LM_ACCEPT;
-
-	main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
-						HCI_LP_HOLD | HCI_LP_PARK;
+	boolean = g_key_file_get_boolean(config, "General",
+						"DisableParkState", &err);
+	if (err)
+		g_clear_error(&err);
+	else if (boolean)
+		main_opts.link_policy &= (~HCI_LP_PARK);
 }
 
 /*
@@ -276,6 +278,9 @@ static void init_defaults(void)
 	main_opts.remember_powered = TRUE;
 	main_opts.reverse_sdp = TRUE;
 	main_opts.name_resolv = TRUE;
+	main_opts.link_mode = HCI_LM_ACCEPT;
+	main_opts.link_policy = HCI_LP_RSWITCH | HCI_LP_SNIFF |
+				HCI_LP_HOLD | HCI_LP_PARK;
 
 	if (gethostname(main_opts.host_name, sizeof(main_opts.host_name) - 1) < 0)
 		strcpy(main_opts.host_name, "noname");
diff --git a/src/main.conf b/src/main.conf
index b252a82..6f975fc 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -50,3 +50,8 @@ ReverseServiceDiscovery = true
 # Enable name resolving after inquiry. Set it to 'false' if you don't need
 # remote devices name and want shorter discovery cycle. Defaults to 'true'.
 NameResolving = true
+
+# Disable park state in default link policy. Some devices have connection
+# stability issue or fail to setup SCO when the link is in park state. Defaults
+# to 'false'.
+DisableParkState = false
-- 
1.5.6.3