2011-03-30 18:33:55

by Andre Dieb Martins

[permalink] [raw]
Subject: [RFC 1/2] Add Type property to Device

---
doc/device-api.txt | 4 ++++
src/device.c | 18 ++++++++++++++++++
src/device.h | 2 ++
3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/doc/device-api.txt b/doc/device-api.txt
index d1feb18..a667296 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -130,6 +130,10 @@ Properties string Address [readonly]
Proposed icon name according to the freedesktop.org
icon naming specification.

+ string Type [readonly]
+
+ Device type (BR/EDR, LE, BR/EDR/LE).
+
uint32 Class [readonly]

The Bluetooth class of device of the remote device.
diff --git a/src/device.c b/src/device.c
index d567952..96683de 100644
--- a/src/device.c
+++ b/src/device.c
@@ -228,6 +228,20 @@ static void device_free(gpointer user_data)
g_free(device);
}

+const char *devtype2str(device_type_t type)
+{
+ switch (type) {
+ case DEVICE_TYPE_BREDR:
+ return "BR/EDR";
+ case DEVICE_TYPE_LE:
+ return "LE";
+ case DEVICE_TYPE_DUALMODE:
+ return "BR/EDR/LE";
+ default:
+ return "Unknown";
+ }
+}
+
gboolean device_is_paired(struct btd_device *device)
{
return device->paired;
@@ -302,6 +316,10 @@ static DBusMessage *get_properties(DBusConnection *conn,
DBUS_TYPE_STRING, &icon);
}

+ /* Type */
+ ptr = devtype2str(device->type);
+ dict_append_entry(&dict, "Type", DBUS_TYPE_STRING, &ptr);
+
/* Paired */
boolean = device_is_paired(device);
dict_append_entry(&dict, "Paired", DBUS_TYPE_BOOLEAN, &boolean);
diff --git a/src/device.h b/src/device.h
index 3ce212b..b385070 100644
--- a/src/device.h
+++ b/src/device.h
@@ -41,6 +41,8 @@ typedef enum {
DEVICE_TYPE_DUALMODE
} device_type_t;

+const char *devtype2str(device_type_t type);
+
struct btd_device *device_create(DBusConnection *conn,
struct btd_adapter *adapter,
const gchar *address, device_type_t type);
--
1.7.1



2011-03-31 20:44:22

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [RFC 1/2] Add Type property to Device

Hi Marcel/Johan/Luiz,

We need opinions for this change...

Why this information is needed: Appearance characteristic is not
available in the advertising. Currently, BlueZ implements interleaved
discovery: 5.12sec inquiring + 5.12sec scanning.
Found devices are reported through the DeviceFound() signal. The user
space needs to check the "Broadcaster" property inside the DeviceFound
signal to check if the device is Basic Rate or LE, which is not
acceptable(in my opinion). We need a meaningful property to
distinguish LE devices.

Before we push these patches upstream we need to define which
information it exposes: Operational Mode or host/controller
capability?

My opinion:
1. for the Adapter it should represent the operational mode, a
controller can support LE, but the host can disable it. Setting
EnableLE =3D false(main.conf) should "disable" LE functionalities.
2. for DeviceFound(), the UI needs this info only to distinguish LE
devices. In my opinion UUIDs and Broadcaster properties are not
enough/appropriated for this purpose. "Qualified" devices will not
enable inquiry scan and advertising at same time. Our main target is
to connect to LE only devices. "BR/EDR/LE" can be confusing in this
context. Suggestions here?
3. for the Device, I am not sure if we need to expose this information
to the users, appearance characteristic is mandatory, if a Device
"object" exists in the system the appearance value will be
available(it will be retrieved after discovery primary services)

Comments?

Andre:
You need to check the adapter features, old adapters doesn't support EDR.

Claudio.

On Wed, Mar 30, 2011 at 6:33 PM, Andre Dieb Martins
<[email protected]> wrote:
> ---
> =C2=A0doc/device-api.txt | =C2=A0 =C2=A04 ++++
> =C2=A0src/device.c =C2=A0 =C2=A0 =C2=A0 | =C2=A0 18 ++++++++++++++++++
> =C2=A0src/device.h =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A02 ++
> =C2=A03 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index d1feb18..a667296 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -130,6 +130,10 @@ Properties string Address [readonly]
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0Proposed icon name according to the freedesktop.org
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0icon naming specification.
>
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 string Type [readonly]
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 Device type (BR/EDR, LE, BR/EDR/LE).
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint32 Class [read=
only]
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0The Bluetooth class of device of the remote device.
> diff --git a/src/device.c b/src/device.c
> index d567952..96683de 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -228,6 +228,20 @@ static void device_free(gpointer user_data)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0g_free(device);
> =C2=A0}
>
> +const char *devtype2str(device_type_t type)

If the plan is to use the same type(device_type_t) for devices and
adapters we should rename this type.


> +{
> + =C2=A0 =C2=A0 =C2=A0 switch (type) {
> + =C2=A0 =C2=A0 =C2=A0 case DEVICE_TYPE_BREDR:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return "BR/EDR";
> + =C2=A0 =C2=A0 =C2=A0 case DEVICE_TYPE_LE:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return "LE";
> + =C2=A0 =C2=A0 =C2=A0 case DEVICE_TYPE_DUALMODE:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return "BR/EDR/LE";
> + =C2=A0 =C2=A0 =C2=A0 default:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return "Unknown";
> + =C2=A0 =C2=A0 =C2=A0 }
> +}
> +
> =C2=A0gboolean device_is_paired(struct btd_device *device)
> =C2=A0{
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return device->paired;
> @@ -302,6 +316,10 @@ static DBusMessage *get_properties(DBusConnection *c=
onn,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0DBUS_TYPE_STRING, &icon);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
> + =C2=A0 =C2=A0 =C2=A0 /* Type */
> + =C2=A0 =C2=A0 =C2=A0 ptr =3D devtype2str(device->type);
> + =C2=A0 =C2=A0 =C2=A0 dict_append_entry(&dict, "Type", DBUS_TYPE_STRING,=
&ptr);
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Paired */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0boolean =3D device_is_paired(device);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0dict_append_entry(&dict, "Paired", DBUS_TYPE_B=
OOLEAN, &boolean);
> diff --git a/src/device.h b/src/device.h
> index 3ce212b..b385070 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -41,6 +41,8 @@ typedef enum {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0DEVICE_TYPE_DUALMODE
> =C2=A0} device_type_t;
>
> +const char *devtype2str(device_type_t type);
> +
> =C2=A0struct btd_device *device_create(DBusConnection *conn,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct btd_adapter *adapter,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0const gchar *address, device_type_t t=
ype);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>

2011-03-30 18:33:56

by Andre Dieb Martins

[permalink] [raw]
Subject: [RFC 2/2] Add type property to Adapter

---
doc/adapter-api.txt | 4 ++++
src/adapter.c | 17 ++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index f34d58f..c199fa1 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -225,6 +225,10 @@ Properties string Address [readonly]

The Bluetooth class of device.

+ string Type [readonly]
+
+ Adapter type (BR/EDR, LE or BR/EDR/LE).
+
boolean Powered [readwrite]

Switch an adapter on or off. This will also set the
diff --git a/src/adapter.c b/src/adapter.c
index 4ce7e4c..39a8d0a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -112,6 +112,7 @@ struct btd_adapter {
char *path; /* adapter object path */
bdaddr_t bdaddr; /* adapter Bluetooth Address */
uint32_t dev_class; /* Class of Device */
+ device_type_t type; /* Type (BR/EDR, LE, etc) */
guint discov_timeout_id; /* discoverable timeout id */
guint stop_discov_id; /* stop inquiry/scanning id */
uint32_t discov_timeout; /* discoverable time(sec) */
@@ -1361,6 +1362,10 @@ static DBusMessage *get_properties(DBusConnection *conn,
dict_append_entry(&dict, "Class",
DBUS_TYPE_UINT32, &adapter->dev_class);

+ /* Type */
+ property = devtype2str(adapter->type);
+ dict_append_entry(&dict, "Type", DBUS_TYPE_STRING, &property);
+
/* Powered */
value = (adapter->up && !adapter->off_requested) ? TRUE : FALSE;
dict_append_entry(&dict, "Powered", DBUS_TYPE_BOOLEAN, &value);
@@ -2699,7 +2704,6 @@ gboolean adapter_init(struct btd_adapter *adapter)
adapter->dev_id, strerror(-err), -err);
return FALSE;
}
-
dev = &adapter->dev;

dev->hci_rev = ver.hci_rev;
@@ -2714,6 +2718,17 @@ gboolean adapter_init(struct btd_adapter *adapter)
return FALSE;
}

+ /* Determine controller type based on LMP features */
+ if (dev->features[4] & LMP_NO_BREDR) {
+ if (dev->features[4] & LMP_LE)
+ adapter->type = DEVICE_TYPE_LE;
+ else
+ adapter->type = DEVICE_TYPE_UNKNOWN;
+ } else if (dev->features[6] & LMP_LE_BREDR)
+ adapter->type = DEVICE_TYPE_DUALMODE;
+ else
+ adapter->type = DEVICE_TYPE_BREDR;
+
if (read_local_name(&adapter->bdaddr, adapter->dev.name) < 0)
expand_name(adapter->dev.name, MAX_NAME_LENGTH, main_opts.name,
adapter->dev_id);
--
1.7.1


2011-04-11 17:18:43

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [RFC 1/2] Add Type property to Device

Hi guys,

On Mon, Apr 11, 2011 at 4:00 PM, Gustavo F. Padovan
<[email protected]> wrote:
> * Andre Dieb Martins <[email protected]> [2011-03-30 15:33:55 -0300]:
>
>> ---
>>  doc/device-api.txt |    4 ++++
>>  src/device.c       |   18 ++++++++++++++++++
>>  src/device.h       |    2 ++
>>  3 files changed, 24 insertions(+), 0 deletions(-)
>>
>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>> index d1feb18..a667296 100644
>> --- a/doc/device-api.txt
>> +++ b/doc/device-api.txt
>> @@ -130,6 +130,10 @@ Properties       string Address [readonly]
>>                       Proposed icon name according to the freedesktop.org
>>                       icon naming specification.
>>
>> +             string Type [readonly]
>> +
>> +                     Device type (BR/EDR, LE, BR/EDR/LE).
>> +
>>               uint32 Class [readonly]
>>
>>                       The Bluetooth class of device of the remote device.
>> diff --git a/src/device.c b/src/device.c
>> index d567952..96683de 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -228,6 +228,20 @@ static void device_free(gpointer user_data)
>>       g_free(device);
>>  }
>>
>> +const char *devtype2str(device_type_t type)
>> +{
>> +     switch (type) {
>> +     case DEVICE_TYPE_BREDR:
>> +             return "BR/EDR";
>> +     case DEVICE_TYPE_LE:
>> +             return "LE";
>> +     case DEVICE_TYPE_DUALMODE:
>> +             return "BR/EDR/LE";
>
> I think that  use "Dual Mode" is much better here.
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
> --


Marcel asked me to wait until we have a request from UI developers.

This property is more useful in the DeviceFound() signal, since it is
not clear how to guess the type of the device found. For the Adapter
and Device objects it is less relevant at the moment.

BR,
Claudio

2011-04-11 16:00:58

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [RFC 1/2] Add Type property to Device

* Andre Dieb Martins <[email protected]> [2011-03-30 15:33:55 -0300]:

> ---
> doc/device-api.txt | 4 ++++
> src/device.c | 18 ++++++++++++++++++
> src/device.h | 2 ++
> 3 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index d1feb18..a667296 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -130,6 +130,10 @@ Properties string Address [readonly]
> Proposed icon name according to the freedesktop.org
> icon naming specification.
>
> + string Type [readonly]
> +
> + Device type (BR/EDR, LE, BR/EDR/LE).
> +
> uint32 Class [readonly]
>
> The Bluetooth class of device of the remote device.
> diff --git a/src/device.c b/src/device.c
> index d567952..96683de 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -228,6 +228,20 @@ static void device_free(gpointer user_data)
> g_free(device);
> }
>
> +const char *devtype2str(device_type_t type)
> +{
> + switch (type) {
> + case DEVICE_TYPE_BREDR:
> + return "BR/EDR";
> + case DEVICE_TYPE_LE:
> + return "LE";
> + case DEVICE_TYPE_DUALMODE:
> + return "BR/EDR/LE";

I think that use "Dual Mode" is much better here.

--
Gustavo F. Padovan
http://profusion.mobi

2011-04-11 13:08:20

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: Re: [RFC 1/2] Add Type property to Device

2011/3/31 Claudio Takahasi <[email protected]>
>
> Hi Marcel/Johan/Luiz,
>
> We need opinions for this change...
>
> Why this information is needed: Appearance characteristic is not
> available in the advertising. Currently, BlueZ implements interleaved
> discovery: 5.12sec inquiring + 5.12sec scanning.
> Found devices are reported through the DeviceFound() signal. The user
> space needs to check the "Broadcaster" property inside the DeviceFound
> signal to check if the device is Basic Rate or LE, which is not
> acceptable(in my opinion). We need a meaningful property to
> distinguish LE devices.
>
> Before we push these patches upstream we need to define which
> information it exposes: Operational Mode or host/controller
> capability?
>
> My opinion:
> 1. for the Adapter it should represent the operational mode, a
> controller can support LE, but the host can disable it. Setting
> EnableLE = false(main.conf) should "disable" LE functionalities.
> 2. for DeviceFound(), the UI needs this info only to distinguish LE
> devices. In my opinion UUIDs and Broadcaster properties are not
> enough/appropriated for this purpose. "Qualified" devices will not
> enable inquiry scan and advertising at same time. Our main target is
> to connect to LE only devices. "BR/EDR/LE" can be confusing in this
> context. Suggestions here?
> 3. for the Device, I am not sure if we need to expose this information
> to the users, appearance characteristic is mandatory, if a Device
> "object" exists in the system the appearance value will be
> available(it will be retrieved after discovery primary services)
>
> Comments?

Any news on this?

>
> Andre:
> You need to check the adapter features, old adapters doesn't support EDR.

Andre is on vacation (lucky guy), so I will do the fixes once we have
settled the other questions.

>
> Claudio.
>
> On Wed, Mar 30, 2011 at 6:33 PM, Andre Dieb Martins
> <[email protected]> wrote:
> > ---
> > ?doc/device-api.txt | ? ?4 ++++
> > ?src/device.c ? ? ? | ? 18 ++++++++++++++++++
> > ?src/device.h ? ? ? | ? ?2 ++
> > ?3 files changed, 24 insertions(+), 0 deletions(-)
> >
> > diff --git a/doc/device-api.txt b/doc/device-api.txt
> > index d1feb18..a667296 100644
> > --- a/doc/device-api.txt
> > +++ b/doc/device-api.txt
> > @@ -130,6 +130,10 @@ Properties string Address [readonly]
> > ? ? ? ? ? ? ? ? ? ? ? ?Proposed icon name according to the freedesktop.org
> > ? ? ? ? ? ? ? ? ? ? ? ?icon naming specification.
> >
> > + ? ? ? ? ? ? ? string Type [readonly]
> > +
> > + ? ? ? ? ? ? ? ? ? ? ? Device type (BR/EDR, LE, BR/EDR/LE).
> > +
> > ? ? ? ? ? ? ? ?uint32 Class [readonly]
> >
> > ? ? ? ? ? ? ? ? ? ? ? ?The Bluetooth class of device of the remote device.
> > diff --git a/src/device.c b/src/device.c
> > index d567952..96683de 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -228,6 +228,20 @@ static void device_free(gpointer user_data)
> > ? ? ? ?g_free(device);
> > ?}
> >
> > +const char *devtype2str(device_type_t type)
>
> If the plan is to use the same type(device_type_t) for devices and
> adapters we should rename this type.
>
>
> > +{
> > + ? ? ? switch (type) {
> > + ? ? ? case DEVICE_TYPE_BREDR:
> > + ? ? ? ? ? ? ? return "BR/EDR";
> > + ? ? ? case DEVICE_TYPE_LE:
> > + ? ? ? ? ? ? ? return "LE";
> > + ? ? ? case DEVICE_TYPE_DUALMODE:
> > + ? ? ? ? ? ? ? return "BR/EDR/LE";
> > + ? ? ? default:
> > + ? ? ? ? ? ? ? return "Unknown";
> > + ? ? ? }
> > +}
> > +
> > ?gboolean device_is_paired(struct btd_device *device)
> > ?{
> > ? ? ? ?return device->paired;
> > @@ -302,6 +316,10 @@ static DBusMessage *get_properties(DBusConnection *conn,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_STRING, &icon);
> > ? ? ? ?}
> >
> > + ? ? ? /* Type */
> > + ? ? ? ptr = devtype2str(device->type);
> > + ? ? ? dict_append_entry(&dict, "Type", DBUS_TYPE_STRING, &ptr);
> > +
> > ? ? ? ?/* Paired */
> > ? ? ? ?boolean = device_is_paired(device);
> > ? ? ? ?dict_append_entry(&dict, "Paired", DBUS_TYPE_BOOLEAN, &boolean);
> > diff --git a/src/device.h b/src/device.h
> > index 3ce212b..b385070 100644
> > --- a/src/device.h
> > +++ b/src/device.h
> > @@ -41,6 +41,8 @@ typedef enum {
> > ? ? ? ?DEVICE_TYPE_DUALMODE
> > ?} device_type_t;
> >
> > +const char *devtype2str(device_type_t type);
> > +
> > ?struct btd_device *device_create(DBusConnection *conn,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct btd_adapter *adapter,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const gchar *address, device_type_t type);
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to [email protected]
> > More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html