[RFC 0/6] android: Configuration command

[RFC 0/6] android: Configuration command

[Szymon Janc]

Hi,

This will allow to pass Android properties over IPC and use them
in daemon for customization eg. Device Information or default adapter name.

Open points:
 - what properties should be read, currently I use ro.product.* as those
   are present on AOSP Nexus devices, but eg. on Xperia phones there
   seem to be ro.semc.product.* with more userfriendly name, so this
   may vary upon devices. (eg ST18i vs Xperia Ray in model property).
   Or maybe just use ro.bluetooth.* and require integrator to set those
   accrodingly?

 - should we also use this instead of 'Mode' passed on service init?
   This would keep handling of all properities consistent but I'm fine
   if we decide to keep Modes special.

Comments are welcome.

BR
Szymon Janc

Szymon Janc (6):
  android/hal-msg: Add configuration command
  android: Add support for configuration command
  android: Add functions for getting configuration options
  android/hal-bluetooth: Add support for sending configuration
  android/bluetooth: Use system id for default adapter name
  android/gatt: Use configuration data for device information profile

 android/bluetooth.c     |  16 +++++--
 android/gatt.c          | 125 +++++++++++++++++++++++++-----------------------
 android/hal-bluetooth.c |  48 +++++++++++++++++++
 android/hal-msg.h       |  18 +++++++
 android/main.c          |  90 ++++++++++++++++++++++++++++++++++
 android/utils.h         |   5 ++
 6 files changed, 238 insertions(+), 64 deletions(-)

-- 
1.9.1

[RFC 1/6] android/hal-msg: Add configuration command

[Szymon Janc]

---
 android/hal-msg.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index 09bd9a0..8daea2d 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -69,6 +69,24 @@ struct hal_cmd_unregister_module {
 	uint8_t service_id;
 } __attribute__((packed));
 
+#define HAL_CONFIG_MANUFACTURER_NAME	0x00
+#define HAL_CONFIG_SYSTEM_ID		0x01
+#define HAL_CONFIG_MODEL_NUMBER		0x02
+#define HAL_CONFIG_SERIAL_NUMBER	0x03
+
+#define HAL_CONFIG_PROP_LEN		92
+
+struct hal_config_prop {
+	uint8_t type;
+	uint8_t buf[HAL_CONFIG_PROP_LEN];
+} __attribute__((packed));
+
+#define HAL_OP_CONFIGURATION		0x03
+struct hal_cmd_configuration {
+	uint8_t num;
+	struct hal_config_prop props[0];
+} __attribute__((packed));
+
 /* Bluetooth Core HAL API */
 
 #define HAL_OP_ENABLE			0x01
-- 
1.9.1

[RFC 2/6] android: Add support for configuration command

[Szymon Janc]

---
 android/main.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/android/main.c b/android/main.c
index 0a0c150..e85f696 100644
--- a/android/main.c
+++ b/android/main.c
@@ -45,6 +45,7 @@
 
 #include "src/log.h"
 #include "src/sdpd.h"
+#include "src/shared/util.h"
 
 #include "lib/bluetooth.h"
 
@@ -64,6 +65,11 @@
 #define STARTUP_GRACE_SECONDS 5
 #define SHUTDOWN_GRACE_SECONDS 10
 
+static char *config_manufacturer = NULL;
+static char *config_system_id = NULL;
+static char *config_model = NULL;
+static char *config_serial = NULL;
+
 static guint bluetooth_start_timeout = 0;
 
 static bdaddr_t adapter_bdaddr;
@@ -221,11 +227,65 @@ failed:
 								status);
 }
 
+static char *get_prop(char *prop, const uint8_t *buf)
+{
+	if (!prop)
+		prop = malloc0(HAL_CONFIG_PROP_LEN);
+
+	if (!prop)
+		return NULL;
+
+	memcpy(prop, buf, HAL_CONFIG_PROP_LEN);
+	prop[HAL_CONFIG_PROP_LEN - 1] = '\0';
+
+	return prop;
+}
+
+static void configuration(const void *buf, uint16_t len)
+{
+	const struct hal_cmd_configuration *cmd = buf;
+	unsigned int i;
+
+	if (len != sizeof(*cmd) + (cmd->num * sizeof(cmd->props[0]))) {
+		error("Invalid configuration command, terminating");
+		raise(SIGTERM);
+		return;
+	}
+
+	for (i = 0; i < cmd->num; i++) {
+		switch (cmd->props[i].type) {
+		case HAL_CONFIG_MANUFACTURER_NAME:
+			config_manufacturer = get_prop(config_manufacturer,
+							cmd->props[i].buf);
+			break;
+		case HAL_CONFIG_SYSTEM_ID:
+			config_system_id = get_prop(config_system_id,
+							cmd->props[i].buf);
+			break;
+		case HAL_CONFIG_MODEL_NUMBER:
+			config_model = get_prop(config_model,
+							cmd->props[i].buf);
+			break;
+		case HAL_CONFIG_SERIAL_NUMBER:
+			config_serial = get_prop(config_serial,
+							cmd->props[i].buf);
+			break;
+		default:
+			break;
+		}
+	}
+
+	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_CORE, HAL_OP_CONFIGURATION,
+							HAL_STATUS_SUCCESS);
+}
+
 static const struct ipc_handler cmd_handlers[] = {
 	/* HAL_OP_REGISTER_MODULE */
 	{ service_register, false, sizeof(struct hal_cmd_register_module) },
 	/* HAL_OP_UNREGISTER_MODULE */
 	{ service_unregister, false, sizeof(struct hal_cmd_unregister_module) },
+	/* HAL_OP_CONFIGURATION */
+	{ configuration, true, sizeof(struct hal_cmd_configuration) },
 };
 
 static void bluetooth_stopped(void)
-- 
1.9.1

[RFC 3/6] android: Add functions for getting configuration options

[Szymon Janc]

---
 android/main.c  | 30 ++++++++++++++++++++++++++++++
 android/utils.h |  5 +++++
 2 files changed, 35 insertions(+)

diff --git a/android/main.c b/android/main.c
index e85f696..bf4235a 100644
--- a/android/main.c
+++ b/android/main.c
@@ -61,6 +61,10 @@
 #include "handsfree.h"
 #include "gatt.h"
 #include "health.h"
+#include "utils.h"
+
+#define DEFAULT_MANUFACTURER_NAME "BlueZ"
+#define DEFAULT_SYSTEM_ID "BlueZ for Android"
 
 #define STARTUP_GRACE_SECONDS 5
 #define SHUTDOWN_GRACE_SECONDS 10
@@ -80,6 +84,32 @@ static struct ipc *hal_ipc = NULL;
 
 static bool services[HAL_SERVICE_ID_MAX + 1] = { false };
 
+const char *bt_config_get_manufacturer(void)
+{
+	if (config_manufacturer)
+		return config_manufacturer;
+
+	return DEFAULT_MANUFACTURER_NAME;
+}
+
+const char *bt_config_get_system_id(void)
+{
+	if (config_system_id)
+		return config_system_id;
+
+	return DEFAULT_SYSTEM_ID;
+}
+
+const char *bt_config_get_model(void)
+{
+	return config_model;
+}
+
+const char *bt_config_get_serial(void)
+{
+	return config_serial;
+}
+
 static void service_register(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_register_module *m = buf;
diff --git a/android/utils.h b/android/utils.h
index 560e991..e1e791a 100644
--- a/android/utils.h
+++ b/android/utils.h
@@ -30,3 +30,8 @@ static inline void bdaddr2android(const bdaddr_t *src, void *buf)
 {
 	baswap(buf, src);
 }
+
+const char *bt_config_get_manufacturer(void);
+const char *bt_config_get_system_id(void);
+const char *bt_config_get_model(void);
+const char *bt_config_get_serial(void);
-- 
1.9.1

[RFC 4/6] android/hal-bluetooth: Add support for sending configuration

[Szymon Janc]

---
 android/hal-bluetooth.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 2155978..05d785b 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -31,6 +31,11 @@
 
 #define MODE_PROPERTY_NAME "persist.sys.bluetooth.mode"
 
+#define CONFIG_PROP_MANUFACTURER "ro.product.manufacturer"
+#define CONFIG_PROP_SYSTEM_ID "ro.product.name"
+#define CONFIG_PROP_MODEL "ro.product.model"
+#define CONFIG_PROP_SERIAL "ro.serialno"
+
 static const bt_callbacks_t *bt_hal_cbacks = NULL;
 
 #define enum_prop_to_hal(prop, hal_prop, type) do { \
@@ -431,6 +436,43 @@ static uint8_t get_mode(void)
 	return HAL_MODE_DEFAULT;
 }
 
+static int send_configuration(void)
+{
+	char buf[IPC_MTU];
+	struct hal_cmd_configuration *cmd = (void *) buf;
+	char prop[PROPERTY_VALUE_MAX];
+
+	cmd->num = 0;
+
+	if (property_get(CONFIG_PROP_MANUFACTURER, prop, NULL) > 0) {
+		strcpy((char *) cmd->props[cmd->num].buf, prop);
+		cmd->props[cmd->num].type = HAL_CONFIG_MANUFACTURER_NAME;
+		cmd->num ++;
+	}
+
+	if (property_get(CONFIG_PROP_SYSTEM_ID, prop, NULL) > 0) {
+		strcpy((char *) cmd->props[cmd->num].buf, prop);
+		cmd->props[cmd->num].type = HAL_CONFIG_SYSTEM_ID;
+		cmd->num ++;
+	}
+
+	if (property_get(CONFIG_PROP_MODEL, prop, NULL) > 0) {
+		strcpy((char *) cmd->props[cmd->num].buf, prop);
+		cmd->props[cmd->num].type = HAL_CONFIG_MODEL_NUMBER;
+		cmd->num ++;
+	}
+
+	if (property_get(CONFIG_PROP_SERIAL, prop, NULL) > 0) {
+		strcpy((char *) cmd->props[cmd->num].buf, prop);
+		cmd->props[cmd->num].type = HAL_CONFIG_SERIAL_NUMBER;
+		cmd->num ++;
+	}
+
+	return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_CONFIGURATION,
+				sizeof(*cmd) + cmd->num * sizeof(cmd->props[0]),
+				cmd, NULL, NULL, NULL);
+}
+
 static int init(bt_callbacks_t *callbacks)
 {
 	struct hal_cmd_register_module cmd;
@@ -451,6 +493,12 @@ static int init(bt_callbacks_t *callbacks)
 		return BT_STATUS_FAIL;
 	}
 
+	status = send_configuration();
+	if (status != BT_STATUS_SUCCESS) {
+		error("Failed to send configuration");
+		goto fail;
+	}
+
 	cmd.service_id = HAL_SERVICE_ID_BLUETOOTH;
 	cmd.mode = get_mode();
 
-- 
1.9.1

[RFC 5/6] android/bluetooth: Use system id for default adapter name

[Szymon Janc]

---
 android/bluetooth.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 2a7c0e6..e8d6762 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -53,8 +53,6 @@
 #include "utils.h"
 #include "bluetooth.h"
 
-#define DEFAULT_ADAPTER_NAME "BlueZ for Android"
-
 #define DUT_MODE_FILE "/sys/kernel/debug/bluetooth/hci%u/dut_mode"
 
 #define SETTINGS_FILE ANDROID_STORAGEDIR"/settings"
@@ -189,7 +187,11 @@ static void store_adapter_config(void)
 	ba2str(&adapter.bdaddr, addr);
 
 	g_key_file_set_string(key_file, "General", "Address", addr);
-	g_key_file_set_string(key_file, "General", "Name", adapter.name);
+
+	if (adapter.name)
+		g_key_file_set_string(key_file, "General", "Name",
+				adapter.name);
+
 	g_key_file_set_integer(key_file, "General", "DiscoverableTimeout",
 						adapter.discoverable_timeout);
 
@@ -2413,7 +2415,6 @@ static void read_info_complete(uint8_t status, uint16_t length,
 
 	if (!bacmp(&adapter.bdaddr, BDADDR_ANY)) {
 		bacpy(&adapter.bdaddr, &rp->bdaddr);
-		adapter.name = g_strdup(DEFAULT_ADAPTER_NAME);
 		store_adapter_config();
 	} else if (bacmp(&adapter.bdaddr, &rp->bdaddr)) {
 		error("Bluetooth address mismatch");
@@ -2421,7 +2422,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
 		goto failed;
 	}
 
-	if (g_strcmp0(adapter.name, (const char *) rp->name))
+	if (adapter.name && g_strcmp0(adapter.name, (const char *) rp->name))
 		set_adapter_name((uint8_t *)adapter.name, strlen(adapter.name));
 
 	set_adapter_class();
@@ -4043,6 +4044,11 @@ bool bt_bluetooth_register(struct ipc *ipc, uint8_t mode)
 		return false;
 	}
 
+	if (!adapter.name) {
+		adapter.name = g_strdup(bt_config_get_system_id());
+		set_adapter_name((uint8_t *)adapter.name, strlen(adapter.name));
+	}
+
 	hal_ipc = ipc;
 
 	ipc_register(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, cmd_handlers,
-- 
1.9.1

[RFC 6/6] android/gatt: Use configuration data for device information profile

[Szymon Janc]

---
 android/gatt.c | 125 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 66 insertions(+), 59 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 157ebe6..c213eb0 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4532,25 +4532,6 @@ static void register_gap_service(void)
 	gatt_db_service_set_active(gatt_db, gap_srvc_data.srvc , true);
 }
 
-/* TODO: Get those data from device possible via androig/bluetooth.c */
-static struct device_info {
-	const char *manufacturer_name;
-	const char *system_id;
-	const char *model_number;
-	const char *serial_number;
-	const char *firmware_rev;
-	const char *hardware_rev;
-	const char *software_rev;
-} device_info = {
-	.manufacturer_name =	"BlueZ",
-	.system_id =		"BlueZ for Android",
-	.model_number =		"model no",
-	.serial_number =	"serial no",
-	.firmware_rev =		"firmware rev",
-	.hardware_rev =		"hardware rev",
-	.software_rev =		"software rev",
-};
-
 static void device_info_read_cb(uint16_t handle, uint16_t offset,
 					uint8_t att_opcode, bdaddr_t *bdaddr,
 					void *user_data)
@@ -4575,6 +4556,7 @@ static void register_device_info_service(void)
 {
 	bt_uuid_t uuid;
 	uint16_t srvc_handle;
+	const char *data;
 
 	DBG("");
 
@@ -4583,47 +4565,72 @@ static void register_device_info_service(void)
 	srvc_handle = gatt_db_add_service(gatt_db, &uuid, true, 15);
 
 	/* User data are not const hence (void *) cast is used */
-	bt_uuid16_create(&uuid, GATT_CHARAC_SYSTEM_ID);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.system_id);
-
-	bt_uuid16_create(&uuid, GATT_CHARAC_MODEL_NUMBER_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.model_number);
-
-	bt_uuid16_create(&uuid, GATT_CHARAC_SERIAL_NUMBER_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.serial_number);
 
-	bt_uuid16_create(&uuid, GATT_CHARAC_FIRMWARE_REVISION_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.firmware_rev);
-
-	bt_uuid16_create(&uuid, GATT_CHARAC_HARDWARE_REVISION_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.hardware_rev);
-
-	bt_uuid16_create(&uuid, GATT_CHARAC_SOFTWARE_REVISION_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.software_rev);
-
-	bt_uuid16_create(&uuid, GATT_CHARAC_MANUFACTURER_NAME_STRING);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
-					GATT_CHR_PROP_READ,
-					device_info_read_cb, NULL,
-					(void *) device_info.manufacturer_name);
+	data = bt_config_get_system_id();
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_SYSTEM_ID);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
+
+	data = bt_config_get_model();
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_MODEL_NUMBER_STRING);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
+
+	data = bt_config_get_serial();
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_SERIAL_NUMBER_STRING);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
+
+	/* TODO */
+	data = NULL;
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_FIRMWARE_REVISION_STRING);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
+
+	/* TODO */
+	data = NULL;
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_HARDWARE_REVISION_STRING);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
+
+	/* TODO */
+	data = NULL;
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_SOFTWARE_REVISION_STRING);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
+
+	data = bt_config_get_manufacturer();
+	if (data) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_MANUFACTURER_NAME_STRING);
+		gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, 0,
+						GATT_CHR_PROP_READ,
+						device_info_read_cb, NULL,
+						(void *) data);
+	}
 
 	gatt_db_service_set_active(gatt_db, srvc_handle, true);
 }
-- 
1.9.1

Re: [RFC 0/6] android: Configuration command

[Marcel Holtmann]

Hi Szymon,

> This will allow to pass Android properties over IPC and use them
> in daemon for customization eg. Device Information or default adapter name.
> 
> Open points:
> - what properties should be read, currently I use ro.product.* as those
>   are present on AOSP Nexus devices, but eg. on Xperia phones there
>   seem to be ro.semc.product.* with more userfriendly name, so this
>   may vary upon devices. (eg ST18i vs Xperia Ray in model property).
>   Or maybe just use ro.bluetooth.* and require integrator to set those
>   accrodingly?

I think we should take ro.bluetooth.* and fallback to ro.product.*. If some manufactures want to do something different, they can either set ro.bluetooth.* or hack the code.

Nexus devices should default to something useful. We have to take that as standard location for product information.

Since non of the DID details are mandatory, I prefer also that they are just not present if they are not configured in any property. Maybe BlueZ defaults for manufacturer and systemd id.

> - should we also use this instead of 'Mode' passed on service init?
>   This would keep handling of all properities consistent but I'm fine
>   if we decide to keep Modes special.

Do not understand the question. What do you mean.

Regards

Marcel

Re: [RFC 0/6] android: Configuration command

[Szymon Janc]

Hi Marcel,

On Tuesday 13 May 2014 11:54:55 Marcel Holtmann wrote:
> Hi Szymon,
> 
> > This will allow to pass Android properties over IPC and use them
> > in daemon for customization eg. Device Information or default adapter
> > name.
> > 
> > Open points:
> > - what properties should be read, currently I use ro.product.* as those
> > 
> >   are present on AOSP Nexus devices, but eg. on Xperia phones there
> >   seem to be ro.semc.product.* with more userfriendly name, so this
> >   may vary upon devices. (eg ST18i vs Xperia Ray in model property).
> >   Or maybe just use ro.bluetooth.* and require integrator to set those
> >   accrodingly?
> 
> I think we should take ro.bluetooth.* and fallback to ro.product.*. If some
> manufactures want to do something different, they can either set
> ro.bluetooth.* or hack the code.
>
> Nexus devices should default to something useful. We have to take that as
> standard location for product information.
> 
> Since non of the DID details are mandatory, I prefer also that they are just
> not present if they are not configured in any property. Maybe BlueZ
> defaults for manufacturer and systemd id.

Sounds good.

> > - should we also use this instead of 'Mode' passed on service init?
> > 
> >   This would keep handling of all properities consistent but I'm fine
> >   if we decide to keep Modes special.
> 
> Do not understand the question. What do you mean.

I mean that we could pass all properties this way, including ones used to 
configure HALs (persist.sys.bluetooth.handsfree, persist.sys.bluetooth.mode) 
that currently are passed as "Mode" while registering service.

I also wonder if we could change our customization properties from 
persist.sys.bluetooth.* to ro.bluetooth.*. I'm not sure why we choose to use 
sys.persist.bluetooth in first place... do we really want this to be runtime 
configurable?

-- 
Szymon K. Janc
szymon.janc@gmail.com

Re: [RFC 0/6] android: Configuration command

[Marcel Holtmann]

Hi Szymon,

>>> This will allow to pass Android properties over IPC and use them
>>> in daemon for customization eg. Device Information or default adapter
>>> name.
>>> 
>>> Open points:
>>> - what properties should be read, currently I use ro.product.* as those
>>> 
>>>  are present on AOSP Nexus devices, but eg. on Xperia phones there
>>>  seem to be ro.semc.product.* with more userfriendly name, so this
>>>  may vary upon devices. (eg ST18i vs Xperia Ray in model property).
>>>  Or maybe just use ro.bluetooth.* and require integrator to set those
>>>  accrodingly?
>> 
>> I think we should take ro.bluetooth.* and fallback to ro.product.*. If some
>> manufactures want to do something different, they can either set
>> ro.bluetooth.* or hack the code.
>> 
>> Nexus devices should default to something useful. We have to take that as
>> standard location for product information.
>> 
>> Since non of the DID details are mandatory, I prefer also that they are just
>> not present if they are not configured in any property. Maybe BlueZ
>> defaults for manufacturer and systemd id.
> 
> Sounds good.
> 
>>> - should we also use this instead of 'Mode' passed on service init?
>>> 
>>>  This would keep handling of all properities consistent but I'm fine
>>>  if we decide to keep Modes special.
>> 
>> Do not understand the question. What do you mean.
> 
> I mean that we could pass all properties this way, including ones used to 
> configure HALs (persist.sys.bluetooth.handsfree, persist.sys.bluetooth.mode) 
> that currently are passed as "Mode" while registering service.
> 
> I also wonder if we could change our customization properties from 
> persist.sys.bluetooth.* to ro.bluetooth.*. I'm not sure why we choose to use 
> sys.persist.bluetooth in first place... do we really want this to be runtime 
> configurable?

I think we should use sys.persist.bluetooth first and if not present check ro.bluetooth as backup and if both are not present fallback to defaults. That gives us most flexibility.

Regards

Marcel