diff mbox series

[BlueZ,1/2] src/profile.c: Allow the "Address Type" to be set

Message ID 98fed7ef-1975-48ca-9ea3-1482d938a806@EXC04-ATKLA.omicron.at (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series Add support for LE profiles (LE-L2CAP) | expand

Commit Message

Mark Marshall Aug. 19, 2020, 3:09 p.m. UTC 
This allows us to have profiles that use LE L2CAP connections.
---
 doc/profile-api.txt | 10 ++++++++++
 src/profile.c       | 18 +++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

Comments

Luiz Augusto von Dentz Aug. 19, 2020, 6:46 p.m. UTC | #1 
Hi Mark,

On Wed, Aug 19, 2020 at 8:13 AM Mark Marshall
<mark.marshall@omicronenergy.com> wrote:
>
> This allows us to have profiles that use LE L2CAP connections.
> ---
>  doc/profile-api.txt | 10 ++++++++++
>  src/profile.c       | 18 +++++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/doc/profile-api.txt b/doc/profile-api.txt
> index 183c6c11a..8c7d0a06d 100644
> --- a/doc/profile-api.txt
> +++ b/doc/profile-api.txt
> @@ -112,6 +112,16 @@ Object path        /org/bluez
>
>                                         Profile features (for SDP record)
>
> +                               uint16 AddressType
> +
> +                                       Allows the Address Type to be
> +                                       selected, can be either
> +                                       BDADDR_BREDR, BDADDR_LE_PUBLIC
> +                                       or BDADDR_LE_RANDOM.  If an LE
> +                                       address is selected and the
> +                                       device is not found, the other
> +                                       sort of LE address is tried.

This sounds like a good idea but aren't we missing the GATT service
that actually exposes the PSM for the client to connect to? We should
probably make RegisterApplication accept Profile1 interfaces in such
cases so one can register both at the same time. Also note that this
should be consistent with Device.AddressType which uses a string to
not a uint16.

> +
>                         Possible errors: org.bluez.Error.InvalidArguments
>                                          org.bluez.Error.AlreadyExists
>
> diff --git a/src/profile.c b/src/profile.c
> index 6961a107b..10850f305 100644
> --- a/src/profile.c
> +++ b/src/profile.c
> @@ -677,6 +677,7 @@ struct ext_profile {
>         guint id;
>
>         BtIOMode mode;
> +       uint8_t addr_type;
>         BtIOSecLevel sec_level;
>         bool authorize;
>
> @@ -1173,9 +1174,16 @@ static struct ext_io *create_conn(struct ext_io *server, GIOChannel *io,
>         struct btd_service *service;
>         struct ext_io *conn;
>         GIOCondition cond;
> +       uint8_t addr_type;
>         char addr[18];
>
> -       device = btd_adapter_find_device(server->adapter, dst, BDADDR_BREDR);
> +       addr_type = server->ext->addr_type;
> +       device = btd_adapter_find_device(server->adapter, dst, addr_type);
> +       if (device == NULL && addr_type != BDADDR_BREDR) {
> +               addr_type ^= (BDADDR_LE_PUBLIC | BDADDR_LE_RANDOM);
> +               device = btd_adapter_find_device(server->adapter, dst,
> +                                                addr_type);
> +       }
>         if (device == NULL) {
>                 ba2str(dst, addr);
>                 error("%s device %s not found", server->ext->name, addr);
> @@ -1350,6 +1358,7 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
>                 io = bt_io_listen(connect, confirm, l2cap, NULL, &err,
>                                         BT_IO_OPT_SOURCE_BDADDR,
>                                         btd_adapter_get_address(adapter),
> +                                       BT_IO_OPT_SOURCE_TYPE, ext->addr_type,
>                                         BT_IO_OPT_MODE, ext->mode,
>                                         BT_IO_OPT_PSM, psm,
>                                         BT_IO_OPT_SEC_LEVEL, ext->sec_level,
> @@ -1567,6 +1576,8 @@ static int connect_io(struct ext_io *conn, const bdaddr_t *src,
>                 io = bt_io_connect(ext_connect, conn, NULL, &gerr,
>                                         BT_IO_OPT_SOURCE_BDADDR, src,
>                                         BT_IO_OPT_DEST_BDADDR, dst,
> +                                       BT_IO_OPT_SOURCE_TYPE, ext->addr_type,
> +                                       BT_IO_OPT_DEST_TYPE, ext->addr_type,
>                                         BT_IO_OPT_SEC_LEVEL, ext->sec_level,
>                                         BT_IO_OPT_PSM, conn->psm,
>                                         BT_IO_OPT_INVALID);
> @@ -2285,6 +2296,11 @@ static int parse_ext_opt(struct ext_profile *ext, const char *key,
>                 dbus_message_iter_get_basic(value, &str);
>                 free(ext->service);
>                 ext->service = bt_name2string(str);
> +       } else if (strcasecmp(key, "AddressType") == 0) {
> +               if (type != DBUS_TYPE_UINT16)
> +                       return -EINVAL;
> +               dbus_message_iter_get_basic(value, &u16);
> +               ext->addr_type = u16;
>         }
>
>         return 0;
> --
> 2.17.1
>
Mark Marshall Aug. 20, 2020, 11:50 a.m. UTC | #2 
Hi

On Wed, 2020-08-19 at 11:46 -0700, Luiz Augusto von Dentz wrote:
> Hi Mark,
> 
> On Wed, Aug 19, 2020 at 8:13 AM Mark Marshall
> <mark.marshall@omicronenergy.com> wrote:
> > 
> > This allows us to have profiles that use LE L2CAP connections.
> > ---
> >  doc/profile-api.txt | 10 ++++++++++
> >  src/profile.c       | 18 +++++++++++++++++-
> >  2 files changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/profile-api.txt b/doc/profile-api.txt
> > index 183c6c11a..8c7d0a06d 100644
> > --- a/doc/profile-api.txt
> > +++ b/doc/profile-api.txt
> > @@ -112,6 +112,16 @@ Object path        /org/bluez
> > 
> >                                         Profile features (for SDP record)
> > 
> > +                               uint16 AddressType
> > +
> > +                                       Allows the Address Type to be
> > +                                       selected, can be either
> > +                                       BDADDR_BREDR, BDADDR_LE_PUBLIC
> > +                                       or BDADDR_LE_RANDOM.  If an LE
> > +                                       address is selected and the
> > +                                       device is not found, the other
> > +                                       sort of LE address is tried.
> 
> This sounds like a good idea but aren't we missing the GATT service
> that actually exposes the PSM for the client to connect to? We should
> probably make RegisterApplication accept Profile1 interfaces in such
> cases so one can register both at the same time. Also note that this
> should be consistent with Device.AddressType which uses a string to
> not a uint16.

I didn't want to plumb this into the GATT stuff as it is not specified
how it would work.  I thought we would have to make up attribute UUIDs,
and I thought this was better left to user space?

I can use a string, yes, but I don't care about the difference between
public and private, just that it's LE, not br/edr.  I would treat
"public" as LE/public, "private" as LE/private and "value not present"
as BR/EDR?  Or should I make up a string for BR ("bredr"?)

> 
> > +
> >                         Possible errors: org.bluez.Error.InvalidArguments
> >                                          org.bluez.Error.AlreadyExists
> > 
> > diff --git a/src/profile.c b/src/profile.c
> > index 6961a107b..10850f305 100644
> > --- a/src/profile.c
> > +++ b/src/profile.c
> > @@ -677,6 +677,7 @@ struct ext_profile {
> >         guint id;
> > 
> >         BtIOMode mode;
> > +       uint8_t addr_type;
> >         BtIOSecLevel sec_level;
> >         bool authorize;
> > 
> > @@ -1173,9 +1174,16 @@ static struct ext_io *create_conn(struct ext_io *server, GIOChannel *io,
> >         struct btd_service *service;
> >         struct ext_io *conn;
> >         GIOCondition cond;
> > +       uint8_t addr_type;
> >         char addr[18];
> > 
> > -       device = btd_adapter_find_device(server->adapter, dst, BDADDR_BREDR);
> > +       addr_type = server->ext->addr_type;
> > +       device = btd_adapter_find_device(server->adapter, dst, addr_type);
> > +       if (device == NULL && addr_type != BDADDR_BREDR) {
> > +               addr_type ^= (BDADDR_LE_PUBLIC | BDADDR_LE_RANDOM);
> > +               device = btd_adapter_find_device(server->adapter, dst,
> > +                                                addr_type);
> > +       }
> >         if (device == NULL) {
> >                 ba2str(dst, addr);
> >                 error("%s device %s not found", server->ext->name, addr);
> > @@ -1350,6 +1358,7 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
> >                 io = bt_io_listen(connect, confirm, l2cap, NULL, &err,
> >                                         BT_IO_OPT_SOURCE_BDADDR,
> >                                         btd_adapter_get_address(adapter),
> > +                                       BT_IO_OPT_SOURCE_TYPE, ext->addr_type,
> >                                         BT_IO_OPT_MODE, ext->mode,
> >                                         BT_IO_OPT_PSM, psm,
> >                                         BT_IO_OPT_SEC_LEVEL, ext->sec_level,
> > @@ -1567,6 +1576,8 @@ static int connect_io(struct ext_io *conn, const bdaddr_t *src,
> >                 io = bt_io_connect(ext_connect, conn, NULL, &gerr,
> >                                         BT_IO_OPT_SOURCE_BDADDR, src,
> >                                         BT_IO_OPT_DEST_BDADDR, dst,
> > +                                       BT_IO_OPT_SOURCE_TYPE, ext->addr_type,
> > +                                       BT_IO_OPT_DEST_TYPE, ext->addr_type,
> >                                         BT_IO_OPT_SEC_LEVEL, ext->sec_level,
> >                                         BT_IO_OPT_PSM, conn->psm,
> >                                         BT_IO_OPT_INVALID);
> > @@ -2285,6 +2296,11 @@ static int parse_ext_opt(struct ext_profile *ext, const char *key,
> >                 dbus_message_iter_get_basic(value, &str);
> >                 free(ext->service);
> >                 ext->service = bt_name2string(str);
> > +       } else if (strcasecmp(key, "AddressType") == 0) {
> > +               if (type != DBUS_TYPE_UINT16)
> > +                       return -EINVAL;
> > +               dbus_message_iter_get_basic(value, &u16);
> > +               ext->addr_type = u16;
> >         }
> > 
> >         return 0;
> > --
> > 2.17.1
> > 
> 
>
diff mbox series

Patch

diff --git a/doc/profile-api.txt b/doc/profile-api.txt
index 183c6c11a..8c7d0a06d 100644
--- a/doc/profile-api.txt
+++ b/doc/profile-api.txt
@@ -112,6 +112,16 @@  Object path	/org/bluez
 
 					Profile features (for SDP record)
 
+				uint16 AddressType
+
+					Allows the Address Type to be
+					selected, can be either
+					BDADDR_BREDR, BDADDR_LE_PUBLIC
+					or BDADDR_LE_RANDOM.  If an LE
+					address is selected and the
+					device is not found, the other
+					sort of LE address is tried.
+
 			Possible errors: org.bluez.Error.InvalidArguments
 			                 org.bluez.Error.AlreadyExists
 
diff --git a/src/profile.c b/src/profile.c
index 6961a107b..10850f305 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -677,6 +677,7 @@  struct ext_profile {
 	guint id;
 
 	BtIOMode mode;
+	uint8_t addr_type;
 	BtIOSecLevel sec_level;
 	bool authorize;
 
@@ -1173,9 +1174,16 @@  static struct ext_io *create_conn(struct ext_io *server, GIOChannel *io,
 	struct btd_service *service;
 	struct ext_io *conn;
 	GIOCondition cond;
+	uint8_t addr_type;
 	char addr[18];
 
-	device = btd_adapter_find_device(server->adapter, dst, BDADDR_BREDR);
+	addr_type = server->ext->addr_type;
+	device = btd_adapter_find_device(server->adapter, dst, addr_type);
+	if (device == NULL && addr_type != BDADDR_BREDR) {
+		addr_type ^= (BDADDR_LE_PUBLIC | BDADDR_LE_RANDOM);
+		device = btd_adapter_find_device(server->adapter, dst,
+						 addr_type);
+	}
 	if (device == NULL) {
 		ba2str(dst, addr);
 		error("%s device %s not found", server->ext->name, addr);
@@ -1350,6 +1358,7 @@  static uint32_t ext_start_servers(struct ext_profile *ext,
 		io = bt_io_listen(connect, confirm, l2cap, NULL, &err,
 					BT_IO_OPT_SOURCE_BDADDR,
 					btd_adapter_get_address(adapter),
+					BT_IO_OPT_SOURCE_TYPE, ext->addr_type,
 					BT_IO_OPT_MODE, ext->mode,
 					BT_IO_OPT_PSM, psm,
 					BT_IO_OPT_SEC_LEVEL, ext->sec_level,
@@ -1567,6 +1576,8 @@  static int connect_io(struct ext_io *conn, const bdaddr_t *src,
 		io = bt_io_connect(ext_connect, conn, NULL, &gerr,
 					BT_IO_OPT_SOURCE_BDADDR, src,
 					BT_IO_OPT_DEST_BDADDR, dst,
+					BT_IO_OPT_SOURCE_TYPE, ext->addr_type,
+					BT_IO_OPT_DEST_TYPE, ext->addr_type,
 					BT_IO_OPT_SEC_LEVEL, ext->sec_level,
 					BT_IO_OPT_PSM, conn->psm,
 					BT_IO_OPT_INVALID);
@@ -2285,6 +2296,11 @@  static int parse_ext_opt(struct ext_profile *ext, const char *key,
 		dbus_message_iter_get_basic(value, &str);
 		free(ext->service);
 		ext->service = bt_name2string(str);
+	} else if (strcasecmp(key, "AddressType") == 0) {
+		if (type != DBUS_TYPE_UINT16)
+			return -EINVAL;
+		dbus_message_iter_get_basic(value, &u16);
+		ext->addr_type = u16;
 	}
 
 	return 0;