All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
@ 2015-02-10  0:34 Jakub Pawlowski
  2015-02-16  8:46 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-10  0:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jakub Pawlowski

This patch proposes new method, StartFilteredDiscovery to D-Bus Adapter
API for desktop bluetoothd. It will allow for rapid discovery of nearby
devices that advertise services.

It also adds FilteredDiscovery property, that is set to true when any
StartFilteredDiscovery session is active.

Signed-off-by: Jakub Pawlowski <jpawlowski@google.com>
---
 doc/adapter-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index 74d235a..b59019d 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -22,6 +22,50 @@ Methods		void StartDiscovery()
 			Possible errors: org.bluez.Error.NotReady
 					 org.bluez.Error.Failed
 
+		void StartFilteredDiscovery(dict filter)
+
+			This method starts the device discovery session with
+			filtering by uuids, and rssi or pathloss value. Use
+			StopDiscovery to release the sessions acquired.
+
+ 			Parameters that can be set in filter dictionary include
+ 			the following:
+
+			array{string} UUIDs : filtered service UUIDs (required)
+			int16 RSSI	: RSSI threshold value (optional)
+			uint16 pathloss	: Pathloss threshold value (optional)
+			string transport: type of scan to run
+
+			When a device is found that advertise any UUID from
+			UUIDs, it will be reported if:
+			- pathloss and RSSI are both empty,
+			- only pathloss param is set, device advertise TX pwer,
+			  and computed pathloss is less than pathloss param,
+			- only RSSI param is set, and received RSSI is higher
+			  than RSSI param,
+
+			transport parameter determines the type of scan:
+				"auto"	- interleaved scan
+				"bredr"	- br/edr inquiry
+				"le"	- le only scan, default value
+
+			This process will start creating Device objects as new
+			devices matching criteria are discovered. It will also
+			emit PropertiesChanged signal for already existing
+			Device objects, with updated RSSI value.
+
+			StartDiscovery (SD) takes precedence over
+			StartFilteredDiscovery (SFD). That means that you should
+			check Discovering property before calling SFD, otherwise
+			it would fail when any SD session is in progress.
+			Calling SD when SFD is in progress pause all SFD
+			sessions, and start them back when all SD sessions are
+			finished. When filtered discovery state is changed,
+			FilteredDiscovery variable is updated accordingly.
+
+			Possible errors: org.bluez.Error.NotReady
+					 org.bluez.Error.Failed
+
 		void StopDiscovery()
 
 			This method will cancel any previous StartDiscovery
@@ -144,6 +188,11 @@ Properties	string Address [readonly]
 
 			Indicates that a device discovery procedure is active.
 
+		boolean FilteredDiscovery [readonly]
+
+			Indicates that a filtered device discovery procedure is
+			active.
+
 		array{string} UUIDs [readonly]
 
 			List of 128-bit UUIDs that represents the available
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-10  0:34 [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method Jakub Pawlowski
@ 2015-02-16  8:46 ` Luiz Augusto von Dentz
  2015-02-20 17:09   ` Jakub Pawlowski
  2015-02-23 13:09   ` Johan Hedberg
  0 siblings, 2 replies; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-16  8:46 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: linux-bluetooth, Marcel Holtmann, Johan Hedberg

Hi Jakub,

On Tue, Feb 10, 2015 at 2:34 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> This patch proposes new method, StartFilteredDiscovery to D-Bus Adapter
> API for desktop bluetoothd. It will allow for rapid discovery of nearby
> devices that advertise services.
>
> It also adds FilteredDiscovery property, that is set to true when any
> StartFilteredDiscovery session is active.
>
> Signed-off-by: Jakub Pawlowski <jpawlowski@google.com>

We don't use Signed-off-byin userspace, I can fix this myself this
time but please don't sent patches with it anymore.

> ---
>  doc/adapter-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 74d235a..b59019d 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -22,6 +22,50 @@ Methods              void StartDiscovery()
>                         Possible errors: org.bluez.Error.NotReady
>                                          org.bluez.Error.Failed
>
> +               void StartFilteredDiscovery(dict filter)
> +
> +                       This method starts the device discovery session with
> +                       filtering by uuids, and rssi or pathloss value. Use
> +                       StopDiscovery to release the sessions acquired.
> +
> +                       Parameters that can be set in filter dictionary include
> +                       the following:
> +
> +                       array{string} UUIDs : filtered service UUIDs (required)
> +                       int16 RSSI      : RSSI threshold value (optional)
> +                       uint16 pathloss : Pathloss threshold value (optional)
> +                       string transport: type of scan to run
> +
> +                       When a device is found that advertise any UUID from
> +                       UUIDs, it will be reported if:
> +                       - pathloss and RSSI are both empty,
> +                       - only pathloss param is set, device advertise TX pwer,
> +                         and computed pathloss is less than pathloss param,
> +                       - only RSSI param is set, and received RSSI is higher
> +                         than RSSI param,
> +
> +                       transport parameter determines the type of scan:
> +                               "auto"  - interleaved scan
> +                               "bredr" - br/edr inquiry
> +                               "le"    - le only scan, default value
> +
> +                       This process will start creating Device objects as new
> +                       devices matching criteria are discovered. It will also
> +                       emit PropertiesChanged signal for already existing
> +                       Device objects, with updated RSSI value.
> +
> +                       StartDiscovery (SD) takes precedence over
> +                       StartFilteredDiscovery (SFD). That means that you should
> +                       check Discovering property before calling SFD, otherwise
> +                       it would fail when any SD session is in progress.
> +                       Calling SD when SFD is in progress pause all SFD
> +                       sessions, and start them back when all SD sessions are
> +                       finished. When filtered discovery state is changed,
> +                       FilteredDiscovery variable is updated accordingly.
> +
> +                       Possible errors: org.bluez.Error.NotReady
> +                                        org.bluez.Error.Failed
> +
>                 void StopDiscovery()
>
>                         This method will cancel any previous StartDiscovery
> @@ -144,6 +188,11 @@ Properties string Address [readonly]
>
>                         Indicates that a device discovery procedure is active.
>
> +               boolean FilteredDiscovery [readonly]
> +
> +                       Indicates that a filtered device discovery procedure is
> +                       active.
> +
>                 array{string} UUIDs [readonly]
>
>                         List of 128-bit UUIDs that represents the available
> --
> 2.2.0.rc0.207.ga3a616c

@Marcel, Johan: Is this API okay for you? We should probably make them
experimental to begin with.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-16  8:46 ` Luiz Augusto von Dentz
@ 2015-02-20 17:09   ` Jakub Pawlowski
  2015-02-23 13:09   ` Johan Hedberg
  1 sibling, 0 replies; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-20 17:09 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Marcel Holtmann, Johan Hedberg

On Mon, Feb 16, 2015 at 12:46 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Jakub,
>
> On Tue, Feb 10, 2015 at 2:34 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>> This patch proposes new method, StartFilteredDiscovery to D-Bus Adapter
>> API for desktop bluetoothd. It will allow for rapid discovery of nearby
>> devices that advertise services.
>>
>> It also adds FilteredDiscovery property, that is set to true when any
>> StartFilteredDiscovery session is active.
>>
>> Signed-off-by: Jakub Pawlowski <jpawlowski@google.com>
>
> We don't use Signed-off-byin userspace, I can fix this myself this
> time but please don't sent patches with it anymore.

Ok, thanks
>
>> ---
>>  doc/adapter-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>> index 74d235a..b59019d 100644
>> --- a/doc/adapter-api.txt
>> +++ b/doc/adapter-api.txt
>> @@ -22,6 +22,50 @@ Methods              void StartDiscovery()
>>                         Possible errors: org.bluez.Error.NotReady
>>                                          org.bluez.Error.Failed
>>
>> +               void StartFilteredDiscovery(dict filter)
>> +
>> +                       This method starts the device discovery session with
>> +                       filtering by uuids, and rssi or pathloss value. Use
>> +                       StopDiscovery to release the sessions acquired.
>> +
>> +                       Parameters that can be set in filter dictionary include
>> +                       the following:
>> +
>> +                       array{string} UUIDs : filtered service UUIDs (required)
>> +                       int16 RSSI      : RSSI threshold value (optional)
>> +                       uint16 pathloss : Pathloss threshold value (optional)
>> +                       string transport: type of scan to run
>> +
>> +                       When a device is found that advertise any UUID from
>> +                       UUIDs, it will be reported if:
>> +                       - pathloss and RSSI are both empty,
>> +                       - only pathloss param is set, device advertise TX pwer,
>> +                         and computed pathloss is less than pathloss param,
>> +                       - only RSSI param is set, and received RSSI is higher
>> +                         than RSSI param,
>> +
>> +                       transport parameter determines the type of scan:
>> +                               "auto"  - interleaved scan
>> +                               "bredr" - br/edr inquiry
>> +                               "le"    - le only scan, default value
>> +
>> +                       This process will start creating Device objects as new
>> +                       devices matching criteria are discovered. It will also
>> +                       emit PropertiesChanged signal for already existing
>> +                       Device objects, with updated RSSI value.
>> +
>> +                       StartDiscovery (SD) takes precedence over
>> +                       StartFilteredDiscovery (SFD). That means that you should
>> +                       check Discovering property before calling SFD, otherwise
>> +                       it would fail when any SD session is in progress.
>> +                       Calling SD when SFD is in progress pause all SFD
>> +                       sessions, and start them back when all SD sessions are
>> +                       finished. When filtered discovery state is changed,
>> +                       FilteredDiscovery variable is updated accordingly.
>> +
>> +                       Possible errors: org.bluez.Error.NotReady
>> +                                        org.bluez.Error.Failed
>> +
>>                 void StopDiscovery()
>>
>>                         This method will cancel any previous StartDiscovery
>> @@ -144,6 +188,11 @@ Properties string Address [readonly]
>>
>>                         Indicates that a device discovery procedure is active.
>>
>> +               boolean FilteredDiscovery [readonly]
>> +
>> +                       Indicates that a filtered device discovery procedure is
>> +                       active.
>> +
>>                 array{string} UUIDs [readonly]
>>
>>                         List of 128-bit UUIDs that represents the available
>> --
>> 2.2.0.rc0.207.ga3a616c
>
> @Marcel, Johan: Is this API okay for you? We should probably make them
> experimental to begin with.

Ping ?

>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-16  8:46 ` Luiz Augusto von Dentz
  2015-02-20 17:09   ` Jakub Pawlowski
@ 2015-02-23 13:09   ` Johan Hedberg
  2015-02-23 16:41     ` Jakub Pawlowski
  1 sibling, 1 reply; 15+ messages in thread
From: Johan Hedberg @ 2015-02-23 13:09 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Jakub Pawlowski, linux-bluetooth, Marcel Holtmann

Hi Luiz,

On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
> > +               void StartFilteredDiscovery(dict filter)
> > +
> > +                       This method starts the device discovery session with
> > +                       filtering by uuids, and rssi or pathloss value. Use
> > +                       StopDiscovery to release the sessions acquired.
> > +
> > +                       Parameters that can be set in filter dictionary include
> > +                       the following:
> > +
> > +                       array{string} UUIDs : filtered service UUIDs (required)
> > +                       int16 RSSI      : RSSI threshold value (optional)
> > +                       uint16 pathloss : Pathloss threshold value (optional)
> > +                       string transport: type of scan to run
> > +
> > +                       When a device is found that advertise any UUID from
> > +                       UUIDs, it will be reported if:
> > +                       - pathloss and RSSI are both empty,
> > +                       - only pathloss param is set, device advertise TX pwer,
> > +                         and computed pathloss is less than pathloss param,
> > +                       - only RSSI param is set, and received RSSI is higher
> > +                         than RSSI param,
> > +
> > +                       transport parameter determines the type of scan:
> > +                               "auto"  - interleaved scan
> > +                               "bredr" - br/edr inquiry
> > +                               "le"    - le only scan, default value
> > +
> > +                       This process will start creating Device objects as new
> > +                       devices matching criteria are discovered. It will also
> > +                       emit PropertiesChanged signal for already existing
> > +                       Device objects, with updated RSSI value.
> > +
> > +                       StartDiscovery (SD) takes precedence over
> > +                       StartFilteredDiscovery (SFD). That means that you should
> > +                       check Discovering property before calling SFD, otherwise
> > +                       it would fail when any SD session is in progress.
> > +                       Calling SD when SFD is in progress pause all SFD
> > +                       sessions, and start them back when all SD sessions are
> > +                       finished. When filtered discovery state is changed,
> > +                       FilteredDiscovery variable is updated accordingly.
> > +
> > +                       Possible errors: org.bluez.Error.NotReady
> > +                                        org.bluez.Error.Failed
> > +
> >                 void StopDiscovery()
> >
> >                         This method will cancel any previous StartDiscovery
> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
> >
> >                         Indicates that a device discovery procedure is active.
> >
> > +               boolean FilteredDiscovery [readonly]
> > +
> > +                       Indicates that a filtered device discovery procedure is
> > +                       active.
> > +
> >                 array{string} UUIDs [readonly]
> >
> >                         List of 128-bit UUIDs that represents the available
> > --
> > 2.2.0.rc0.207.ga3a616c
> 
> @Marcel, Johan: Is this API okay for you? We should probably make them
> experimental to begin with.

For the most part I'm fine with this, especially if it's marked as
experimental. As for the relationship to StartDiscovery, I'd take a
similar approach as we have done with the mgmt API, i.e. StartDiscovery
would be just a special kind of filtered discovery without any filters
(i.e. reporting all devices).

For the case of multiple applications requesting different discoveries
at the same time we'd go with the common denominator approach, i.e.
applications must be prepared to see devices outside of the range of
filters that it has specified (the only way to avoid something like that
support is to do unicast D-Bus signals which in turn would break
"passive observer" applications). Calling StartDiscovery when there are
one or more service discoveries in progress would simply clear out the
filters and start reporting all devices.

Considering the above style approach, the new property seems a bit
pointless.

Since we can't any more track multiple different discoveries within the
same application (D-Bus connection) the StopDiscovery behavior is now
quite broken. The simplest way around that would be to add a discovery
instance return parameter to StartServiceDiscovery and to have a new
StopServiceDiscovery D-Bus method that'd take this as an input
parameter.

Johan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 13:09   ` Johan Hedberg
@ 2015-02-23 16:41     ` Jakub Pawlowski
  2015-02-23 18:43       ` Jakub Pawlowski
  2015-02-24 10:37       ` Johan Hedberg
  0 siblings, 2 replies; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-23 16:41 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Jakub Pawlowski, linux-bluetooth,
	Marcel Holtmann

Hi Johan, Luiz

On Mon, Feb 23, 2015 at 5:09 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Luiz,
>
> On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
>> > +               void StartFilteredDiscovery(dict filter)
>> > +
>> > +                       This method starts the device discovery session with
>> > +                       filtering by uuids, and rssi or pathloss value. Use
>> > +                       StopDiscovery to release the sessions acquired.
>> > +
>> > +                       Parameters that can be set in filter dictionary include
>> > +                       the following:
>> > +
>> > +                       array{string} UUIDs : filtered service UUIDs (required)
>> > +                       int16 RSSI      : RSSI threshold value (optional)
>> > +                       uint16 pathloss : Pathloss threshold value (optional)
>> > +                       string transport: type of scan to run
>> > +
>> > +                       When a device is found that advertise any UUID from
>> > +                       UUIDs, it will be reported if:
>> > +                       - pathloss and RSSI are both empty,
>> > +                       - only pathloss param is set, device advertise TX pwer,
>> > +                         and computed pathloss is less than pathloss param,
>> > +                       - only RSSI param is set, and received RSSI is higher
>> > +                         than RSSI param,
>> > +
>> > +                       transport parameter determines the type of scan:
>> > +                               "auto"  - interleaved scan
>> > +                               "bredr" - br/edr inquiry
>> > +                               "le"    - le only scan, default value
>> > +
>> > +                       This process will start creating Device objects as new
>> > +                       devices matching criteria are discovered. It will also
>> > +                       emit PropertiesChanged signal for already existing
>> > +                       Device objects, with updated RSSI value.
>> > +
>> > +                       StartDiscovery (SD) takes precedence over
>> > +                       StartFilteredDiscovery (SFD). That means that you should
>> > +                       check Discovering property before calling SFD, otherwise
>> > +                       it would fail when any SD session is in progress.
>> > +                       Calling SD when SFD is in progress pause all SFD
>> > +                       sessions, and start them back when all SD sessions are
>> > +                       finished. When filtered discovery state is changed,
>> > +                       FilteredDiscovery variable is updated accordingly.
>> > +
>> > +                       Possible errors: org.bluez.Error.NotReady
>> > +                                        org.bluez.Error.Failed
>> > +
>> >                 void StopDiscovery()
>> >
>> >                         This method will cancel any previous StartDiscovery
>> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
>> >
>> >                         Indicates that a device discovery procedure is active.
>> >
>> > +               boolean FilteredDiscovery [readonly]
>> > +
>> > +                       Indicates that a filtered device discovery procedure is
>> > +                       active.
>> > +
>> >                 array{string} UUIDs [readonly]
>> >
>> >                         List of 128-bit UUIDs that represents the available
>> > --
>> > 2.2.0.rc0.207.ga3a616c
>>
>> @Marcel, Johan: Is this API okay for you? We should probably make them
>> experimental to begin with.
>
> For the most part I'm fine with this, especially if it's marked as
> experimental. As for the relationship to StartDiscovery, I'd take a
> similar approach as we have done with the mgmt API, i.e. StartDiscovery
> would be just a special kind of filtered discovery without any filters
> (i.e. reporting all devices).

I want StartDiscovery to behave differently than
StartFilteredDiscovery without any filter:
1. StartDiscovery is interleaved scan, StartFilteredDiscovery is by
default doing LE only scan (BR/EDR doesn't make no point here). If
StartFilteredDiscovery would do interleaved scan, it would waste half
of time not reporting anything.
2. StartDiscovery is reporting RSSI changes with delta of 8dB or more,
StartFilteredDiscovery will report every RSSI change it gets, to allow
better proximity approximation. If StartDiscovery report every RSSI
change it would break some use cases (i.e. menus showing list of
devices would flicker), if StartFilteredDiscovery won't return every
RSSI change it would kill my use case.


>
> For the case of multiple applications requesting different discoveries
> at the same time we'd go with the common denominator approach, i.e.
> applications must be prepared to see devices outside of the range of
> filters that it has specified (the only way to avoid something like that
> support is to do unicast D-Bus signals which in turn would break
> "passive observer" applications). Calling StartDiscovery when there are
> one or more service discoveries in progress would simply clear out the
> filters and start reporting all devices.

Yes, applications must be prepared to see devices outside of range.
When you call StartDiscovery when StartFilteredDiscovery is in
progress, filtered discoveries should be paused, and properties should
be changed accordingly ("FilteredDiscovery"=false,
"Discovering"=true).  This would make sure that we don't cause
regressions in apps already using StartDiscovery, and new apps that
use StartFilteredDiscovery might detect this change by looking at
properties, and try to handle it correctly (if they're fine with less
frequent RSSI updates, no proximity) or show some message and wait for
other apps to stop their discovery.

>
> Considering the above style approach, the new property seems a bit
> pointless.

As I mentioned it'll be needed for apps that do StartFilteredDiscovery
to check wether it's currently paused to allow other app to do it's
StartDiscovery (it takes precedence over StartFilteredDiscovery).

>
> Since we can't any more track multiple different discoveries within the
> same application (D-Bus connection) the StopDiscovery behavior is now
> quite broken. The simplest way around that would be to add a discovery
> instance return parameter to StartServiceDiscovery and to have a new
> StopServiceDiscovery D-Bus method that'd take this as an input
> parameter.

Right now each application can call StartDiscovery only once, I want
each application to be able to call StartDiscovery OR
StartFilteredDiscovery only once. This way we are completly fine with
one StopDiscovery method. If some application needs to change filter,
it can always stop and restart it's scan.

>
> Johan

Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 16:41     ` Jakub Pawlowski
@ 2015-02-23 18:43       ` Jakub Pawlowski
  2015-02-23 21:43         ` Arman Uguray
  2015-02-24 10:37       ` Johan Hedberg
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-23 18:43 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Jakub Pawlowski, linux-bluetooth,
	Marcel Holtmann

On Mon, Feb 23, 2015 at 8:41 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> Hi Johan, Luiz
>
> On Mon, Feb 23, 2015 at 5:09 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Hi Luiz,
>>
>> On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
>>> > +               void StartFilteredDiscovery(dict filter)
>>> > +
>>> > +                       This method starts the device discovery session with
>>> > +                       filtering by uuids, and rssi or pathloss value. Use
>>> > +                       StopDiscovery to release the sessions acquired.
>>> > +
>>> > +                       Parameters that can be set in filter dictionary include
>>> > +                       the following:
>>> > +
>>> > +                       array{string} UUIDs : filtered service UUIDs (required)
>>> > +                       int16 RSSI      : RSSI threshold value (optional)
>>> > +                       uint16 pathloss : Pathloss threshold value (optional)
>>> > +                       string transport: type of scan to run
>>> > +
>>> > +                       When a device is found that advertise any UUID from
>>> > +                       UUIDs, it will be reported if:
>>> > +                       - pathloss and RSSI are both empty,
>>> > +                       - only pathloss param is set, device advertise TX pwer,
>>> > +                         and computed pathloss is less than pathloss param,
>>> > +                       - only RSSI param is set, and received RSSI is higher
>>> > +                         than RSSI param,
>>> > +
>>> > +                       transport parameter determines the type of scan:
>>> > +                               "auto"  - interleaved scan
>>> > +                               "bredr" - br/edr inquiry
>>> > +                               "le"    - le only scan, default value
>>> > +
>>> > +                       This process will start creating Device objects as new
>>> > +                       devices matching criteria are discovered. It will also
>>> > +                       emit PropertiesChanged signal for already existing
>>> > +                       Device objects, with updated RSSI value.
>>> > +
>>> > +                       StartDiscovery (SD) takes precedence over
>>> > +                       StartFilteredDiscovery (SFD). That means that you should
>>> > +                       check Discovering property before calling SFD, otherwise
>>> > +                       it would fail when any SD session is in progress.
>>> > +                       Calling SD when SFD is in progress pause all SFD
>>> > +                       sessions, and start them back when all SD sessions are
>>> > +                       finished. When filtered discovery state is changed,
>>> > +                       FilteredDiscovery variable is updated accordingly.
>>> > +
>>> > +                       Possible errors: org.bluez.Error.NotReady
>>> > +                                        org.bluez.Error.Failed
>>> > +
>>> >                 void StopDiscovery()
>>> >
>>> >                         This method will cancel any previous StartDiscovery
>>> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
>>> >
>>> >                         Indicates that a device discovery procedure is active.
>>> >
>>> > +               boolean FilteredDiscovery [readonly]
>>> > +
>>> > +                       Indicates that a filtered device discovery procedure is
>>> > +                       active.
>>> > +
>>> >                 array{string} UUIDs [readonly]
>>> >
>>> >                         List of 128-bit UUIDs that represents the available
>>> > --
>>> > 2.2.0.rc0.207.ga3a616c
>>>
>>> @Marcel, Johan: Is this API okay for you? We should probably make them
>>> experimental to begin with.
>>
>> For the most part I'm fine with this, especially if it's marked as
>> experimental. As for the relationship to StartDiscovery, I'd take a
>> similar approach as we have done with the mgmt API, i.e. StartDiscovery
>> would be just a special kind of filtered discovery without any filters
>> (i.e. reporting all devices).
>
> I want StartDiscovery to behave differently than
> StartFilteredDiscovery without any filter:
> 1. StartDiscovery is interleaved scan, StartFilteredDiscovery is by
> default doing LE only scan (BR/EDR doesn't make no point here). If
> StartFilteredDiscovery would do interleaved scan, it would waste half
> of time not reporting anything.
> 2. StartDiscovery is reporting RSSI changes with delta of 8dB or more,
> StartFilteredDiscovery will report every RSSI change it gets, to allow
> better proximity approximation. If StartDiscovery report every RSSI
> change it would break some use cases (i.e. menus showing list of
> devices would flicker), if StartFilteredDiscovery won't return every
> RSSI change it would kill my use case.
>
>
>>
>> For the case of multiple applications requesting different discoveries
>> at the same time we'd go with the common denominator approach, i.e.
>> applications must be prepared to see devices outside of the range of
>> filters that it has specified (the only way to avoid something like that
>> support is to do unicast D-Bus signals which in turn would break
>> "passive observer" applications). Calling StartDiscovery when there are
>> one or more service discoveries in progress would simply clear out the
>> filters and start reporting all devices.
>
> Yes, applications must be prepared to see devices outside of range.
> When you call StartDiscovery when StartFilteredDiscovery is in
> progress, filtered discoveries should be paused, and properties should
> be changed accordingly ("FilteredDiscovery"=false,
> "Discovering"=true).  This would make sure that we don't cause
> regressions in apps already using StartDiscovery, and new apps that
> use StartFilteredDiscovery might detect this change by looking at
> properties, and try to handle it correctly (if they're fine with less
> frequent RSSI updates, no proximity) or show some message and wait for
> other apps to stop their discovery.
>
>>
>> Considering the above style approach, the new property seems a bit
>> pointless.
>
> As I mentioned it'll be needed for apps that do StartFilteredDiscovery
> to check wether it's currently paused to allow other app to do it's
> StartDiscovery (it takes precedence over StartFilteredDiscovery).
>
>>
>> Since we can't any more track multiple different discoveries within the
>> same application (D-Bus connection) the StopDiscovery behavior is now
>> quite broken. The simplest way around that would be to add a discovery
>> instance return parameter to StartServiceDiscovery and to have a new
>> StopServiceDiscovery D-Bus method that'd take this as an input
>> parameter.
>
> Right now each application can call StartDiscovery only once, I want
> each application to be able to call StartDiscovery OR
> StartFilteredDiscovery only once. This way we are completly fine with
> one StopDiscovery method. If some application needs to change filter,
> it can always stop and restart it's scan.

It might be good idea to allow multpile StartFilteredDiscovery calls
from one client, each replacing the old filter. It would still require
only one StopDiscovery method.

>
>>
>> Johan
>
> Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 18:43       ` Jakub Pawlowski
@ 2015-02-23 21:43         ` Arman Uguray
  2015-02-23 22:44           ` Jakub Pawlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Arman Uguray @ 2015-02-23 21:43 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Marcel Holtmann

Hi Jakub, Johan,

> On Mon, Feb 23, 2015 at 10:43 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> On Mon, Feb 23, 2015 at 8:41 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>> Hi Johan, Luiz
>>
>> On Mon, Feb 23, 2015 at 5:09 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>>> Hi Luiz,
>>>
>>> On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
>>>> > +               void StartFilteredDiscovery(dict filter)
>>>> > +
>>>> > +                       This method starts the device discovery session with
>>>> > +                       filtering by uuids, and rssi or pathloss value. Use
>>>> > +                       StopDiscovery to release the sessions acquired.
>>>> > +
>>>> > +                       Parameters that can be set in filter dictionary include
>>>> > +                       the following:
>>>> > +
>>>> > +                       array{string} UUIDs : filtered service UUIDs (required)
>>>> > +                       int16 RSSI      : RSSI threshold value (optional)
>>>> > +                       uint16 pathloss : Pathloss threshold value (optional)
>>>> > +                       string transport: type of scan to run
>>>> > +
>>>> > +                       When a device is found that advertise any UUID from
>>>> > +                       UUIDs, it will be reported if:
>>>> > +                       - pathloss and RSSI are both empty,
>>>> > +                       - only pathloss param is set, device advertise TX pwer,
>>>> > +                         and computed pathloss is less than pathloss param,
>>>> > +                       - only RSSI param is set, and received RSSI is higher
>>>> > +                         than RSSI param,
>>>> > +
>>>> > +                       transport parameter determines the type of scan:
>>>> > +                               "auto"  - interleaved scan
>>>> > +                               "bredr" - br/edr inquiry
>>>> > +                               "le"    - le only scan, default value
>>>> > +
>>>> > +                       This process will start creating Device objects as new
>>>> > +                       devices matching criteria are discovered. It will also
>>>> > +                       emit PropertiesChanged signal for already existing
>>>> > +                       Device objects, with updated RSSI value.
>>>> > +
>>>> > +                       StartDiscovery (SD) takes precedence over
>>>> > +                       StartFilteredDiscovery (SFD). That means that you should
>>>> > +                       check Discovering property before calling SFD, otherwise
>>>> > +                       it would fail when any SD session is in progress.
>>>> > +                       Calling SD when SFD is in progress pause all SFD
>>>> > +                       sessions, and start them back when all SD sessions are
>>>> > +                       finished. When filtered discovery state is changed,
>>>> > +                       FilteredDiscovery variable is updated accordingly.
>>>> > +
>>>> > +                       Possible errors: org.bluez.Error.NotReady
>>>> > +                                        org.bluez.Error.Failed
>>>> > +
>>>> >                 void StopDiscovery()
>>>> >
>>>> >                         This method will cancel any previous StartDiscovery
>>>> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
>>>> >
>>>> >                         Indicates that a device discovery procedure is active.
>>>> >
>>>> > +               boolean FilteredDiscovery [readonly]
>>>> > +
>>>> > +                       Indicates that a filtered device discovery procedure is
>>>> > +                       active.
>>>> > +
>>>> >                 array{string} UUIDs [readonly]
>>>> >
>>>> >                         List of 128-bit UUIDs that represents the available
>>>> > --
>>>> > 2.2.0.rc0.207.ga3a616c
>>>>
>>>> @Marcel, Johan: Is this API okay for you? We should probably make them
>>>> experimental to begin with.
>>>
>>> For the most part I'm fine with this, especially if it's marked as
>>> experimental. As for the relationship to StartDiscovery, I'd take a
>>> similar approach as we have done with the mgmt API, i.e. StartDiscovery
>>> would be just a special kind of filtered discovery without any filters
>>> (i.e. reporting all devices).
>>
>> I want StartDiscovery to behave differently than
>> StartFilteredDiscovery without any filter:
>> 1. StartDiscovery is interleaved scan, StartFilteredDiscovery is by
>> default doing LE only scan (BR/EDR doesn't make no point here). If
>> StartFilteredDiscovery would do interleaved scan, it would waste half
>> of time not reporting anything.
>> 2. StartDiscovery is reporting RSSI changes with delta of 8dB or more,
>> StartFilteredDiscovery will report every RSSI change it gets, to allow
>> better proximity approximation. If StartDiscovery report every RSSI
>> change it would break some use cases (i.e. menus showing list of
>> devices would flicker), if StartFilteredDiscovery won't return every
>> RSSI change it would kill my use case.
>>
>>
>>>
>>> For the case of multiple applications requesting different discoveries
>>> at the same time we'd go with the common denominator approach, i.e.
>>> applications must be prepared to see devices outside of the range of
>>> filters that it has specified (the only way to avoid something like that
>>> support is to do unicast D-Bus signals which in turn would break
>>> "passive observer" applications). Calling StartDiscovery when there are
>>> one or more service discoveries in progress would simply clear out the
>>> filters and start reporting all devices.
>>
>> Yes, applications must be prepared to see devices outside of range.
>> When you call StartDiscovery when StartFilteredDiscovery is in
>> progress, filtered discoveries should be paused, and properties should
>> be changed accordingly ("FilteredDiscovery"=false,
>> "Discovering"=true).  This would make sure that we don't cause
>> regressions in apps already using StartDiscovery, and new apps that
>> use StartFilteredDiscovery might detect this change by looking at
>> properties, and try to handle it correctly (if they're fine with less
>> frequent RSSI updates, no proximity) or show some message and wait for
>> other apps to stop their discovery.
>>
>>>
>>> Considering the above style approach, the new property seems a bit
>>> pointless.
>>
>> As I mentioned it'll be needed for apps that do StartFilteredDiscovery
>> to check wether it's currently paused to allow other app to do it's
>> StartDiscovery (it takes precedence over StartFilteredDiscovery).
>>
>>>
>>> Since we can't any more track multiple different discoveries within the
>>> same application (D-Bus connection) the StopDiscovery behavior is now
>>> quite broken. The simplest way around that would be to add a discovery
>>> instance return parameter to StartServiceDiscovery and to have a new
>>> StopServiceDiscovery D-Bus method that'd take this as an input
>>> parameter.
>>
>> Right now each application can call StartDiscovery only once, I want
>> each application to be able to call StartDiscovery OR
>> StartFilteredDiscovery only once. This way we are completly fine with
>> one StopDiscovery method. If some application needs to change filter,
>> it can always stop and restart it's scan.
>
> It might be good idea to allow multpile StartFilteredDiscovery calls
> from one client, each replacing the old filter. It would still require
> only one StopDiscovery method.
>

The 8 dB RSSI argument isn't really valid since this is something that
we can change in the API definition. You're saying that interleaved
scan is wasteful but that's beside the point here, since the API does
allow passing "auto" and "bredr" as the argument and "le" just happens
to be the default option.

>From an API stand point, I like Johan's suggestion better. Basically
we treat all discovery APIs as "filtered" except StartDiscovery is
just a special case of StartFilteredDiscovery that has 8 dB as the
"RSSI" argument and "auto" for transport. If an app wants to use a
lower RSSI threshold then whether or not the UI will flicker should be
the responsibility of the UI code. Basically we should treat
StartDiscovery as a legacy API and perhaps mark it as deprecated while
maintaining support for it for backwards compatibility.

Within the new framework, many apps can call StartFilteredDiscovery
with differing filters so the daemon should just adapt the filter
based on the biggest common denominator. So if I called
StartFilteredDiscovery with no UUIDs and someone else calls it with a
UUID, the initial call should win. If the first app calls
StopDiscovery then we fallback to the second app's filter, etc.

>>
>>>
>>> Johan
>>
>> Jakub
> --
> 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

Arman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 21:43         ` Arman Uguray
@ 2015-02-23 22:44           ` Jakub Pawlowski
  2015-02-23 23:07             ` Arman Uguray
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-23 22:44 UTC (permalink / raw)
  To: Arman Uguray; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Marcel Holtmann

On Mon, Feb 23, 2015 at 1:43 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Jakub, Johan,
>
>> On Mon, Feb 23, 2015 at 10:43 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>> On Mon, Feb 23, 2015 at 8:41 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>>> Hi Johan, Luiz
>>>
>>> On Mon, Feb 23, 2015 at 5:09 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>>>> Hi Luiz,
>>>>
>>>> On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
>>>>> > +               void StartFilteredDiscovery(dict filter)
>>>>> > +
>>>>> > +                       This method starts the device discovery session with
>>>>> > +                       filtering by uuids, and rssi or pathloss value. Use
>>>>> > +                       StopDiscovery to release the sessions acquired.
>>>>> > +
>>>>> > +                       Parameters that can be set in filter dictionary include
>>>>> > +                       the following:
>>>>> > +
>>>>> > +                       array{string} UUIDs : filtered service UUIDs (required)
>>>>> > +                       int16 RSSI      : RSSI threshold value (optional)
>>>>> > +                       uint16 pathloss : Pathloss threshold value (optional)
>>>>> > +                       string transport: type of scan to run
>>>>> > +
>>>>> > +                       When a device is found that advertise any UUID from
>>>>> > +                       UUIDs, it will be reported if:
>>>>> > +                       - pathloss and RSSI are both empty,
>>>>> > +                       - only pathloss param is set, device advertise TX pwer,
>>>>> > +                         and computed pathloss is less than pathloss param,
>>>>> > +                       - only RSSI param is set, and received RSSI is higher
>>>>> > +                         than RSSI param,
>>>>> > +
>>>>> > +                       transport parameter determines the type of scan:
>>>>> > +                               "auto"  - interleaved scan
>>>>> > +                               "bredr" - br/edr inquiry
>>>>> > +                               "le"    - le only scan, default value
>>>>> > +
>>>>> > +                       This process will start creating Device objects as new
>>>>> > +                       devices matching criteria are discovered. It will also
>>>>> > +                       emit PropertiesChanged signal for already existing
>>>>> > +                       Device objects, with updated RSSI value.
>>>>> > +
>>>>> > +                       StartDiscovery (SD) takes precedence over
>>>>> > +                       StartFilteredDiscovery (SFD). That means that you should
>>>>> > +                       check Discovering property before calling SFD, otherwise
>>>>> > +                       it would fail when any SD session is in progress.
>>>>> > +                       Calling SD when SFD is in progress pause all SFD
>>>>> > +                       sessions, and start them back when all SD sessions are
>>>>> > +                       finished. When filtered discovery state is changed,
>>>>> > +                       FilteredDiscovery variable is updated accordingly.
>>>>> > +
>>>>> > +                       Possible errors: org.bluez.Error.NotReady
>>>>> > +                                        org.bluez.Error.Failed
>>>>> > +
>>>>> >                 void StopDiscovery()
>>>>> >
>>>>> >                         This method will cancel any previous StartDiscovery
>>>>> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
>>>>> >
>>>>> >                         Indicates that a device discovery procedure is active.
>>>>> >
>>>>> > +               boolean FilteredDiscovery [readonly]
>>>>> > +
>>>>> > +                       Indicates that a filtered device discovery procedure is
>>>>> > +                       active.
>>>>> > +
>>>>> >                 array{string} UUIDs [readonly]
>>>>> >
>>>>> >                         List of 128-bit UUIDs that represents the available
>>>>> > --
>>>>> > 2.2.0.rc0.207.ga3a616c
>>>>>
>>>>> @Marcel, Johan: Is this API okay for you? We should probably make them
>>>>> experimental to begin with.
>>>>
>>>> For the most part I'm fine with this, especially if it's marked as
>>>> experimental. As for the relationship to StartDiscovery, I'd take a
>>>> similar approach as we have done with the mgmt API, i.e. StartDiscovery
>>>> would be just a special kind of filtered discovery without any filters
>>>> (i.e. reporting all devices).
>>>
>>> I want StartDiscovery to behave differently than
>>> StartFilteredDiscovery without any filter:
>>> 1. StartDiscovery is interleaved scan, StartFilteredDiscovery is by
>>> default doing LE only scan (BR/EDR doesn't make no point here). If
>>> StartFilteredDiscovery would do interleaved scan, it would waste half
>>> of time not reporting anything.
>>> 2. StartDiscovery is reporting RSSI changes with delta of 8dB or more,
>>> StartFilteredDiscovery will report every RSSI change it gets, to allow
>>> better proximity approximation. If StartDiscovery report every RSSI
>>> change it would break some use cases (i.e. menus showing list of
>>> devices would flicker), if StartFilteredDiscovery won't return every
>>> RSSI change it would kill my use case.
>>>
>>>
>>>>
>>>> For the case of multiple applications requesting different discoveries
>>>> at the same time we'd go with the common denominator approach, i.e.
>>>> applications must be prepared to see devices outside of the range of
>>>> filters that it has specified (the only way to avoid something like that
>>>> support is to do unicast D-Bus signals which in turn would break
>>>> "passive observer" applications). Calling StartDiscovery when there are
>>>> one or more service discoveries in progress would simply clear out the
>>>> filters and start reporting all devices.
>>>
>>> Yes, applications must be prepared to see devices outside of range.
>>> When you call StartDiscovery when StartFilteredDiscovery is in
>>> progress, filtered discoveries should be paused, and properties should
>>> be changed accordingly ("FilteredDiscovery"=false,
>>> "Discovering"=true).  This would make sure that we don't cause
>>> regressions in apps already using StartDiscovery, and new apps that
>>> use StartFilteredDiscovery might detect this change by looking at
>>> properties, and try to handle it correctly (if they're fine with less
>>> frequent RSSI updates, no proximity) or show some message and wait for
>>> other apps to stop their discovery.
>>>
>>>>
>>>> Considering the above style approach, the new property seems a bit
>>>> pointless.
>>>
>>> As I mentioned it'll be needed for apps that do StartFilteredDiscovery
>>> to check wether it's currently paused to allow other app to do it's
>>> StartDiscovery (it takes precedence over StartFilteredDiscovery).
>>>
>>>>
>>>> Since we can't any more track multiple different discoveries within the
>>>> same application (D-Bus connection) the StopDiscovery behavior is now
>>>> quite broken. The simplest way around that would be to add a discovery
>>>> instance return parameter to StartServiceDiscovery and to have a new
>>>> StopServiceDiscovery D-Bus method that'd take this as an input
>>>> parameter.
>>>
>>> Right now each application can call StartDiscovery only once, I want
>>> each application to be able to call StartDiscovery OR
>>> StartFilteredDiscovery only once. This way we are completly fine with
>>> one StopDiscovery method. If some application needs to change filter,
>>> it can always stop and restart it's scan.
>>
>> It might be good idea to allow multpile StartFilteredDiscovery calls
>> from one client, each replacing the old filter. It would still require
>> only one StopDiscovery method.
>>
>
> The 8 dB RSSI argument isn't really valid since this is something that
> we can change in the API definition. You're saying that interleaved
> scan is wasteful but that's beside the point here, since the API does
> allow passing "auto" and "bredr" as the argument and "le" just happens
> to be the default option.
>
> From an API stand point, I like Johan's suggestion better. Basically
> we treat all discovery APIs as "filtered" except StartDiscovery is
> just a special case of StartFilteredDiscovery that has 8 dB as the
> "RSSI" argument and "auto" for transport. If an app wants to use a
> lower RSSI threshold then whether or not the UI will flicker should be
> the responsibility of the UI code. Basically we should treat
> StartDiscovery as a legacy API and perhaps mark it as deprecated while
> maintaining support for it for backwards compatibility.

I think you get the meaning of RSSI parameter wrong. It's not delta,
chow much rssi must change to report device again (I think chromeos
change that internally to something smaller). It's for proximity - if
device reported RSSI is bigger than this RSSI parameter then report
it, and there's no delta - we report every RSSI change we get (around
3 times/sec).

When it comes to rest of what you write I completly argree.


>
> Within the new framework, many apps can call StartFilteredDiscovery
> with differing filters so the daemon should just adapt the filter
> based on the biggest common denominator. So if I called
> StartFilteredDiscovery with no UUIDs and someone else calls it with a
> UUID, the initial call should win. If the first app calls
> StopDiscovery then we fallback to the second app's filter, etc.

Yes, version that I have implemented right now can merge filters from
multiple clients into one filter that's send to kernel according to
rules you mentioned.
The question is what should happen when one client call
StartFilteredDiscovery multiple times. For StartDiscovery you'd get
error. Right now for StartFilteredDiscovery I proposed same thing,
However, when I started writing client that uses this new API it's
cumbersome to stop and then restart filtered discovery. It would be
much more convinient, if for one client calling StartFilteredDiscovery
multiple times, it's effective filter is the one from last request
(it'll be still merged with filters from other applications).

>>>
>>>>
>>>> Johan
>>>
>>> Jakub
>> --
>> 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
>
> Arman

Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 22:44           ` Jakub Pawlowski
@ 2015-02-23 23:07             ` Arman Uguray
  2015-02-24  0:01               ` Jakub Pawlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Arman Uguray @ 2015-02-23 23:07 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Marcel Holtmann

Hi Jakub,

> On Mon, Feb 23, 2015 at 2:44 PM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> On Mon, Feb 23, 2015 at 1:43 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Jakub, Johan,
>>
>>> On Mon, Feb 23, 2015 at 10:43 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>>> On Mon, Feb 23, 2015 at 8:41 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>>>> Hi Johan, Luiz
>>>>
>>>> On Mon, Feb 23, 2015 at 5:09 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>>>>> Hi Luiz,
>>>>>
>>>>> On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
>>>>>> > +               void StartFilteredDiscovery(dict filter)
>>>>>> > +
>>>>>> > +                       This method starts the device discovery session with
>>>>>> > +                       filtering by uuids, and rssi or pathloss value. Use
>>>>>> > +                       StopDiscovery to release the sessions acquired.
>>>>>> > +
>>>>>> > +                       Parameters that can be set in filter dictionary include
>>>>>> > +                       the following:
>>>>>> > +
>>>>>> > +                       array{string} UUIDs : filtered service UUIDs (required)
>>>>>> > +                       int16 RSSI      : RSSI threshold value (optional)
>>>>>> > +                       uint16 pathloss : Pathloss threshold value (optional)
>>>>>> > +                       string transport: type of scan to run
>>>>>> > +
>>>>>> > +                       When a device is found that advertise any UUID from
>>>>>> > +                       UUIDs, it will be reported if:
>>>>>> > +                       - pathloss and RSSI are both empty,
>>>>>> > +                       - only pathloss param is set, device advertise TX pwer,
>>>>>> > +                         and computed pathloss is less than pathloss param,
>>>>>> > +                       - only RSSI param is set, and received RSSI is higher
>>>>>> > +                         than RSSI param,
>>>>>> > +
>>>>>> > +                       transport parameter determines the type of scan:
>>>>>> > +                               "auto"  - interleaved scan
>>>>>> > +                               "bredr" - br/edr inquiry
>>>>>> > +                               "le"    - le only scan, default value
>>>>>> > +
>>>>>> > +                       This process will start creating Device objects as new
>>>>>> > +                       devices matching criteria are discovered. It will also
>>>>>> > +                       emit PropertiesChanged signal for already existing
>>>>>> > +                       Device objects, with updated RSSI value.
>>>>>> > +
>>>>>> > +                       StartDiscovery (SD) takes precedence over
>>>>>> > +                       StartFilteredDiscovery (SFD). That means that you should
>>>>>> > +                       check Discovering property before calling SFD, otherwise
>>>>>> > +                       it would fail when any SD session is in progress.
>>>>>> > +                       Calling SD when SFD is in progress pause all SFD
>>>>>> > +                       sessions, and start them back when all SD sessions are
>>>>>> > +                       finished. When filtered discovery state is changed,
>>>>>> > +                       FilteredDiscovery variable is updated accordingly.
>>>>>> > +
>>>>>> > +                       Possible errors: org.bluez.Error.NotReady
>>>>>> > +                                        org.bluez.Error.Failed
>>>>>> > +
>>>>>> >                 void StopDiscovery()
>>>>>> >
>>>>>> >                         This method will cancel any previous StartDiscovery
>>>>>> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
>>>>>> >
>>>>>> >                         Indicates that a device discovery procedure is active.
>>>>>> >
>>>>>> > +               boolean FilteredDiscovery [readonly]
>>>>>> > +
>>>>>> > +                       Indicates that a filtered device discovery procedure is
>>>>>> > +                       active.
>>>>>> > +
>>>>>> >                 array{string} UUIDs [readonly]
>>>>>> >
>>>>>> >                         List of 128-bit UUIDs that represents the available
>>>>>> > --
>>>>>> > 2.2.0.rc0.207.ga3a616c
>>>>>>
>>>>>> @Marcel, Johan: Is this API okay for you? We should probably make them
>>>>>> experimental to begin with.
>>>>>
>>>>> For the most part I'm fine with this, especially if it's marked as
>>>>> experimental. As for the relationship to StartDiscovery, I'd take a
>>>>> similar approach as we have done with the mgmt API, i.e. StartDiscovery
>>>>> would be just a special kind of filtered discovery without any filters
>>>>> (i.e. reporting all devices).
>>>>
>>>> I want StartDiscovery to behave differently than
>>>> StartFilteredDiscovery without any filter:
>>>> 1. StartDiscovery is interleaved scan, StartFilteredDiscovery is by
>>>> default doing LE only scan (BR/EDR doesn't make no point here). If
>>>> StartFilteredDiscovery would do interleaved scan, it would waste half
>>>> of time not reporting anything.
>>>> 2. StartDiscovery is reporting RSSI changes with delta of 8dB or more,
>>>> StartFilteredDiscovery will report every RSSI change it gets, to allow
>>>> better proximity approximation. If StartDiscovery report every RSSI
>>>> change it would break some use cases (i.e. menus showing list of
>>>> devices would flicker), if StartFilteredDiscovery won't return every
>>>> RSSI change it would kill my use case.
>>>>
>>>>
>>>>>
>>>>> For the case of multiple applications requesting different discoveries
>>>>> at the same time we'd go with the common denominator approach, i.e.
>>>>> applications must be prepared to see devices outside of the range of
>>>>> filters that it has specified (the only way to avoid something like that
>>>>> support is to do unicast D-Bus signals which in turn would break
>>>>> "passive observer" applications). Calling StartDiscovery when there are
>>>>> one or more service discoveries in progress would simply clear out the
>>>>> filters and start reporting all devices.
>>>>
>>>> Yes, applications must be prepared to see devices outside of range.
>>>> When you call StartDiscovery when StartFilteredDiscovery is in
>>>> progress, filtered discoveries should be paused, and properties should
>>>> be changed accordingly ("FilteredDiscovery"=false,
>>>> "Discovering"=true).  This would make sure that we don't cause
>>>> regressions in apps already using StartDiscovery, and new apps that
>>>> use StartFilteredDiscovery might detect this change by looking at
>>>> properties, and try to handle it correctly (if they're fine with less
>>>> frequent RSSI updates, no proximity) or show some message and wait for
>>>> other apps to stop their discovery.
>>>>
>>>>>
>>>>> Considering the above style approach, the new property seems a bit
>>>>> pointless.
>>>>
>>>> As I mentioned it'll be needed for apps that do StartFilteredDiscovery
>>>> to check wether it's currently paused to allow other app to do it's
>>>> StartDiscovery (it takes precedence over StartFilteredDiscovery).
>>>>
>>>>>
>>>>> Since we can't any more track multiple different discoveries within the
>>>>> same application (D-Bus connection) the StopDiscovery behavior is now
>>>>> quite broken. The simplest way around that would be to add a discovery
>>>>> instance return parameter to StartServiceDiscovery and to have a new
>>>>> StopServiceDiscovery D-Bus method that'd take this as an input
>>>>> parameter.
>>>>
>>>> Right now each application can call StartDiscovery only once, I want
>>>> each application to be able to call StartDiscovery OR
>>>> StartFilteredDiscovery only once. This way we are completly fine with
>>>> one StopDiscovery method. If some application needs to change filter,
>>>> it can always stop and restart it's scan.
>>>
>>> It might be good idea to allow multpile StartFilteredDiscovery calls
>>> from one client, each replacing the old filter. It would still require
>>> only one StopDiscovery method.
>>>
>>
>> The 8 dB RSSI argument isn't really valid since this is something that
>> we can change in the API definition. You're saying that interleaved
>> scan is wasteful but that's beside the point here, since the API does
>> allow passing "auto" and "bredr" as the argument and "le" just happens
>> to be the default option.
>>
>> From an API stand point, I like Johan's suggestion better. Basically
>> we treat all discovery APIs as "filtered" except StartDiscovery is
>> just a special case of StartFilteredDiscovery that has 8 dB as the
>> "RSSI" argument and "auto" for transport. If an app wants to use a
>> lower RSSI threshold then whether or not the UI will flicker should be
>> the responsibility of the UI code. Basically we should treat
>> StartDiscovery as a legacy API and perhaps mark it as deprecated while
>> maintaining support for it for backwards compatibility.
>
> I think you get the meaning of RSSI parameter wrong. It's not delta,
> chow much rssi must change to report device again (I think chromeos
> change that internally to something smaller). It's for proximity - if
> device reported RSSI is bigger than this RSSI parameter then report
> it, and there's no delta - we report every RSSI change we get (around
> 3 times/sec).
>

You're right, it's just delta so I was wrong there. Still, the 8 dBm
thing shouldn't dictate how the discovery API should work since it
only affects what delta threshold to use when updating the
Device1.RSSI property. So, perhaps we can add a global D-Bus property
for RSSI delta-threshold or perhaps make that a filter parameter of
StartFilteredDiscovery.

> When it comes to rest of what you write I completly argree.
>
>
>>
>> Within the new framework, many apps can call StartFilteredDiscovery
>> with differing filters so the daemon should just adapt the filter
>> based on the biggest common denominator. So if I called
>> StartFilteredDiscovery with no UUIDs and someone else calls it with a
>> UUID, the initial call should win. If the first app calls
>> StopDiscovery then we fallback to the second app's filter, etc.
>
> Yes, version that I have implemented right now can merge filters from
> multiple clients into one filter that's send to kernel according to
> rules you mentioned.
> The question is what should happen when one client call
> StartFilteredDiscovery multiple times. For StartDiscovery you'd get
> error. Right now for StartFilteredDiscovery I proposed same thing,
> However, when I started writing client that uses this new API it's
> cumbersome to stop and then restart filtered discovery. It would be
> much more convinient, if for one client calling StartFilteredDiscovery
> multiple times, it's effective filter is the one from last request
> (it'll be still merged with filters from other applications).
>

I don't see why the same application should need to call
StartFilteredDiscovery more than once without stopping discovery in
between. Then perhaps we need a different API, for instance instead of
having a StartFilteredDiscovery maybe we should stick to
StartDiscovery and instead add a "SetDiscoveryFilter" method that
manages a per-application discovery filter. So if your application
needs to reset its filter it can call this method.

>>>>
>>>>>
>>>>> Johan
>>>>
>>>> Jakub
>>> --
>>> 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
>>
>> Arman
>
> Jakub

Arman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 23:07             ` Arman Uguray
@ 2015-02-24  0:01               ` Jakub Pawlowski
  2015-02-24 10:51                 ` Johan Hedberg
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-24  0:01 UTC (permalink / raw)
  To: Arman Uguray; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Marcel Holtmann

Hi Arman,

On Mon, Feb 23, 2015 at 3:07 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Jakub,
>
>> On Mon, Feb 23, 2015 at 2:44 PM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>> On Mon, Feb 23, 2015 at 1:43 PM, Arman Uguray <armansito@chromium.org> wrote:
>>> Hi Jakub, Johan,
>>>
>>>> On Mon, Feb 23, 2015 at 10:43 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>>>> On Mon, Feb 23, 2015 at 8:41 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
>>>>> Hi Johan, Luiz
>>>>>
>>>>> On Mon, Feb 23, 2015 at 5:09 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>>>>>> Hi Luiz,
>>>>>>
>>>>>> On Mon, Feb 16, 2015, Luiz Augusto von Dentz wrote:
>>>>>>> > +               void StartFilteredDiscovery(dict filter)
>>>>>>> > +
>>>>>>> > +                       This method starts the device discovery session with
>>>>>>> > +                       filtering by uuids, and rssi or pathloss value. Use
>>>>>>> > +                       StopDiscovery to release the sessions acquired.
>>>>>>> > +
>>>>>>> > +                       Parameters that can be set in filter dictionary include
>>>>>>> > +                       the following:
>>>>>>> > +
>>>>>>> > +                       array{string} UUIDs : filtered service UUIDs (required)
>>>>>>> > +                       int16 RSSI      : RSSI threshold value (optional)
>>>>>>> > +                       uint16 pathloss : Pathloss threshold value (optional)
>>>>>>> > +                       string transport: type of scan to run
>>>>>>> > +
>>>>>>> > +                       When a device is found that advertise any UUID from
>>>>>>> > +                       UUIDs, it will be reported if:
>>>>>>> > +                       - pathloss and RSSI are both empty,
>>>>>>> > +                       - only pathloss param is set, device advertise TX pwer,
>>>>>>> > +                         and computed pathloss is less than pathloss param,
>>>>>>> > +                       - only RSSI param is set, and received RSSI is higher
>>>>>>> > +                         than RSSI param,
>>>>>>> > +
>>>>>>> > +                       transport parameter determines the type of scan:
>>>>>>> > +                               "auto"  - interleaved scan
>>>>>>> > +                               "bredr" - br/edr inquiry
>>>>>>> > +                               "le"    - le only scan, default value
>>>>>>> > +
>>>>>>> > +                       This process will start creating Device objects as new
>>>>>>> > +                       devices matching criteria are discovered. It will also
>>>>>>> > +                       emit PropertiesChanged signal for already existing
>>>>>>> > +                       Device objects, with updated RSSI value.
>>>>>>> > +
>>>>>>> > +                       StartDiscovery (SD) takes precedence over
>>>>>>> > +                       StartFilteredDiscovery (SFD). That means that you should
>>>>>>> > +                       check Discovering property before calling SFD, otherwise
>>>>>>> > +                       it would fail when any SD session is in progress.
>>>>>>> > +                       Calling SD when SFD is in progress pause all SFD
>>>>>>> > +                       sessions, and start them back when all SD sessions are
>>>>>>> > +                       finished. When filtered discovery state is changed,
>>>>>>> > +                       FilteredDiscovery variable is updated accordingly.
>>>>>>> > +
>>>>>>> > +                       Possible errors: org.bluez.Error.NotReady
>>>>>>> > +                                        org.bluez.Error.Failed
>>>>>>> > +
>>>>>>> >                 void StopDiscovery()
>>>>>>> >
>>>>>>> >                         This method will cancel any previous StartDiscovery
>>>>>>> > @@ -144,6 +188,11 @@ Properties string Address [readonly]
>>>>>>> >
>>>>>>> >                         Indicates that a device discovery procedure is active.
>>>>>>> >
>>>>>>> > +               boolean FilteredDiscovery [readonly]
>>>>>>> > +
>>>>>>> > +                       Indicates that a filtered device discovery procedure is
>>>>>>> > +                       active.
>>>>>>> > +
>>>>>>> >                 array{string} UUIDs [readonly]
>>>>>>> >
>>>>>>> >                         List of 128-bit UUIDs that represents the available
>>>>>>> > --
>>>>>>> > 2.2.0.rc0.207.ga3a616c
>>>>>>>
>>>>>>> @Marcel, Johan: Is this API okay for you? We should probably make them
>>>>>>> experimental to begin with.
>>>>>>
>>>>>> For the most part I'm fine with this, especially if it's marked as
>>>>>> experimental. As for the relationship to StartDiscovery, I'd take a
>>>>>> similar approach as we have done with the mgmt API, i.e. StartDiscovery
>>>>>> would be just a special kind of filtered discovery without any filters
>>>>>> (i.e. reporting all devices).
>>>>>
>>>>> I want StartDiscovery to behave differently than
>>>>> StartFilteredDiscovery without any filter:
>>>>> 1. StartDiscovery is interleaved scan, StartFilteredDiscovery is by
>>>>> default doing LE only scan (BR/EDR doesn't make no point here). If
>>>>> StartFilteredDiscovery would do interleaved scan, it would waste half
>>>>> of time not reporting anything.
>>>>> 2. StartDiscovery is reporting RSSI changes with delta of 8dB or more,
>>>>> StartFilteredDiscovery will report every RSSI change it gets, to allow
>>>>> better proximity approximation. If StartDiscovery report every RSSI
>>>>> change it would break some use cases (i.e. menus showing list of
>>>>> devices would flicker), if StartFilteredDiscovery won't return every
>>>>> RSSI change it would kill my use case.
>>>>>
>>>>>
>>>>>>
>>>>>> For the case of multiple applications requesting different discoveries
>>>>>> at the same time we'd go with the common denominator approach, i.e.
>>>>>> applications must be prepared to see devices outside of the range of
>>>>>> filters that it has specified (the only way to avoid something like that
>>>>>> support is to do unicast D-Bus signals which in turn would break
>>>>>> "passive observer" applications). Calling StartDiscovery when there are
>>>>>> one or more service discoveries in progress would simply clear out the
>>>>>> filters and start reporting all devices.
>>>>>
>>>>> Yes, applications must be prepared to see devices outside of range.
>>>>> When you call StartDiscovery when StartFilteredDiscovery is in
>>>>> progress, filtered discoveries should be paused, and properties should
>>>>> be changed accordingly ("FilteredDiscovery"=false,
>>>>> "Discovering"=true).  This would make sure that we don't cause
>>>>> regressions in apps already using StartDiscovery, and new apps that
>>>>> use StartFilteredDiscovery might detect this change by looking at
>>>>> properties, and try to handle it correctly (if they're fine with less
>>>>> frequent RSSI updates, no proximity) or show some message and wait for
>>>>> other apps to stop their discovery.
>>>>>
>>>>>>
>>>>>> Considering the above style approach, the new property seems a bit
>>>>>> pointless.
>>>>>
>>>>> As I mentioned it'll be needed for apps that do StartFilteredDiscovery
>>>>> to check wether it's currently paused to allow other app to do it's
>>>>> StartDiscovery (it takes precedence over StartFilteredDiscovery).
>>>>>
>>>>>>
>>>>>> Since we can't any more track multiple different discoveries within the
>>>>>> same application (D-Bus connection) the StopDiscovery behavior is now
>>>>>> quite broken. The simplest way around that would be to add a discovery
>>>>>> instance return parameter to StartServiceDiscovery and to have a new
>>>>>> StopServiceDiscovery D-Bus method that'd take this as an input
>>>>>> parameter.
>>>>>
>>>>> Right now each application can call StartDiscovery only once, I want
>>>>> each application to be able to call StartDiscovery OR
>>>>> StartFilteredDiscovery only once. This way we are completly fine with
>>>>> one StopDiscovery method. If some application needs to change filter,
>>>>> it can always stop and restart it's scan.
>>>>
>>>> It might be good idea to allow multpile StartFilteredDiscovery calls
>>>> from one client, each replacing the old filter. It would still require
>>>> only one StopDiscovery method.
>>>>
>>>
>>> The 8 dB RSSI argument isn't really valid since this is something that
>>> we can change in the API definition. You're saying that interleaved
>>> scan is wasteful but that's beside the point here, since the API does
>>> allow passing "auto" and "bredr" as the argument and "le" just happens
>>> to be the default option.
>>>
>>> From an API stand point, I like Johan's suggestion better. Basically
>>> we treat all discovery APIs as "filtered" except StartDiscovery is
>>> just a special case of StartFilteredDiscovery that has 8 dB as the
>>> "RSSI" argument and "auto" for transport. If an app wants to use a
>>> lower RSSI threshold then whether or not the UI will flicker should be
>>> the responsibility of the UI code. Basically we should treat
>>> StartDiscovery as a legacy API and perhaps mark it as deprecated while
>>> maintaining support for it for backwards compatibility.
>>
>> I think you get the meaning of RSSI parameter wrong. It's not delta,
>> chow much rssi must change to report device again (I think chromeos
>> change that internally to something smaller). It's for proximity - if
>> device reported RSSI is bigger than this RSSI parameter then report
>> it, and there's no delta - we report every RSSI change we get (around
>> 3 times/sec).
>>
>
> You're right, it's just delta so I was wrong there. Still, the 8 dBm
> thing shouldn't dictate how the discovery API should work since it
> only affects what delta threshold to use when updating the
> Device1.RSSI property. So, perhaps we can add a global D-Bus property
> for RSSI delta-threshold or perhaps make that a filter parameter of
> StartFilteredDiscovery.
>

If we add global property for changing RSSI delta-threshold we would
have to define api to change and probably also negotiate RSSI when
multiple apps require different values.
Putting it into StartFilteredDiscovery parameter seems more
reasonable, but do we really need that ?
Can't we just have no RSSI delta for filtered scan, and let apps
decide on their own in their logic ?


>> When it comes to rest of what you write I completly argree.
>>
>>
>>>
>>> Within the new framework, many apps can call StartFilteredDiscovery
>>> with differing filters so the daemon should just adapt the filter
>>> based on the biggest common denominator. So if I called
>>> StartFilteredDiscovery with no UUIDs and someone else calls it with a
>>> UUID, the initial call should win. If the first app calls
>>> StopDiscovery then we fallback to the second app's filter, etc.
>>
>> Yes, version that I have implemented right now can merge filters from
>> multiple clients into one filter that's send to kernel according to
>> rules you mentioned.
>> The question is what should happen when one client call
>> StartFilteredDiscovery multiple times. For StartDiscovery you'd get
>> error. Right now for StartFilteredDiscovery I proposed same thing,
>> However, when I started writing client that uses this new API it's
>> cumbersome to stop and then restart filtered discovery. It would be
>> much more convinient, if for one client calling StartFilteredDiscovery
>> multiple times, it's effective filter is the one from last request
>> (it'll be still merged with filters from other applications).
>>
>
> I don't see why the same application should need to call
> StartFilteredDiscovery more than once without stopping discovery in
> between. Then perhaps we need a different API, for instance instead of
> having a StartFilteredDiscovery maybe we should stick to
> StartDiscovery and instead add a "SetDiscoveryFilter" method that
> manages a per-application discovery filter. So if your application
> needs to reset its filter it can call this method.

Client would call StartFilteredDiscovery when it needs to start
filtered discovery, or change it's filter, i.e. add new uuid to it. It
can do StopDiscovery and then StartFilteredDiscovery with update
filter, but just for convinience I wanted to be able to call
StartFilteredDiscovery multiple times. I don't think it's worth
splitting it to StartDiscovery and SetDiscoveryFilter.


>>>>>
>>>>>>
>>>>>> Johan
>>>>>
>>>>> Jakub
>>>> --
>>>> 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
>>>
>>> Arman
>>
>> Jakub
>
> Arman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-23 16:41     ` Jakub Pawlowski
  2015-02-23 18:43       ` Jakub Pawlowski
@ 2015-02-24 10:37       ` Johan Hedberg
  2015-02-24 20:23         ` Jakub Pawlowski
  1 sibling, 1 reply; 15+ messages in thread
From: Johan Hedberg @ 2015-02-24 10:37 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Marcel Holtmann

Hi Jakub,

On Mon, Feb 23, 2015, Jakub Pawlowski wrote:
> > Since we can't any more track multiple different discoveries within the
> > same application (D-Bus connection) the StopDiscovery behavior is now
> > quite broken. The simplest way around that would be to add a discovery
> > instance return parameter to StartServiceDiscovery and to have a new
> > StopServiceDiscovery D-Bus method that'd take this as an input
> > parameter.
> 
> Right now each application can call StartDiscovery only once, I want
> each application to be able to call StartDiscovery OR
> StartFilteredDiscovery only once. This way we are completly fine with
> one StopDiscovery method. If some application needs to change filter,
> it can always stop and restart it's scan.

Actually I remembered wrong how the current code works. I thought it was
allowing multiple StartDiscovery() per app, but there's indeed a
hard-coded limit of just one. Keeping this policy around would allow
StopDiscovery to be used for StartDiscovery and StartServiceDiscovery
alike, and each app would then only be allowed to call one of those at a
time.

Johan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-24  0:01               ` Jakub Pawlowski
@ 2015-02-24 10:51                 ` Johan Hedberg
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2015-02-24 10:51 UTC (permalink / raw)
  To: Jakub Pawlowski
  Cc: Arman Uguray, Luiz Augusto von Dentz, linux-bluetooth, Marcel Holtmann

Hi Jakub,

On Mon, Feb 23, 2015, Jakub Pawlowski wrote:
>>>> From an API stand point, I like Johan's suggestion better. Basically
>>>> we treat all discovery APIs as "filtered" except StartDiscovery is
>>>> just a special case of StartFilteredDiscovery that has 8 dB as the
>>>> "RSSI" argument and "auto" for transport. If an app wants to use a
>>>> lower RSSI threshold then whether or not the UI will flicker should be
>>>> the responsibility of the UI code. Basically we should treat
>>>> StartDiscovery as a legacy API and perhaps mark it as deprecated while
>>>> maintaining support for it for backwards compatibility.
>>>
>>> I think you get the meaning of RSSI parameter wrong. It's not delta,
>>> chow much rssi must change to report device again (I think chromeos
>>> change that internally to something smaller). It's for proximity - if
>>> device reported RSSI is bigger than this RSSI parameter then report
>>> it, and there's no delta - we report every RSSI change we get (around
>>> 3 times/sec).
>>
>> You're right, it's just delta so I was wrong there. Still, the 8 dBm
>> thing shouldn't dictate how the discovery API should work since it
>> only affects what delta threshold to use when updating the
>> Device1.RSSI property. So, perhaps we can add a global D-Bus property
>> for RSSI delta-threshold or perhaps make that a filter parameter of
>> StartFilteredDiscovery.
>
> If we add global property for changing RSSI delta-threshold we would
> have to define api to change and probably also negotiate RSSI when
> multiple apps require different values.
> Putting it into StartFilteredDiscovery parameter seems more
> reasonable, but do we really need that ?
> Can't we just have no RSSI delta for filtered scan, and let apps
> decide on their own in their logic ?

If something like this was exposed through D-Bus then a parameter to
StartFilteredDiscovery would be the way to go, I think. However, until
we have a clear need for it we can simply keep track of this internally.
I.e. StartDiscovery keeps imposing the delta-threshold but as soon as
there's one or more StartFilteredDiscovery active this type of filtering
is removed.

Johan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-24 10:37       ` Johan Hedberg
@ 2015-02-24 20:23         ` Jakub Pawlowski
  2015-02-24 22:50           ` Marcel Holtmann
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-24 20:23 UTC (permalink / raw)
  To: Jakub Pawlowski, Luiz Augusto von Dentz, linux-bluetooth,
	Marcel Holtmann, Arman Uguray

Hi Johan,

On Tue, Feb 24, 2015 at 2:37 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Jakub,
>
> On Mon, Feb 23, 2015, Jakub Pawlowski wrote:
>> > Since we can't any more track multiple different discoveries within the
>> > same application (D-Bus connection) the StopDiscovery behavior is now
>> > quite broken. The simplest way around that would be to add a discovery
>> > instance return parameter to StartServiceDiscovery and to have a new
>> > StopServiceDiscovery D-Bus method that'd take this as an input
>> > parameter.
>>
>> Right now each application can call StartDiscovery only once, I want
>> each application to be able to call StartDiscovery OR
>> StartFilteredDiscovery only once. This way we are completly fine with
>> one StopDiscovery method. If some application needs to change filter,
>> it can always stop and restart it's scan.
>
> Actually I remembered wrong how the current code works. I thought it was
> allowing multiple StartDiscovery() per app, but there's indeed a
> hard-coded limit of just one. Keeping this policy around would allow
> StopDiscovery to be used for StartDiscovery and StartServiceDiscovery
> alike, and each app would then only be allowed to call one of those at a
> time.
>

So I want to keep the limit of one session per app, but I think that
app should be able to call StartServiceDiscovery multiple times, in
order to update it's filter without calling StopDiscovery to first
stop it's discovery. That would simplify filter updating for apps that
would have such need, and make their logic simpler.

Example: App wants to update it's filter. If It can't call
StartServiceDiscovery more than once, here are steps:
1. app call StopDiscovery.
2. Discovering property changed to false.
3. It calls StartFilteredDiscovery with updated filter.
4. Discovering property changed to true

If calling StartFilteredDiscovery more than once is possible:
1. app call StartServiceDiscovery with updated filter, filter is
updated internally and "Discovering" property is not updated at all.


I think I argree with all other comments from other emails. I'll
update documentation accordingly.
I'll say in doc, that StartServiceDiscovery might be called multiple
times, and keep only one session. If anyone doesn't argree with that
I'll remove that, as that's only for convinience, and doesn't really
affect functionality.


> Johan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-24 20:23         ` Jakub Pawlowski
@ 2015-02-24 22:50           ` Marcel Holtmann
  2015-02-24 23:12             ` Jakub Pawlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Marcel Holtmann @ 2015-02-24 22:50 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Arman Uguray

Hi Jakub,

>>>> Since we can't any more track multiple different discoveries within the
>>>> same application (D-Bus connection) the StopDiscovery behavior is now
>>>> quite broken. The simplest way around that would be to add a discovery
>>>> instance return parameter to StartServiceDiscovery and to have a new
>>>> StopServiceDiscovery D-Bus method that'd take this as an input
>>>> parameter.
>>> 
>>> Right now each application can call StartDiscovery only once, I want
>>> each application to be able to call StartDiscovery OR
>>> StartFilteredDiscovery only once. This way we are completly fine with
>>> one StopDiscovery method. If some application needs to change filter,
>>> it can always stop and restart it's scan.
>> 
>> Actually I remembered wrong how the current code works. I thought it was
>> allowing multiple StartDiscovery() per app, but there's indeed a
>> hard-coded limit of just one. Keeping this policy around would allow
>> StopDiscovery to be used for StartDiscovery and StartServiceDiscovery
>> alike, and each app would then only be allowed to call one of those at a
>> time.
>> 
> 
> So I want to keep the limit of one session per app, but I think that
> app should be able to call StartServiceDiscovery multiple times, in
> order to update it's filter without calling StopDiscovery to first
> stop it's discovery. That would simplify filter updating for apps that
> would have such need, and make their logic simpler.

we have never done such an API design before. I am a bit reluctant to allow that now. The whole API design is falling into one simple approach that the name of method calls and properties is pretty self explanatory. That StartServiceDiscovery means also additionally UpdateServiceDiscovery is not something I would consider intuitive.

> Example: App wants to update it's filter. If It can't call
> StartServiceDiscovery more than once, here are steps:
> 1. app call StopDiscovery.
> 2. Discovering property changed to false.
> 3. It calls StartFilteredDiscovery with updated filter.
> 4. Discovering property changed to true
> 
> If calling StartFilteredDiscovery more than once is possible:
> 1. app call StartServiceDiscovery with updated filter, filter is
> updated internally and "Discovering" property is not updated at all.
> 
> 
> I think I argree with all other comments from other emails. I'll
> update documentation accordingly.
> I'll say in doc, that StartServiceDiscovery might be called multiple
> times, and keep only one session. If anyone doesn't argree with that
> I'll remove that, as that's only for convinience, and doesn't really
> affect functionality.

While I just looked at this for only a few minutes, the double execution of StartServiceDiscovery is something that does not fell right. Why not introduce UpdateServiceDiscovery or ChangeServiceDiscovery.

Regards

Marcel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method.
  2015-02-24 22:50           ` Marcel Holtmann
@ 2015-02-24 23:12             ` Jakub Pawlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Jakub Pawlowski @ 2015-02-24 23:12 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Arman Uguray

On Tue, Feb 24, 2015 at 2:50 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Jakub,
>
>>>>> Since we can't any more track multiple different discoveries within t=
he
>>>>> same application (D-Bus connection) the StopDiscovery behavior is now
>>>>> quite broken. The simplest way around that would be to add a discover=
y
>>>>> instance return parameter to StartServiceDiscovery and to have a new
>>>>> StopServiceDiscovery D-Bus method that'd take this as an input
>>>>> parameter.
>>>>
>>>> Right now each application can call StartDiscovery only once, I want
>>>> each application to be able to call StartDiscovery OR
>>>> StartFilteredDiscovery only once. This way we are completly fine with
>>>> one StopDiscovery method. If some application needs to change filter,
>>>> it can always stop and restart it's scan.
>>>
>>> Actually I remembered wrong how the current code works. I thought it wa=
s
>>> allowing multiple StartDiscovery() per app, but there's indeed a
>>> hard-coded limit of just one. Keeping this policy around would allow
>>> StopDiscovery to be used for StartDiscovery and StartServiceDiscovery
>>> alike, and each app would then only be allowed to call one of those at =
a
>>> time.
>>>
>>
>> So I want to keep the limit of one session per app, but I think that
>> app should be able to call StartServiceDiscovery multiple times, in
>> order to update it's filter without calling StopDiscovery to first
>> stop it's discovery. That would simplify filter updating for apps that
>> would have such need, and make their logic simpler.
>
> we have never done such an API design before. I am a bit reluctant to all=
ow that now. The whole API design is falling into one simple approach that =
the name of method calls and properties is pretty self explanatory. That St=
artServiceDiscovery means also additionally UpdateServiceDiscovery is not s=
omething I would consider intuitive.
>
>> Example: App wants to update it's filter. If It can't call
>> StartServiceDiscovery more than once, here are steps:
>> 1. app call StopDiscovery.
>> 2. Discovering property changed to false.
>> 3. It calls StartFilteredDiscovery with updated filter.
>> 4. Discovering property changed to true
>>
>> If calling StartFilteredDiscovery more than once is possible:
>> 1. app call StartServiceDiscovery with updated filter, filter is
>> updated internally and "Discovering" property is not updated at all.
>>
>>
>> I think I argree with all other comments from other emails. I'll
>> update documentation accordingly.
>> I'll say in doc, that StartServiceDiscovery might be called multiple
>> times, and keep only one session. If anyone doesn't argree with that
>> I'll remove that, as that's only for convinience, and doesn't really
>> affect functionality.
>
> While I just looked at this for only a few minutes, the double execution =
of StartServiceDiscovery is something that does not fell right. Why not int=
roduce UpdateServiceDiscovery or ChangeServiceDiscovery.

Ok, I'll make it so you can call StartFilteredDiscovery only once. If
you want to change filter just stop and start the filtered scan again.

>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-02-24 23:12 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-10  0:34 [PATCH BlueZ v5] doc/adapter-api.txt: StartFilteredDiscovery method Jakub Pawlowski
2015-02-16  8:46 ` Luiz Augusto von Dentz
2015-02-20 17:09   ` Jakub Pawlowski
2015-02-23 13:09   ` Johan Hedberg
2015-02-23 16:41     ` Jakub Pawlowski
2015-02-23 18:43       ` Jakub Pawlowski
2015-02-23 21:43         ` Arman Uguray
2015-02-23 22:44           ` Jakub Pawlowski
2015-02-23 23:07             ` Arman Uguray
2015-02-24  0:01               ` Jakub Pawlowski
2015-02-24 10:51                 ` Johan Hedberg
2015-02-24 10:37       ` Johan Hedberg
2015-02-24 20:23         ` Jakub Pawlowski
2015-02-24 22:50           ` Marcel Holtmann
2015-02-24 23:12             ` Jakub Pawlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.