[RFC] adapter: Add CreateDevice method

[RFC] adapter: Add CreateDevice method

[Szymon Janc]

This allows to create Device1 object without discovery. This is needed for
some of qualification tests where there is no general discovery upfront
and we need to do connection to device with provided address.

Another usecase is for scenario where scanning happen on one controller but
connection handling on another.

Implementation wide this results in new temporary device object being created
that if unused will be cleanup just like it would be if found during discovery
session.

This patch implements bare minimum properties needed for connection - address
and address type. If needed eg. for non-NFC based OOB it could be extended with
more options.
---
 doc/adapter-api.txt | 29 ++++++++++++++++
 src/adapter.c       | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index 0533b674a..c8f3ce26e 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -145,6 +145,35 @@ Methods		void StartDiscovery()
 
 			Possible errors: None
 
+		void CreateDevice(dict properties) [experimental]
+
+			Creates new temporary device with defined properties.
+
+			Parameters that may be set in the filter dictionary
+			include the following:
+
+			string Address
+
+				The Bluetooth device address of the remote
+				device. This parameter is mandatory.
+
+			string AddressType
+
+				The Bluetooth device Address Type. This is
+				address type that should be used for initial
+				connection. If this parameter is not present
+				BR/EDR device is created.
+
+				Possible values:
+					"public" - Public address
+					"random" - Random address
+
+			Possible errors: org.bluez.Error.InvalidArguments
+					 org.bluez.Error.AlreadyExists
+					 org.bluez.Error.NotSupported
+					 org.bluez.Error.NotReady
+					 org.bluez.Error.Failed
+
 Properties	string Address [readonly]
 
 			The Bluetooth device address.
diff --git a/src/adapter.c b/src/adapter.c
index 0a25ae27e..0d18b58aa 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3080,6 +3080,99 @@ static DBusMessage *get_discovery_filters(DBusConnection *conn,
 	return reply;
 }
 
+static DBusMessage *create_device(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	DBusMessageIter iter, subiter, dictiter, value;
+	uint8_t addr_type = BDADDR_BREDR;
+	bdaddr_t addr = *BDADDR_ANY;
+	struct btd_device *device;
+
+	DBG("sender %s", dbus_message_get_sender(msg));
+
+	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+		return btd_error_not_ready(msg);
+
+	dbus_message_iter_init(msg, &iter);
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY ||
+	    dbus_message_iter_get_element_type(&iter) != DBUS_TYPE_DICT_ENTRY)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&iter, &subiter);
+	do {
+		int type = dbus_message_iter_get_arg_type(&subiter);
+		char *key;
+		char *str;
+
+		if (type == DBUS_TYPE_INVALID)
+			break;
+
+		dbus_message_iter_recurse(&subiter, &dictiter);
+
+		dbus_message_iter_get_basic(&dictiter, &key);
+		if (!dbus_message_iter_next(&dictiter))
+			return btd_error_invalid_args(msg);
+
+		if (dbus_message_iter_get_arg_type(&dictiter) !=
+							DBUS_TYPE_VARIANT)
+			return btd_error_invalid_args(msg);
+
+		dbus_message_iter_recurse(&dictiter, &value);
+
+		if (!strcmp(key, "Address")) {
+			if (dbus_message_iter_get_arg_type(&value) !=
+							DBUS_TYPE_STRING)
+				return btd_error_invalid_args(msg);
+
+			dbus_message_iter_get_basic(&value, &str);
+
+			if (str2ba(str, &addr) < 0 )
+				return btd_error_invalid_args(msg);
+		} else if (!strcmp(key, "AddressType")) {
+			if (dbus_message_iter_get_arg_type(&value) !=
+							DBUS_TYPE_STRING)
+				return btd_error_invalid_args(msg);
+
+			dbus_message_iter_get_basic(&value, &str);
+
+
+			if (!strcmp(str, "public"))
+				addr_type = BDADDR_LE_PUBLIC;
+			else if (!strcmp(str, "random"))
+				addr_type = BDADDR_LE_RANDOM;
+			else
+				return btd_error_invalid_args(msg);
+		} else {
+			return btd_error_invalid_args(msg);
+		}
+
+		dbus_message_iter_next(&subiter);
+	} while (true);
+
+	if (!bacmp(&addr, BDADDR_ANY))
+		return btd_error_invalid_args(msg);
+
+	device = btd_adapter_find_device(adapter, &addr, addr_type);
+	if (device)
+		return btd_error_already_exists(msg);
+
+
+	device = adapter_create_device(adapter, &addr, addr_type);
+	if (!device)
+		return btd_error_failed(msg, "Failed to create device");
+
+	device_update_last_seen(device, addr_type);
+	btd_device_set_temporary(device, true);
+
+	if (!adapter->temp_devices_timeout)
+		adapter->temp_devices_timeout = g_timeout_add_seconds(
+						TEMP_DEV_TIMEOUT,
+						remove_temp_devices, adapter);
+
+	return dbus_message_new_method_return(msg);
+}
+
 static const GDBusMethodTable adapter_methods[] = {
 	{ GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
 	{ GDBUS_METHOD("SetDiscoveryFilter",
@@ -3091,6 +3184,9 @@ static const GDBusMethodTable adapter_methods[] = {
 	{ GDBUS_METHOD("GetDiscoveryFilters", NULL,
 			GDBUS_ARGS({ "filters", "as" }),
 			get_discovery_filters) },
+	{ GDBUS_EXPERIMENTAL_METHOD("CreateDevice",
+				GDBUS_ARGS({ "properties", "a{sv}" }), NULL,
+				create_device) },
 	{ }
 };
 
-- 
2.14.3

Re: [RFC] adapter: Add CreateDevice method

[Szymon Janc]

Hi,

On Thursday, 11 January 2018 12:04:23 CET Szymon Janc wrote:
> This allows to create Device1 object without discovery. This is needed for
> some of qualification tests where there is no general discovery upfront
> and we need to do connection to device with provided address.
> 
> Another usecase is for scenario where scanning happen on one controller but
> connection handling on another.
> 
> Implementation wide this results in new temporary device object being
> created that if unused will be cleanup just like it would be if found
> during discovery session.
> 
> This patch implements bare minimum properties needed for connection -
> address and address type. If needed eg. for non-NFC based OOB it could be
> extended with more options.
> ---
>  doc/adapter-api.txt | 29 ++++++++++++++++
>  src/adapter.c       | 96
> +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 125
> insertions(+)
> 
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 0533b674a..c8f3ce26e 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
> 
>  			Possible errors: None
> 
> +		void CreateDevice(dict properties) [experimental]
> +
> +			Creates new temporary device with defined properties.
> +
> +			Parameters that may be set in the filter dictionary
> +			include the following:
> +
> +			string Address
> +
> +				The Bluetooth device address of the remote
> +				device. This parameter is mandatory.
> +
> +			string AddressType
> +
> +				The Bluetooth device Address Type. This is
> +				address type that should be used for initial
> +				connection. If this parameter is not present
> +				BR/EDR device is created.
> +
> +				Possible values:
> +					"public" - Public address
> +					"random" - Random address
> +
> +			Possible errors: org.bluez.Error.InvalidArguments
> +					 org.bluez.Error.AlreadyExists
> +					 org.bluez.Error.NotSupported
> +					 org.bluez.Error.NotReady
> +					 org.bluez.Error.Failed
> +
>  Properties	string Address [readonly]
> 
>  			The Bluetooth device address.
> diff --git a/src/adapter.c b/src/adapter.c
> index 0a25ae27e..0d18b58aa 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -3080,6 +3080,99 @@ static DBusMessage
> *get_discovery_filters(DBusConnection *conn, return reply;
>  }
> 
> +static DBusMessage *create_device(DBusConnection *conn,
> +					DBusMessage *msg, void *user_data)
> +{
> +	struct btd_adapter *adapter = user_data;
> +	DBusMessageIter iter, subiter, dictiter, value;
> +	uint8_t addr_type = BDADDR_BREDR;
> +	bdaddr_t addr = *BDADDR_ANY;
> +	struct btd_device *device;
> +
> +	DBG("sender %s", dbus_message_get_sender(msg));
> +
> +	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
> +		return btd_error_not_ready(msg);
> +
> +	dbus_message_iter_init(msg, &iter);
> +	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY ||
> +	    dbus_message_iter_get_element_type(&iter) != DBUS_TYPE_DICT_ENTRY)
> +		return btd_error_invalid_args(msg);
> +
> +	dbus_message_iter_recurse(&iter, &subiter);
> +	do {
> +		int type = dbus_message_iter_get_arg_type(&subiter);
> +		char *key;
> +		char *str;
> +
> +		if (type == DBUS_TYPE_INVALID)
> +			break;
> +
> +		dbus_message_iter_recurse(&subiter, &dictiter);
> +
> +		dbus_message_iter_get_basic(&dictiter, &key);
> +		if (!dbus_message_iter_next(&dictiter))
> +			return btd_error_invalid_args(msg);
> +
> +		if (dbus_message_iter_get_arg_type(&dictiter) !=
> +							DBUS_TYPE_VARIANT)
> +			return btd_error_invalid_args(msg);
> +
> +		dbus_message_iter_recurse(&dictiter, &value);
> +
> +		if (!strcmp(key, "Address")) {
> +			if (dbus_message_iter_get_arg_type(&value) !=
> +							DBUS_TYPE_STRING)
> +				return btd_error_invalid_args(msg);
> +
> +			dbus_message_iter_get_basic(&value, &str);
> +
> +			if (str2ba(str, &addr) < 0 )
> +				return btd_error_invalid_args(msg);
> +		} else if (!strcmp(key, "AddressType")) {
> +			if (dbus_message_iter_get_arg_type(&value) !=
> +							DBUS_TYPE_STRING)
> +				return btd_error_invalid_args(msg);
> +
> +			dbus_message_iter_get_basic(&value, &str);
> +
> +
> +			if (!strcmp(str, "public"))
> +				addr_type = BDADDR_LE_PUBLIC;
> +			else if (!strcmp(str, "random"))
> +				addr_type = BDADDR_LE_RANDOM;
> +			else
> +				return btd_error_invalid_args(msg);
> +		} else {
> +			return btd_error_invalid_args(msg);
> +		}
> +
> +		dbus_message_iter_next(&subiter);
> +	} while (true);
> +
> +	if (!bacmp(&addr, BDADDR_ANY))
> +		return btd_error_invalid_args(msg);
> +
> +	device = btd_adapter_find_device(adapter, &addr, addr_type);
> +	if (device)
> +		return btd_error_already_exists(msg);
> +
> +
> +	device = adapter_create_device(adapter, &addr, addr_type);
> +	if (!device)
> +		return btd_error_failed(msg, "Failed to create device");
> +
> +	device_update_last_seen(device, addr_type);
> +	btd_device_set_temporary(device, true);
> +
> +	if (!adapter->temp_devices_timeout)
> +		adapter->temp_devices_timeout = g_timeout_add_seconds(
> +						TEMP_DEV_TIMEOUT,
> +						remove_temp_devices, adapter);
> +
> +	return dbus_message_new_method_return(msg);
> +}
> +
>  static const GDBusMethodTable adapter_methods[] = {
>  	{ GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery) },
>  	{ GDBUS_METHOD("SetDiscoveryFilter",
> @@ -3091,6 +3184,9 @@ static const GDBusMethodTable adapter_methods[] = {
>  	{ GDBUS_METHOD("GetDiscoveryFilters", NULL,
>  			GDBUS_ARGS({ "filters", "as" }),
>  			get_discovery_filters) },
> +	{ GDBUS_EXPERIMENTAL_METHOD("CreateDevice",
> +				GDBUS_ARGS({ "properties", "a{sv}" }), NULL,
> +				create_device) },
>  	{ }
>  };

Any feedback on this?


-- 
pozdrawiam
Szymon Janc

Re: [RFC] adapter: Add CreateDevice method

[Grzegorz Kołodziejczyk]

Hi Szymon,

2018-01-16 15:15 GMT+01:00 Szymon Janc <szymon.janc@codecoup.pl>:
> Hi,
>
> On Thursday, 11 January 2018 12:04:23 CET Szymon Janc wrote:
>> This allows to create Device1 object without discovery. This is needed f=
or
>> some of qualification tests where there is no general discovery upfront
>> and we need to do connection to device with provided address.
>>
>> Another usecase is for scenario where scanning happen on one controller =
but
>> connection handling on another.
>>
>> Implementation wide this results in new temporary device object being
>> created that if unused will be cleanup just like it would be if found
>> during discovery session.
>>
>> This patch implements bare minimum properties needed for connection -
>> address and address type. If needed eg. for non-NFC based OOB it could b=
e
>> extended with more options.
>> ---
>>  doc/adapter-api.txt | 29 ++++++++++++++++
>>  src/adapter.c       | 96
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 1=
25
>> insertions(+)
>>
>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>> index 0533b674a..c8f3ce26e 100644
>> --- a/doc/adapter-api.txt
>> +++ b/doc/adapter-api.txt
>> @@ -145,6 +145,35 @@ Methods          void StartDiscovery()
>>
>>                       Possible errors: None
>>
>> +             void CreateDevice(dict properties) [experimental]
>> +
>> +                     Creates new temporary device with defined properti=
es.
>> +
>> +                     Parameters that may be set in the filter dictionar=
y
>> +                     include the following:
>> +
>> +                     string Address
>> +
>> +                             The Bluetooth device address of the remote
>> +                             device. This parameter is mandatory.
>> +
>> +                     string AddressType
>> +
>> +                             The Bluetooth device Address Type. This is
>> +                             address type that should be used for initi=
al
>> +                             connection. If this parameter is not prese=
nt
>> +                             BR/EDR device is created.
>> +
>> +                             Possible values:
>> +                                     "public" - Public address
>> +                                     "random" - Random address
>> +
>> +                     Possible errors: org.bluez.Error.InvalidArguments
>> +                                      org.bluez.Error.AlreadyExists
>> +                                      org.bluez.Error.NotSupported
>> +                                      org.bluez.Error.NotReady
>> +                                      org.bluez.Error.Failed
>> +
>>  Properties   string Address [readonly]
>>
>>                       The Bluetooth device address.
>> diff --git a/src/adapter.c b/src/adapter.c
>> index 0a25ae27e..0d18b58aa 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -3080,6 +3080,99 @@ static DBusMessage
>> *get_discovery_filters(DBusConnection *conn, return reply;
>>  }
>>
>> +static DBusMessage *create_device(DBusConnection *conn,
>> +                                     DBusMessage *msg, void *user_data)
>> +{
>> +     struct btd_adapter *adapter =3D user_data;
>> +     DBusMessageIter iter, subiter, dictiter, value;
>> +     uint8_t addr_type =3D BDADDR_BREDR;
>> +     bdaddr_t addr =3D *BDADDR_ANY;
>> +     struct btd_device *device;
>> +
>> +     DBG("sender %s", dbus_message_get_sender(msg));
>> +
>> +     if (!(adapter->current_settings & MGMT_SETTING_POWERED))
>> +             return btd_error_not_ready(msg);
>> +
>> +     dbus_message_iter_init(msg, &iter);
>> +     if (dbus_message_iter_get_arg_type(&iter) !=3D DBUS_TYPE_ARRAY ||
>> +         dbus_message_iter_get_element_type(&iter) !=3D DBUS_TYPE_DICT_=
ENTRY)
>> +             return btd_error_invalid_args(msg);
>> +
>> +     dbus_message_iter_recurse(&iter, &subiter);
>> +     do {
>> +             int type =3D dbus_message_iter_get_arg_type(&subiter);
>> +             char *key;
>> +             char *str;
>> +
>> +             if (type =3D=3D DBUS_TYPE_INVALID)
>> +                     break;
>> +
>> +             dbus_message_iter_recurse(&subiter, &dictiter);
>> +
>> +             dbus_message_iter_get_basic(&dictiter, &key);
>> +             if (!dbus_message_iter_next(&dictiter))
>> +                     return btd_error_invalid_args(msg);
>> +
>> +             if (dbus_message_iter_get_arg_type(&dictiter) !=3D
>> +                                                     DBUS_TYPE_VARIANT)
>> +                     return btd_error_invalid_args(msg);
>> +
>> +             dbus_message_iter_recurse(&dictiter, &value);
>> +
>> +             if (!strcmp(key, "Address")) {
>> +                     if (dbus_message_iter_get_arg_type(&value) !=3D
>> +                                                     DBUS_TYPE_STRING)
>> +                             return btd_error_invalid_args(msg);
>> +
>> +                     dbus_message_iter_get_basic(&value, &str);
>> +
>> +                     if (str2ba(str, &addr) < 0 )
>> +                             return btd_error_invalid_args(msg);
>> +             } else if (!strcmp(key, "AddressType")) {
>> +                     if (dbus_message_iter_get_arg_type(&value) !=3D
>> +                                                     DBUS_TYPE_STRING)
>> +                             return btd_error_invalid_args(msg);
>> +
>> +                     dbus_message_iter_get_basic(&value, &str);
>> +
>> +
>> +                     if (!strcmp(str, "public"))
>> +                             addr_type =3D BDADDR_LE_PUBLIC;
>> +                     else if (!strcmp(str, "random"))
>> +                             addr_type =3D BDADDR_LE_RANDOM;
>> +                     else
>> +                             return btd_error_invalid_args(msg);
>> +             } else {
>> +                     return btd_error_invalid_args(msg);
>> +             }
>> +
>> +             dbus_message_iter_next(&subiter);
>> +     } while (true);
>> +
>> +     if (!bacmp(&addr, BDADDR_ANY))
>> +             return btd_error_invalid_args(msg);
>> +
>> +     device =3D btd_adapter_find_device(adapter, &addr, addr_type);
>> +     if (device)
>> +             return btd_error_already_exists(msg);
>> +
>> +
>> +     device =3D adapter_create_device(adapter, &addr, addr_type);
>> +     if (!device)
>> +             return btd_error_failed(msg, "Failed to create device");
>> +
>> +     device_update_last_seen(device, addr_type);
>> +     btd_device_set_temporary(device, true);
>> +
>> +     if (!adapter->temp_devices_timeout)
>> +             adapter->temp_devices_timeout =3D g_timeout_add_seconds(
>> +                                             TEMP_DEV_TIMEOUT,
>> +                                             remove_temp_devices, adapt=
er);
>> +
>> +     return dbus_message_new_method_return(msg);
>> +}
>> +
>>  static const GDBusMethodTable adapter_methods[] =3D {
>>       { GDBUS_ASYNC_METHOD("StartDiscovery", NULL, NULL, start_discovery=
) },
>>       { GDBUS_METHOD("SetDiscoveryFilter",
>> @@ -3091,6 +3184,9 @@ static const GDBusMethodTable adapter_methods[] =
=3D {
>>       { GDBUS_METHOD("GetDiscoveryFilters", NULL,
>>                       GDBUS_ARGS({ "filters", "as" }),
>>                       get_discovery_filters) },
>> +     { GDBUS_EXPERIMENTAL_METHOD("CreateDevice",
>> +                             GDBUS_ARGS({ "properties", "a{sv}" }), NUL=
L,
>> +                             create_device) },
>>       { }
>>  };
>
> Any feedback on this?
I've used it internally in btpclient and it works well. +1 from me.
>
>
> --
> pozdrawiam
> Szymon Janc
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

pozdrawiam,
Grzegorz Ko=C5=82odziejczyk

Re: [RFC] adapter: Add CreateDevice method

[Marcel Holtmann]

Hi Szymon,

> This allows to create Device1 object without discovery. This is needed for
> some of qualification tests where there is no general discovery upfront
> and we need to do connection to device with provided address.
> 
> Another usecase is for scenario where scanning happen on one controller but
> connection handling on another.
> 
> Implementation wide this results in new temporary device object being created
> that if unused will be cleanup just like it would be if found during discovery
> session.

so what are the rules around the cleanup? On next discovery it is gone again, then that might needs to be mentioned.

> 
> This patch implements bare minimum properties needed for connection - address
> and address type. If needed eg. for non-NFC based OOB it could be extended with
> more options.
> ---
> doc/adapter-api.txt | 29 ++++++++++++++++
> src/adapter.c       | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 125 insertions(+)
> 
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 0533b674a..c8f3ce26e 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
> 
> 			Possible errors: None
> 
> +		void CreateDevice(dict properties) [experimental]
> +
> +			Creates new temporary device with defined properties.

Why is this not returning the device object path that gets created? Seems a waste time cycles to wait for a device showing up later.

> +
> +			Parameters that may be set in the filter dictionary
> +			include the following:
> +
> +			string Address
> +
> +				The Bluetooth device address of the remote
> +				device. This parameter is mandatory.
> +
> +			string AddressType
> +
> +				The Bluetooth device Address Type. This is
> +				address type that should be used for initial
> +				connection. If this parameter is not present
> +				BR/EDR device is created.
> +
> +				Possible values:
> +					"public" - Public address
> +					"random" - Random address
> +
> +			Possible errors: org.bluez.Error.InvalidArguments
> +					 org.bluez.Error.AlreadyExists
> +					 org.bluez.Error.NotSupported
> +					 org.bluez.Error.NotReady
> +					 org.bluez.Error.Failed
> +

Now what I wonder is that in case of LE, this should actually trigger a quick scan for that device so that you get advertising data and other information about the device. We are missing a kernel API for such a targeted case and that might need fixing as well.

In case of BR/EDR, this might should trigger a connection and SDP discovery.

Otherwise this is not an API and just a hack to create a device path object. So you are essentially just doing an “InjectDevice” instead of anything real.

That said, using a DiscoverDevice() method name might be more accurate.

Regards

Marcel

Re: [RFC] adapter: Add CreateDevice method

[Szymon Janc]

Hi Marcel,

On Thursday, 18 January 2018 10:40:04 CET Marcel Holtmann wrote:
> Hi Szymon,
>=20
> > This allows to create Device1 object without discovery. This is needed =
for
> > some of qualification tests where there is no general discovery upfront
> > and we need to do connection to device with provided address.
> >=20
> > Another usecase is for scenario where scanning happen on one controller
> > but
> > connection handling on another.
> >=20
> > Implementation wide this results in new temporary device object being
> > created that if unused will be cleanup just like it would be if found
> > during discovery session.
>=20
> so what are the rules around the cleanup? On next discovery it is gone
> again, then that might needs to be mentioned.

Those are same rules so it will be gone only if it is left temporary (ie=20
Connect() or Pair() was never called). I'll document that.

> > This patch implements bare minimum properties needed for connection -
> > address and address type. If needed eg. for non-NFC based OOB it could =
be
> > extended with more options.
> > ---
> > doc/adapter-api.txt | 29 ++++++++++++++++
> > src/adapter.c       | 96
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
> > 125 insertions(+)
> >=20
> > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > index 0533b674a..c8f3ce26e 100644
> > --- a/doc/adapter-api.txt
> > +++ b/doc/adapter-api.txt
> > @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
> >=20
> > 			Possible errors: None
> >=20
> > +		void CreateDevice(dict properties) [experimental]
> > +
> > +			Creates new temporary device with defined properties.
>=20
> Why is this not returning the device object path that gets created? Seems=
 a
> waste time cycles to wait for a device showing up later.

I was thinking about that too. I'll make it return object path.

> > +
> > +			Parameters that may be set in the filter dictionary
> > +			include the following:
> > +
> > +			string Address
> > +
> > +				The Bluetooth device address of the remote
> > +				device. This parameter is mandatory.
> > +
> > +			string AddressType
> > +
> > +				The Bluetooth device Address Type. This is
> > +				address type that should be used for initial
> > +				connection. If this parameter is not present
> > +				BR/EDR device is created.
> > +
> > +				Possible values:
> > +					"public" - Public address
> > +					"random" - Random address
> > +
> > +			Possible errors: org.bluez.Error.InvalidArguments
> > +					 org.bluez.Error.AlreadyExists
> > +					 org.bluez.Error.NotSupported
> > +					 org.bluez.Error.NotReady
> > +					 org.bluez.Error.Failed
> > +
>=20
> Now what I wonder is that in case of LE, this should actually trigger a
> quick scan for that device so that you get advertising data and other
> information about the device. We are missing a kernel API for such a
> targeted case and that might need fixing as well.
>=20
> In case of BR/EDR, this might should trigger a connection and SDP discove=
ry.
>=20
> Otherwise this is not an API and just a hack to create a device path obje=
ct.
> So you are essentially just doing an =E2=80=9CInjectDevice=E2=80=9D inste=
ad of anything
> real.

Maybe we should just implicitly make it connect to device? That would keep=
=20
things simple and wouldn't require any additional kernel work. If connect=20
failed we remove device.

How does it sound?

> That said, using a DiscoverDevice() method name might be more accurate.
>=20
> Regards
>=20
> Marcel


=2D-=20
pozdrawiam
Szymon Janc

Re: [RFC] adapter: Add CreateDevice method

[Marcel Holtmann]

Hi Szymon,

>>> This allows to create Device1 object without discovery. This is needed for
>>> some of qualification tests where there is no general discovery upfront
>>> and we need to do connection to device with provided address.
>>> 
>>> Another usecase is for scenario where scanning happen on one controller
>>> but
>>> connection handling on another.
>>> 
>>> Implementation wide this results in new temporary device object being
>>> created that if unused will be cleanup just like it would be if found
>>> during discovery session.
>> 
>> so what are the rules around the cleanup? On next discovery it is gone
>> again, then that might needs to be mentioned.
> 
> Those are same rules so it will be gone only if it is left temporary (ie 
> Connect() or Pair() was never called). I'll document that.
> 
>>> This patch implements bare minimum properties needed for connection -
>>> address and address type. If needed eg. for non-NFC based OOB it could be
>>> extended with more options.
>>> ---
>>> doc/adapter-api.txt | 29 ++++++++++++++++
>>> src/adapter.c       | 96
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
>>> 125 insertions(+)
>>> 
>>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>>> index 0533b674a..c8f3ce26e 100644
>>> --- a/doc/adapter-api.txt
>>> +++ b/doc/adapter-api.txt
>>> @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
>>> 
>>> 			Possible errors: None
>>> 
>>> +		void CreateDevice(dict properties) [experimental]
>>> +
>>> +			Creates new temporary device with defined properties.
>> 
>> Why is this not returning the device object path that gets created? Seems a
>> waste time cycles to wait for a device showing up later.
> 
> I was thinking about that too. I'll make it return object path.
> 
>>> +
>>> +			Parameters that may be set in the filter dictionary
>>> +			include the following:
>>> +
>>> +			string Address
>>> +
>>> +				The Bluetooth device address of the remote
>>> +				device. This parameter is mandatory.
>>> +
>>> +			string AddressType
>>> +
>>> +				The Bluetooth device Address Type. This is
>>> +				address type that should be used for initial
>>> +				connection. If this parameter is not present
>>> +				BR/EDR device is created.
>>> +
>>> +				Possible values:
>>> +					"public" - Public address
>>> +					"random" - Random address
>>> +
>>> +			Possible errors: org.bluez.Error.InvalidArguments
>>> +					 org.bluez.Error.AlreadyExists
>>> +					 org.bluez.Error.NotSupported
>>> +					 org.bluez.Error.NotReady
>>> +					 org.bluez.Error.Failed
>>> +
>> 
>> Now what I wonder is that in case of LE, this should actually trigger a
>> quick scan for that device so that you get advertising data and other
>> information about the device. We are missing a kernel API for such a
>> targeted case and that might need fixing as well.
>> 
>> In case of BR/EDR, this might should trigger a connection and SDP discovery.
>> 
>> Otherwise this is not an API and just a hack to create a device path object.
>> So you are essentially just doing an “InjectDevice” instead of anything
>> real.
> 
> Maybe we should just implicitly make it connect to device? That would keep 
> things simple and wouldn't require any additional kernel work. If connect 
> failed we remove device.

that would work as well and since for LE we always scan first, we get the advertising data as well. Now the question is if just call it DiscoverDevice instead. I am reluctant to call it ConnectDevice since that is a bit overlapping with Device.Connect. And CreateDevice is bad as well since this is temporary connection in most cases.

Regards

Marcel

Re: [RFC] adapter: Add CreateDevice method

[Marcel Holtmann]

Hi Szymon,

>>>> This allows to create Device1 object without discovery. This is needed for
>>>> some of qualification tests where there is no general discovery upfront
>>>> and we need to do connection to device with provided address.
>>>> 
>>>> Another usecase is for scenario where scanning happen on one controller
>>>> but
>>>> connection handling on another.
>>>> 
>>>> Implementation wide this results in new temporary device object being
>>>> created that if unused will be cleanup just like it would be if found
>>>> during discovery session.
>>> 
>>> so what are the rules around the cleanup? On next discovery it is gone
>>> again, then that might needs to be mentioned.
>> 
>> Those are same rules so it will be gone only if it is left temporary (ie 
>> Connect() or Pair() was never called). I'll document that.
>> 
>>>> This patch implements bare minimum properties needed for connection -
>>>> address and address type. If needed eg. for non-NFC based OOB it could be
>>>> extended with more options.
>>>> ---
>>>> doc/adapter-api.txt | 29 ++++++++++++++++
>>>> src/adapter.c       | 96
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
>>>> 125 insertions(+)
>>>> 
>>>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>>>> index 0533b674a..c8f3ce26e 100644
>>>> --- a/doc/adapter-api.txt
>>>> +++ b/doc/adapter-api.txt
>>>> @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
>>>> 
>>>> 			Possible errors: None
>>>> 
>>>> +		void CreateDevice(dict properties) [experimental]
>>>> +
>>>> +			Creates new temporary device with defined properties.
>>> 
>>> Why is this not returning the device object path that gets created? Seems a
>>> waste time cycles to wait for a device showing up later.
>> 
>> I was thinking about that too. I'll make it return object path.
>> 
>>>> +
>>>> +			Parameters that may be set in the filter dictionary
>>>> +			include the following:
>>>> +
>>>> +			string Address
>>>> +
>>>> +				The Bluetooth device address of the remote
>>>> +				device. This parameter is mandatory.
>>>> +
>>>> +			string AddressType
>>>> +
>>>> +				The Bluetooth device Address Type. This is
>>>> +				address type that should be used for initial
>>>> +				connection. If this parameter is not present
>>>> +				BR/EDR device is created.
>>>> +
>>>> +				Possible values:
>>>> +					"public" - Public address
>>>> +					"random" - Random address
>>>> +
>>>> +			Possible errors: org.bluez.Error.InvalidArguments
>>>> +					 org.bluez.Error.AlreadyExists
>>>> +					 org.bluez.Error.NotSupported
>>>> +					 org.bluez.Error.NotReady
>>>> +					 org.bluez.Error.Failed
>>>> +
>>> 
>>> Now what I wonder is that in case of LE, this should actually trigger a
>>> quick scan for that device so that you get advertising data and other
>>> information about the device. We are missing a kernel API for such a
>>> targeted case and that might need fixing as well.
>>> 
>>> In case of BR/EDR, this might should trigger a connection and SDP discovery.
>>> 
>>> Otherwise this is not an API and just a hack to create a device path object.
>>> So you are essentially just doing an “InjectDevice” instead of anything
>>> real.
>> 
>> Maybe we should just implicitly make it connect to device? That would keep 
>> things simple and wouldn't require any additional kernel work. If connect 
>> failed we remove device.
> 
> that would work as well and since for LE we always scan first, we get the advertising data as well. Now the question is if just call it DiscoverDevice instead. I am reluctant to call it ConnectDevice since that is a bit overlapping with Device.Connect. And CreateDevice is bad as well since this is temporary connection in most cases.

actually just trying to connect really only works for BR/EDR. With LE that would not help us with non-connectable devices. So I am leaning towards DiscoverDevice that does BR/EDR connect and SDP. And on LE it does an active scan for that specific device. No connection attempt required on LE.

Regards

Marcel

Re: [RFC] adapter: Add CreateDevice method

[Szymon Janc]

Hi Marcel,

On Thursday, 18 January 2018 11:20:03 CET Marcel Holtmann wrote:
> Hi Szymon,
>=20
> >>>> This allows to create Device1 object without discovery. This is need=
ed
> >>>> for
> >>>> some of qualification tests where there is no general discovery upfr=
ont
> >>>> and we need to do connection to device with provided address.
> >>>>=20
> >>>> Another usecase is for scenario where scanning happen on one control=
ler
> >>>> but
> >>>> connection handling on another.
> >>>>=20
> >>>> Implementation wide this results in new temporary device object being
> >>>> created that if unused will be cleanup just like it would be if found
> >>>> during discovery session.
> >>>=20
> >>> so what are the rules around the cleanup? On next discovery it is gone
> >>> again, then that might needs to be mentioned.
> >>=20
> >> Those are same rules so it will be gone only if it is left temporary (=
ie
> >> Connect() or Pair() was never called). I'll document that.
> >>=20
> >>>> This patch implements bare minimum properties needed for connection -
> >>>> address and address type. If needed eg. for non-NFC based OOB it cou=
ld
> >>>> be
> >>>> extended with more options.
> >>>> ---
> >>>> doc/adapter-api.txt | 29 ++++++++++++++++
> >>>> src/adapter.c       | 96
> >>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files change=
d,
> >>>> 125 insertions(+)
> >>>>=20
> >>>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> >>>> index 0533b674a..c8f3ce26e 100644
> >>>> --- a/doc/adapter-api.txt
> >>>> +++ b/doc/adapter-api.txt
> >>>> @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
> >>>>=20
> >>>> 			Possible errors: None
> >>>>=20
> >>>> +		void CreateDevice(dict properties) [experimental]
> >>>> +
> >>>> +			Creates new temporary device with defined properties.
> >>>=20
> >>> Why is this not returning the device object path that gets created?
> >>> Seems a
> >>> waste time cycles to wait for a device showing up later.
> >>=20
> >> I was thinking about that too. I'll make it return object path.
> >>=20
> >>>> +
> >>>> +			Parameters that may be set in the filter dictionary
> >>>> +			include the following:
> >>>> +
> >>>> +			string Address
> >>>> +
> >>>> +				The Bluetooth device address of the remote
> >>>> +				device. This parameter is mandatory.
> >>>> +
> >>>> +			string AddressType
> >>>> +
> >>>> +				The Bluetooth device Address Type. This is
> >>>> +				address type that should be used for initial
> >>>> +				connection. If this parameter is not present
> >>>> +				BR/EDR device is created.
> >>>> +
> >>>> +				Possible values:
> >>>> +					"public" - Public address
> >>>> +					"random" - Random address
> >>>> +
> >>>> +			Possible errors: org.bluez.Error.InvalidArguments
> >>>> +					 org.bluez.Error.AlreadyExists
> >>>> +					 org.bluez.Error.NotSupported
> >>>> +					 org.bluez.Error.NotReady
> >>>> +					 org.bluez.Error.Failed
> >>>> +
> >>>=20
> >>> Now what I wonder is that in case of LE, this should actually trigger=
 a
> >>> quick scan for that device so that you get advertising data and other
> >>> information about the device. We are missing a kernel API for such a
> >>> targeted case and that might need fixing as well.
> >>>=20
> >>> In case of BR/EDR, this might should trigger a connection and SDP
> >>> discovery.
> >>>=20
> >>> Otherwise this is not an API and just a hack to create a device path
> >>> object. So you are essentially just doing an =E2=80=9CInjectDevice=E2=
=80=9D instead of
> >>> anything real.
> >>=20
> >> Maybe we should just implicitly make it connect to device? That would
> >> keep
> >> things simple and wouldn't require any additional kernel work. If conn=
ect
> >> failed we remove device.
> >=20
> > that would work as well and since for LE we always scan first, we get t=
he
> > advertising data as well. Now the question is if just call it
> > DiscoverDevice instead. I am reluctant to call it ConnectDevice since
> > that is a bit overlapping with Device.Connect. And CreateDevice is bad =
as
> > well since this is temporary connection in most cases.
> actually just trying to connect really only works for BR/EDR. With LE that
> would not help us with non-connectable devices. So I am leaning towards
> DiscoverDevice that does BR/EDR connect and SDP. And on LE it does an
> active scan for that specific device. No connection attempt required on L=
E.

This is explicitly meant for connectable devices so I'd now bother with non-
connectable devices as those are by definition discovered with observation=
=20
procedure.=20

I'm now thinking on method name (was going initially with ConnectDevice but=
=20
you already mentioned that you don't like it:P)

=2D-=20
pozdrawiam
Szymon Janc

Re: [RFC] adapter: Add CreateDevice method

[Marcel Holtmann]

Hi Szymon,

>>>>>> This allows to create Device1 object without discovery. This is needed
>>>>>> for
>>>>>> some of qualification tests where there is no general discovery upfront
>>>>>> and we need to do connection to device with provided address.
>>>>>> 
>>>>>> Another usecase is for scenario where scanning happen on one controller
>>>>>> but
>>>>>> connection handling on another.
>>>>>> 
>>>>>> Implementation wide this results in new temporary device object being
>>>>>> created that if unused will be cleanup just like it would be if found
>>>>>> during discovery session.
>>>>> 
>>>>> so what are the rules around the cleanup? On next discovery it is gone
>>>>> again, then that might needs to be mentioned.
>>>> 
>>>> Those are same rules so it will be gone only if it is left temporary (ie
>>>> Connect() or Pair() was never called). I'll document that.
>>>> 
>>>>>> This patch implements bare minimum properties needed for connection -
>>>>>> address and address type. If needed eg. for non-NFC based OOB it could
>>>>>> be
>>>>>> extended with more options.
>>>>>> ---
>>>>>> doc/adapter-api.txt | 29 ++++++++++++++++
>>>>>> src/adapter.c       | 96
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
>>>>>> 125 insertions(+)
>>>>>> 
>>>>>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>>>>>> index 0533b674a..c8f3ce26e 100644
>>>>>> --- a/doc/adapter-api.txt
>>>>>> +++ b/doc/adapter-api.txt
>>>>>> @@ -145,6 +145,35 @@ Methods		void StartDiscovery()
>>>>>> 
>>>>>> 			Possible errors: None
>>>>>> 
>>>>>> +		void CreateDevice(dict properties) [experimental]
>>>>>> +
>>>>>> +			Creates new temporary device with defined properties.
>>>>> 
>>>>> Why is this not returning the device object path that gets created?
>>>>> Seems a
>>>>> waste time cycles to wait for a device showing up later.
>>>> 
>>>> I was thinking about that too. I'll make it return object path.
>>>> 
>>>>>> +
>>>>>> +			Parameters that may be set in the filter dictionary
>>>>>> +			include the following:
>>>>>> +
>>>>>> +			string Address
>>>>>> +
>>>>>> +				The Bluetooth device address of the remote
>>>>>> +				device. This parameter is mandatory.
>>>>>> +
>>>>>> +			string AddressType
>>>>>> +
>>>>>> +				The Bluetooth device Address Type. This is
>>>>>> +				address type that should be used for initial
>>>>>> +				connection. If this parameter is not present
>>>>>> +				BR/EDR device is created.
>>>>>> +
>>>>>> +				Possible values:
>>>>>> +					"public" - Public address
>>>>>> +					"random" - Random address
>>>>>> +
>>>>>> +			Possible errors: org.bluez.Error.InvalidArguments
>>>>>> +					 org.bluez.Error.AlreadyExists
>>>>>> +					 org.bluez.Error.NotSupported
>>>>>> +					 org.bluez.Error.NotReady
>>>>>> +					 org.bluez.Error.Failed
>>>>>> +
>>>>> 
>>>>> Now what I wonder is that in case of LE, this should actually trigger a
>>>>> quick scan for that device so that you get advertising data and other
>>>>> information about the device. We are missing a kernel API for such a
>>>>> targeted case and that might need fixing as well.
>>>>> 
>>>>> In case of BR/EDR, this might should trigger a connection and SDP
>>>>> discovery.
>>>>> 
>>>>> Otherwise this is not an API and just a hack to create a device path
>>>>> object. So you are essentially just doing an “InjectDevice” instead of
>>>>> anything real.
>>>> 
>>>> Maybe we should just implicitly make it connect to device? That would
>>>> keep
>>>> things simple and wouldn't require any additional kernel work. If connect
>>>> failed we remove device.
>>> 
>>> that would work as well and since for LE we always scan first, we get the
>>> advertising data as well. Now the question is if just call it
>>> DiscoverDevice instead. I am reluctant to call it ConnectDevice since
>>> that is a bit overlapping with Device.Connect. And CreateDevice is bad as
>>> well since this is temporary connection in most cases.
>> actually just trying to connect really only works for BR/EDR. With LE that
>> would not help us with non-connectable devices. So I am leaning towards
>> DiscoverDevice that does BR/EDR connect and SDP. And on LE it does an
>> active scan for that specific device. No connection attempt required on LE.
> 
> This is explicitly meant for connectable devices so I'd now bother with non-
> connectable devices as those are by definition discovered with observation 
> procedure. 
> 
> I'm now thinking on method name (was going initially with ConnectDevice but 
> you already mentioned that you don't like it:P)

maybe ConnectDevice() is fine if it behaves _exactly_ like Device.Connect() does. And of course the relation is clearly stated in the documentation.

Regards

Marcel