[PATCH BlueZ 1/3] audio: Remove profile enabling/disabling logic

[PATCH BlueZ 1/3] audio: Remove profile enabling/disabling logic

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This should be handle by the core for all profiles
---
 profiles/audio/audio.conf |  4 ---
 profiles/audio/manager.c  | 62 ++++++-----------------------------------------
 2 files changed, 8 insertions(+), 58 deletions(-)

diff --git a/profiles/audio/audio.conf b/profiles/audio/audio.conf
index f556610..067b3fc 100644
--- a/profiles/audio/audio.conf
+++ b/profiles/audio/audio.conf
@@ -6,7 +6,3 @@
 
 # Switch to master role for incoming connections (defaults to true)
 #Master=true
-
-# If we want to disable support for specific services
-# Defaults to supporting the services: Sink, Control
-#Disable=Source
diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
index 934227e..42a2b58 100644
--- a/profiles/audio/manager.c
+++ b/profiles/audio/manager.c
@@ -70,12 +70,6 @@
 static GKeyFile *config = NULL;
 static GSList *devices = NULL;
 
-static struct enabled_interfaces enabled = {
-	.sink		= TRUE,
-	.source		= FALSE,
-	.control	= TRUE,
-};
-
 static struct audio_device *get_audio_dev(struct btd_device *device)
 {
 	return manager_get_audio_device(device, TRUE);
@@ -410,47 +404,12 @@ void audio_control_disconnected(struct btd_device *dev, int err)
 
 int audio_manager_init(GKeyFile *conf)
 {
-	char **list;
-	int i;
-
-	if (!conf)
-		goto proceed;
-
-	config = conf;
-
-	list = g_key_file_get_string_list(config, "General", "Enable",
-						NULL, NULL);
-	for (i = 0; list && list[i] != NULL; i++) {
-		if (g_str_equal(list[i], "Sink"))
-			enabled.sink = TRUE;
-		else if (g_str_equal(list[i], "Source"))
-			enabled.source = TRUE;
-		else if (g_str_equal(list[i], "Control"))
-			enabled.control = TRUE;
-	}
-	g_strfreev(list);
-
-	list = g_key_file_get_string_list(config, "General", "Disable",
-						NULL, NULL);
-	for (i = 0; list && list[i] != NULL; i++) {
-		if (g_str_equal(list[i], "Sink"))
-			enabled.sink = FALSE;
-		else if (g_str_equal(list[i], "Source"))
-			enabled.source = FALSE;
-		else if (g_str_equal(list[i], "Control"))
-			enabled.control = FALSE;
-	}
-	g_strfreev(list);
+	if (conf)
+		config = conf;
 
-proceed:
-	if (enabled.source)
-		btd_profile_register(&a2dp_source_profile);
-
-	if (enabled.sink)
-		btd_profile_register(&a2dp_sink_profile);
-
-	if (enabled.control)
-		btd_profile_register(&avrcp_profile);
+	btd_profile_register(&a2dp_source_profile);
+	btd_profile_register(&a2dp_sink_profile);
+	btd_profile_register(&avrcp_profile);
 
 	btd_register_adapter_driver(&media_driver);
 
@@ -464,14 +423,9 @@ void audio_manager_exit(void)
 		config = NULL;
 	}
 
-	if (enabled.source)
-		btd_profile_unregister(&a2dp_source_profile);
-
-	if (enabled.sink)
-		btd_profile_unregister(&a2dp_sink_profile);
-
-	if (enabled.control)
-		btd_profile_unregister(&avrcp_profile);
+	btd_profile_unregister(&a2dp_source_profile);
+	btd_profile_unregister(&a2dp_sink_profile);
+	btd_profile_unregister(&avrcp_profile);
 
 	btd_unregister_adapter_driver(&media_driver);
 }
-- 
1.8.1.2

[PATCH BlueZ 2/3] core: Add DisableProfiles entry to main.conf

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This entry can be used to globally disable profiles, this is specially
useful for qualification purposes where some platforms may decide to
only qualify a subset of the supported profiles.
---
 src/hcid.h    |  1 +
 src/main.c    | 10 ++++++++++
 src/main.conf |  4 ++++
 src/profile.c | 24 ++++++++++++++++++++++++
 4 files changed, 39 insertions(+)

diff --git a/src/hcid.h b/src/hcid.h
index ea67cc2..f465a2b 100644
--- a/src/hcid.h
+++ b/src/hcid.h
@@ -32,6 +32,7 @@ struct main_opts {
 	gboolean	reverse_sdp;
 	gboolean	name_resolv;
 	gboolean	debug_keys;
+	char		**disabled_profiles;
 
 	uint16_t	did_source;
 	uint16_t	did_vendor;
diff --git a/src/main.c b/src/main.c
index 1e40ebc..933c20f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -76,6 +76,7 @@ static const char * const supported_options[] = {
 	"ReverseServiceDiscovery",
 	"NameResolving",
 	"DebugKeys",
+	"DisableProfiles"
 };
 
 static GKeyFile *load_config(const char *file)
@@ -263,6 +264,15 @@ static void parse_config(GKeyFile *config)
 		g_clear_error(&err);
 	else
 		main_opts.debug_keys = boolean;
+
+	str = g_key_file_get_string(config, "General", "DisableProfiles", &err);
+	if (err) {
+		DBG("%s", err->message);
+		g_clear_error(&err);
+	} else {
+		main_opts.disabled_profiles = g_strsplit(str, " ", -1);
+		g_free(str);
+	}
 }
 
 static void init_defaults(void)
diff --git a/src/main.conf b/src/main.conf
index a94274a..5c648bf 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -46,3 +46,7 @@
 # makes debug link keys valid only for the duration of the connection
 # that they were created for.
 #DebugKeys = false
+
+# Disable Profile, both driver name and 128 bits UUIDs can be used.
+# By default all profiles are enabled.
+#DisableProfiles = <profile1> <profile2> ...
diff --git a/src/profile.c b/src/profile.c
index 656506a..fc62f02 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -50,6 +50,7 @@
 #include "adapter.h"
 #include "device.h"
 #include "profile.h"
+#include "hcid.h"
 
 #define DUN_DEFAULT_CHANNEL	1
 #define SPP_DEFAULT_CHANNEL	3
@@ -610,6 +611,29 @@ void btd_profile_foreach(void (*func)(struct btd_profile *p, void *data),
 
 int btd_profile_register(struct btd_profile *profile)
 {
+	int i;
+
+	if (main_opts.disabled_profiles == NULL)
+		goto done;
+
+	for (i = 0; main_opts.disabled_profiles[i]; i++) {
+		if (g_strcmp0(main_opts.disabled_profiles[i],
+						profile->name) == 0)
+			return -EPROTONOSUPPORT;
+
+		if (g_strcmp0(main_opts.disabled_profiles[i],
+						profile->local_uuid) == 0)
+			return -EPROTONOSUPPORT;
+
+		if (profile->remote_uuids == NULL)
+			continue;
+
+		if (g_strcmp0(main_opts.disabled_profiles[i],
+						profile->remote_uuids[0]) == 0)
+			return -EPROTONOSUPPORT;
+	}
+
+done:
 	profiles = g_slist_append(profiles, profile);
 	return 0;
 }
-- 
1.8.1.2

[PATCH BlueZ 3/3] core: Rename hcid.h to main.h

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

main seems more logical than hcid which use to be the binary name of
bluetoothd really long time ago.
---
 Makefile.am                   | 2 +-
 plugins/gatt-example.c        | 2 +-
 plugins/neard.c               | 2 +-
 profiles/proximity/main.c     | 2 +-
 profiles/proximity/reporter.c | 2 +-
 src/adapter.c                 | 2 +-
 src/attrib-server.c           | 2 +-
 src/device.c                  | 2 +-
 src/main.c                    | 2 +-
 src/{hcid.h => main.h}        | 0
 src/plugin.c                  | 2 +-
 src/profile.c                 | 2 +-
 src/rfkill.c                  | 2 +-
 src/sdpd-server.c             | 2 +-
 src/sdpd-service.c            | 2 +-
 15 files changed, 14 insertions(+), 14 deletions(-)
 rename src/{hcid.h => main.h} (100%)

diff --git a/Makefile.am b/Makefile.am
index f75b8d6..74d11de 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -127,7 +127,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/bluetooth.ver \
 			src/main.c src/log.h src/log.c \
 			src/systemd.h src/systemd.c \
-			src/rfkill.c src/hcid.h src/sdpd.h \
+			src/rfkill.c src/main.h src/sdpd.h \
 			src/sdpd-server.c src/sdpd-request.c \
 			src/sdpd-service.c src/sdpd-database.c \
 			src/attrib-server.h src/attrib-server.c \
diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index bd0fbff..fd8fcab 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -32,7 +32,7 @@
 
 #include "lib/uuid.h"
 #include "plugin.h"
-#include "hcid.h"
+#include "main.h"
 #include "log.h"
 #include "attrib/gattrib.h"
 #include "attrib/gatt-service.h"
diff --git a/plugins/neard.c b/plugins/neard.c
index c60c076..7752ba3 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -39,7 +39,7 @@
 #include "device.h"
 #include "eir.h"
 #include "agent.h"
-#include "hcid.h"
+#include "main.h"
 
 #define NEARD_NAME "org.neard"
 #define NEARD_PATH "/"
diff --git a/profiles/proximity/main.c b/profiles/proximity/main.c
index 46468d2..ca3b11e 100644
--- a/profiles/proximity/main.c
+++ b/profiles/proximity/main.c
@@ -34,7 +34,7 @@
 #include "log.h"
 #include "plugin.h"
 #include "manager.h"
-#include "hcid.h"
+#include "main.h"
 
 static GKeyFile *config = NULL;
 
diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index 31c33ef..72e816b 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -42,7 +42,7 @@
 #include "error.h"
 #include "device.h"
 #include "profile.h"
-#include "hcid.h"
+#include "main.h"
 #include "attrib/gattrib.h"
 #include "attrib/att.h"
 #include "attrib/gatt.h"
diff --git a/src/adapter.c b/src/adapter.c
index e553626..e45c467 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -52,7 +52,7 @@
 #include "lib/mgmt.h"
 #include "src/shared/mgmt.h"
 
-#include "hcid.h"
+#include "main.h"
 #include "sdpd.h"
 #include "adapter.h"
 #include "device.h"
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 99656e3..ca9e2ff 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -43,7 +43,7 @@
 #include "log.h"
 #include <btio/btio.h>
 #include "sdpd.h"
-#include "hcid.h"
+#include "main.h"
 #include "adapter.h"
 #include "device.h"
 #include "attrib/gattrib.h"
diff --git a/src/device.c b/src/device.c
index 4320234..9a8dbbe 100644
--- a/src/device.c
+++ b/src/device.c
@@ -50,7 +50,7 @@
 #include "lib/uuid.h"
 #include "lib/mgmt.h"
 #include "attrib/att.h"
-#include "hcid.h"
+#include "main.h"
 #include "adapter.h"
 #include "attrib/gattrib.h"
 #include "attio.h"
diff --git a/src/main.c b/src/main.c
index 933c20f..d78de41 100644
--- a/src/main.c
+++ b/src/main.c
@@ -48,7 +48,7 @@
 #include "log.h"
 
 #include "lib/uuid.h"
-#include "hcid.h"
+#include "main.h"
 #include "sdpd.h"
 #include "adapter.h"
 #include "device.h"
diff --git a/src/hcid.h b/src/main.h
similarity index 100%
rename from src/hcid.h
rename to src/main.h
diff --git a/src/plugin.c b/src/plugin.c
index f231f34..61afc3d 100644
--- a/src/plugin.c
+++ b/src/plugin.c
@@ -37,7 +37,7 @@
 
 #include "plugin.h"
 #include "log.h"
-#include "hcid.h"
+#include "main.h"
 
 static GSList *plugins = NULL;
 
diff --git a/src/profile.c b/src/profile.c
index fc62f02..6949dcb 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -50,7 +50,7 @@
 #include "adapter.h"
 #include "device.h"
 #include "profile.h"
-#include "hcid.h"
+#include "main.h"
 
 #define DUN_DEFAULT_CHANNEL	1
 #define SPP_DEFAULT_CHANNEL	3
diff --git a/src/rfkill.c b/src/rfkill.c
index 70588c0..07609b3 100644
--- a/src/rfkill.c
+++ b/src/rfkill.c
@@ -37,7 +37,7 @@
 
 #include "log.h"
 #include "adapter.h"
-#include "hcid.h"
+#include "main.h"
 
 enum rfkill_type {
 	RFKILL_TYPE_ALL = 0,
diff --git a/src/sdpd-server.c b/src/sdpd-server.c
index bc96d3c..3046a9b 100644
--- a/src/sdpd-server.c
+++ b/src/sdpd-server.c
@@ -45,7 +45,7 @@
 
 #include <glib.h>
 
-#include "hcid.h"
+#include "main.h"
 #include "log.h"
 #include "sdpd.h"
 
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index b6b135d..478ebb1 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -44,7 +44,7 @@
 #include <glib.h>
 #include <dbus/dbus.h>
 
-#include "hcid.h"
+#include "main.h"
 #include "sdpd.h"
 #include "log.h"
 #include "adapter.h"
-- 
1.8.1.2

Re: [PATCH BlueZ 1/3] audio: Remove profile enabling/disabling logic

[Mikel Astiz]

Hi Luis,

On Fri, Mar 1, 2013 at 1:26 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This should be handle by the core for all profiles
> ---
>  profiles/audio/audio.conf |  4 ---
>  profiles/audio/manager.c  | 62 ++++++-----------------------------------------
>  2 files changed, 8 insertions(+), 58 deletions(-)
>
> diff --git a/profiles/audio/audio.conf b/profiles/audio/audio.conf
> index f556610..067b3fc 100644
> --- a/profiles/audio/audio.conf
> +++ b/profiles/audio/audio.conf
> @@ -6,7 +6,3 @@
>
>  # Switch to master role for incoming connections (defaults to true)
>  #Master=true
> -
> -# If we want to disable support for specific services
> -# Defaults to supporting the services: Sink, Control
> -#Disable=Source
> diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
> index 934227e..42a2b58 100644
> --- a/profiles/audio/manager.c
> +++ b/profiles/audio/manager.c
> @@ -70,12 +70,6 @@
>  static GKeyFile *config = NULL;
>  static GSList *devices = NULL;
>
> -static struct enabled_interfaces enabled = {
> -       .sink           = TRUE,
> -       .source         = FALSE,
> -       .control        = TRUE,
> -};
> -
>  static struct audio_device *get_audio_dev(struct btd_device *device)
>  {
>         return manager_get_audio_device(device, TRUE);
> @@ -410,47 +404,12 @@ void audio_control_disconnected(struct btd_device *dev, int err)
>
>  int audio_manager_init(GKeyFile *conf)
>  {
> -       char **list;
> -       int i;
> -
> -       if (!conf)
> -               goto proceed;
> -
> -       config = conf;
> -
> -       list = g_key_file_get_string_list(config, "General", "Enable",
> -                                               NULL, NULL);
> -       for (i = 0; list && list[i] != NULL; i++) {
> -               if (g_str_equal(list[i], "Sink"))
> -                       enabled.sink = TRUE;
> -               else if (g_str_equal(list[i], "Source"))
> -                       enabled.source = TRUE;
> -               else if (g_str_equal(list[i], "Control"))
> -                       enabled.control = TRUE;
> -       }
> -       g_strfreev(list);
> -
> -       list = g_key_file_get_string_list(config, "General", "Disable",
> -                                               NULL, NULL);
> -       for (i = 0; list && list[i] != NULL; i++) {
> -               if (g_str_equal(list[i], "Sink"))
> -                       enabled.sink = FALSE;
> -               else if (g_str_equal(list[i], "Source"))
> -                       enabled.source = FALSE;
> -               else if (g_str_equal(list[i], "Control"))
> -                       enabled.control = FALSE;
> -       }
> -       g_strfreev(list);
> +       if (conf)
> +               config = conf;
>
> -proceed:
> -       if (enabled.source)
> -               btd_profile_register(&a2dp_source_profile);
> -
> -       if (enabled.sink)
> -               btd_profile_register(&a2dp_sink_profile);
> -
> -       if (enabled.control)
> -               btd_profile_register(&avrcp_profile);
> +       btd_profile_register(&a2dp_source_profile);
> +       btd_profile_register(&a2dp_sink_profile);
> +       btd_profile_register(&avrcp_profile);
>
>         btd_register_adapter_driver(&media_driver);
>
> @@ -464,14 +423,9 @@ void audio_manager_exit(void)
>                 config = NULL;
>         }
>
> -       if (enabled.source)
> -               btd_profile_unregister(&a2dp_source_profile);
> -
> -       if (enabled.sink)
> -               btd_profile_unregister(&a2dp_sink_profile);
> -
> -       if (enabled.control)
> -               btd_profile_unregister(&avrcp_profile);
> +       btd_profile_unregister(&a2dp_source_profile);
> +       btd_profile_unregister(&a2dp_sink_profile);
> +       btd_profile_unregister(&avrcp_profile);
>
>         btd_unregister_adapter_driver(&media_driver);
>  }
> --

Enabling all profiles by default is one thing but dropping the enable
flags is IMO too much. Beyond the desktop setups, e.g. in automotive
(or probably even for phones), some roles would be disabled.

What I'm missing is how external profiles would fit for this purpose.
Would BlueZ care at all or would for example oFono require similar
flags?

Cheers,
Mikel

Re: [PATCH BlueZ 1/3] audio: Remove profile enabling/disabling logic

[Luiz Augusto von Dentz]

Hi Mikel,

On Fri, Mar 1, 2013 at 3:48 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> Enabling all profiles by default is one thing but dropping the enable
> flags is IMO too much. Beyond the desktop setups, e.g. in automotive
> (or probably even for phones), some roles would be disabled.

For that reason 2/3 introduces DisableProfiles in the main.conf, so it
is generic for any profile.

> What I'm missing is how external profiles would fit for this purpose.
> Would BlueZ care at all or would for example oFono require similar
> flags?

In theory we could disable even external profiles, but that doesn't
mean this couldn't be disabled directly on the component so for now I
left it out.

--
Luiz Augusto von Dentz

Re: [PATCH BlueZ 3/3] core: Rename hcid.h to main.h

[Marcel Holtmann]

Hi Luiz,


> main seems more logical than hcid which use to be the binary name of
> bluetoothd really long time ago.
> ---
> Makefile.am                   | 2 +-
> plugins/gatt-example.c        | 2 +-
> plugins/neard.c               | 2 +-
> profiles/proximity/main.c     | 2 +-
> profiles/proximity/reporter.c | 2 +-
> src/adapter.c                 | 2 +-
> src/attrib-server.c           | 2 +-
> src/device.c                  | 2 +-
> src/main.c                    | 2 +-
> src/{hcid.h => main.h}        | 0
> src/plugin.c                  | 2 +-
> src/profile.c                 | 2 +-
> src/rfkill.c                  | 2 +-
> src/sdpd-server.c             | 2 +-
> src/sdpd-service.c            | 2 +-
> 15 files changed, 14 insertions(+), 14 deletions(-)
> rename src/{hcid.h => main.h} (100%)

NAK. main.h is one single header we are not going to have. I do not want main.c to share anything actually. I want main.c to consume everything else.

Regards

Marcel

Re: [PATCH BlueZ 2/3] core: Add DisableProfiles entry to main.conf

[Marcel Holtmann]

Hi Luiz,


> This entry can be used to globally disable profiles, this is specially
> useful for qualification purposes where some platforms may decide to
> only qualify a subset of the supported profiles.
> ---
> src/hcid.h    |  1 +
> src/main.c    | 10 ++++++++++
> src/main.conf |  4 ++++
> src/profile.c | 24 ++++++++++++++++++++++++
> 4 files changed, 39 insertions(+)
> 
> diff --git a/src/hcid.h b/src/hcid.h
> index ea67cc2..f465a2b 100644
> --- a/src/hcid.h
> +++ b/src/hcid.h
> @@ -32,6 +32,7 @@ struct main_opts {
> 	gboolean	reverse_sdp;
> 	gboolean	name_resolv;
> 	gboolean	debug_keys;
> +	char		**disabled_profiles;
> 
> 	uint16_t	did_source;
> 	uint16_t	did_vendor;
> diff --git a/src/main.c b/src/main.c
> index 1e40ebc..933c20f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -76,6 +76,7 @@ static const char * const supported_options[] = {
> 	"ReverseServiceDiscovery",
> 	"NameResolving",
> 	"DebugKeys",
> +	"DisableProfiles"
> };
> 
> static GKeyFile *load_config(const char *file)
> @@ -263,6 +264,15 @@ static void parse_config(GKeyFile *config)
> 		g_clear_error(&err);
> 	else
> 		main_opts.debug_keys = boolean;
> +
> +	str = g_key_file_get_string(config, "General", "DisableProfiles", &err);
> +	if (err) {
> +		DBG("%s", err->message);
> +		g_clear_error(&err);
> +	} else {
> +		main_opts.disabled_profiles = g_strsplit(str, " ", -1);
> +		g_free(str);
> +	}
> }

I am not a huge fan of adding this one back. Do we really need it?

Regards

Marcel

Re: [PATCH BlueZ 3/3] core: Rename hcid.h to main.h

[Luiz Augusto von Dentz]

Hi Marcel,

On Fri, Mar 1, 2013 at 6:15 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>
>> main seems more logical than hcid which use to be the binary name of
>> bluetoothd really long time ago.
>> ---
>> Makefile.am                   | 2 +-
>> plugins/gatt-example.c        | 2 +-
>> plugins/neard.c               | 2 +-
>> profiles/proximity/main.c     | 2 +-
>> profiles/proximity/reporter.c | 2 +-
>> src/adapter.c                 | 2 +-
>> src/attrib-server.c           | 2 +-
>> src/device.c                  | 2 +-
>> src/main.c                    | 2 +-
>> src/{hcid.h => main.h}        | 0
>> src/plugin.c                  | 2 +-
>> src/profile.c                 | 2 +-
>> src/rfkill.c                  | 2 +-
>> src/sdpd-server.c             | 2 +-
>> src/sdpd-service.c            | 2 +-
>> 15 files changed, 14 insertions(+), 14 deletions(-)
>> rename src/{hcid.h => main.h} (100%)
>
> NAK. main.h is one single header we are not going to have. I do not want main.c to share anything actually. I want main.c to consume everything else.

Well what is left, bluetoothd.h? Since this is mainly for config
options I guess this would be fine or you have a better idea?


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 2/3] core: Add DisableProfiles entry to main.conf

[Luiz Augusto von Dentz]

Hi Marcel,

On Fri, Mar 1, 2013 at 6:17 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>
>> This entry can be used to globally disable profiles, this is specially
>> useful for qualification purposes where some platforms may decide to
>> only qualify a subset of the supported profiles.
>> ---
>> src/hcid.h    |  1 +
>> src/main.c    | 10 ++++++++++
>> src/main.conf |  4 ++++
>> src/profile.c | 24 ++++++++++++++++++++++++
>> 4 files changed, 39 insertions(+)
>>
>> diff --git a/src/hcid.h b/src/hcid.h
>> index ea67cc2..f465a2b 100644
>> --- a/src/hcid.h
>> +++ b/src/hcid.h
>> @@ -32,6 +32,7 @@ struct main_opts {
>>       gboolean        reverse_sdp;
>>       gboolean        name_resolv;
>>       gboolean        debug_keys;
>> +     char            **disabled_profiles;
>>
>>       uint16_t        did_source;
>>       uint16_t        did_vendor;
>> diff --git a/src/main.c b/src/main.c
>> index 1e40ebc..933c20f 100644
>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -76,6 +76,7 @@ static const char * const supported_options[] = {
>>       "ReverseServiceDiscovery",
>>       "NameResolving",
>>       "DebugKeys",
>> +     "DisableProfiles"
>> };
>>
>> static GKeyFile *load_config(const char *file)
>> @@ -263,6 +264,15 @@ static void parse_config(GKeyFile *config)
>>               g_clear_error(&err);
>>       else
>>               main_opts.debug_keys = boolean;
>> +
>> +     str = g_key_file_get_string(config, "General", "DisableProfiles", &err);
>> +     if (err) {
>> +             DBG("%s", err->message);
>> +             g_clear_error(&err);
>> +     } else {
>> +             main_opts.disabled_profiles = g_strsplit(str, " ", -1);
>> +             g_free(str);
>> +     }
>> }
>
> I am not a huge fan of adding this one back. Do we really need it?

This is for profiles we used to have for plugins but they are not 1:1
to profiles so -p/-P may not be enough, we could instead take as
command line option but 128 bit UUID sounds a little too messy to pass
in the command line. Mikel also comment about having this to block
external process to register profiles, maybe it is a good idea since
we don't have to create a similar mechanism to other components such
as obexd, oFono and PulseAudio just to disable certain roles.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 2/3] core: Add DisableProfiles entry to main.conf

[Marcel Holtmann]

Hi Luiz,

>>> This entry can be used to globally disable profiles, this is specially
>>> useful for qualification purposes where some platforms may decide to
>>> only qualify a subset of the supported profiles.
>>> ---
>>> src/hcid.h    |  1 +
>>> src/main.c    | 10 ++++++++++
>>> src/main.conf |  4 ++++
>>> src/profile.c | 24 ++++++++++++++++++++++++
>>> 4 files changed, 39 insertions(+)
>>> 
>>> diff --git a/src/hcid.h b/src/hcid.h
>>> index ea67cc2..f465a2b 100644
>>> --- a/src/hcid.h
>>> +++ b/src/hcid.h
>>> @@ -32,6 +32,7 @@ struct main_opts {
>>>      gboolean        reverse_sdp;
>>>      gboolean        name_resolv;
>>>      gboolean        debug_keys;
>>> +     char            **disabled_profiles;
>>> 
>>>      uint16_t        did_source;
>>>      uint16_t        did_vendor;
>>> diff --git a/src/main.c b/src/main.c
>>> index 1e40ebc..933c20f 100644
>>> --- a/src/main.c
>>> +++ b/src/main.c
>>> @@ -76,6 +76,7 @@ static const char * const supported_options[] = {
>>>      "ReverseServiceDiscovery",
>>>      "NameResolving",
>>>      "DebugKeys",
>>> +     "DisableProfiles"
>>> };
>>> 
>>> static GKeyFile *load_config(const char *file)
>>> @@ -263,6 +264,15 @@ static void parse_config(GKeyFile *config)
>>>              g_clear_error(&err);
>>>      else
>>>              main_opts.debug_keys = boolean;
>>> +
>>> +     str = g_key_file_get_string(config, "General", "DisableProfiles", &err);
>>> +     if (err) {
>>> +             DBG("%s", err->message);
>>> +             g_clear_error(&err);
>>> +     } else {
>>> +             main_opts.disabled_profiles = g_strsplit(str, " ", -1);
>>> +             g_free(str);
>>> +     }
>>> }
>> 
>> I am not a huge fan of adding this one back. Do we really need it?
> 
> This is for profiles we used to have for plugins but they are not 1:1
> to profiles so -p/-P may not be enough, we could instead take as
> command line option but 128 bit UUID sounds a little too messy to pass
> in the command line. Mikel also comment about having this to block
> external process to register profiles, maybe it is a good idea since
> we don't have to create a similar mechanism to other components such
> as obexd, oFono and PulseAudio just to disable certain roles.

can we at least then make it part of src/profile.c directly and not try to hack it in this way. I mean like after reading the config file, let it tell src/profile.c which profiles are blacklisted.

Regards

Marcel

Re: [PATCH BlueZ 3/3] core: Rename hcid.h to main.h

[Marcel Holtmann]

Hi Luiz,

>>> main seems more logical than hcid which use to be the binary name of
>>> bluetoothd really long time ago.
>>> ---
>>> Makefile.am                   | 2 +-
>>> plugins/gatt-example.c        | 2 +-
>>> plugins/neard.c               | 2 +-
>>> profiles/proximity/main.c     | 2 +-
>>> profiles/proximity/reporter.c | 2 +-
>>> src/adapter.c                 | 2 +-
>>> src/attrib-server.c           | 2 +-
>>> src/device.c                  | 2 +-
>>> src/main.c                    | 2 +-
>>> src/{hcid.h => main.h}        | 0
>>> src/plugin.c                  | 2 +-
>>> src/profile.c                 | 2 +-
>>> src/rfkill.c                  | 2 +-
>>> src/sdpd-server.c             | 2 +-
>>> src/sdpd-service.c            | 2 +-
>>> 15 files changed, 14 insertions(+), 14 deletions(-)
>>> rename src/{hcid.h => main.h} (100%)
>> 
>> NAK. main.h is one single header we are not going to have. I do not want main.c to share anything actually. I want main.c to consume everything else.
> 
> Well what is left, bluetoothd.h? Since this is mainly for config
> options I guess this would be fine or you have a better idea?

within ConnMan and oFono we are using connman.h and ofono.h. Since I am short of coming up with a better name than hcid.h that fits, lets leave it like this.

However one thing that needs clearly to be done is to minimise the usage of this header. For example why is it leaking into profiles and plugins in the first place. So please start cleaning that one up first.

Regards

Marcel