[PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Arman Uguray]

Currently the daemon does not update the RSSI property of a device object unless
it sees a change of at least 8 dBm. This is too large for proximity use cases
involving changes in distance within 4-5 feet. This patch reduces the threshold
to 2 dBm to address this.
---
 src/device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index 875a5c5..821549e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4094,8 +4094,8 @@ void device_set_rssi(struct btd_device *device, int8_t rssi)
 		else
 			delta = rssi - device->rssi;
 
-		/* only report changes of 8 dBm or more */
-		if (delta < 8)
+		/* only report changes of 2 dBm or more */
+		if (delta < 2)
 			return;
 
 		DBG("rssi %d delta %d", rssi, delta);
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Szymon Janc]

Hi Arman,

On Monday 29 of September 2014 15:16:04 Arman Uguray wrote:
> Currently the daemon does not update the RSSI property of a device object unless
> it sees a change of at least 8 dBm. This is too large for proximity use cases
> involving changes in distance within 4-5 feet. This patch reduces the threshold
> to 2 dBm to address this.
> ---
>  src/device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/device.c b/src/device.c
> index 875a5c5..821549e 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4094,8 +4094,8 @@ void device_set_rssi(struct btd_device *device, int8_t rssi)
>  		else
>  			delta = rssi - device->rssi;
>  
> -		/* only report changes of 8 dBm or more */
> -		if (delta < 8)
> +		/* only report changes of 2 dBm or more */
> +		if (delta < 2)
>  			return;

I'm not sure if having this so small is good... ie those few devices I have
they tend to have ~4 dBm RSSI oscillation when just lying next to each other
on the desk.
  
>  		DBG("rssi %d delta %d", rssi, delta);
> 

-- 
Best regards, 
Szymon Janc

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Arman Uguray]

Hi Szymon,


> I'm not sure if having this so small is good... ie those few devices I have
> they tend to have ~4 dBm RSSI oscillation when just lying next to each other
> on the desk.
>
>>               DBG("rssi %d delta %d", rssi, delta);
>>

Yeah, I'm not exactly sure if 2 is the right value to use here, though
8 is definitely too large. I basically want to be able to detect a
user walking away / coming back within a short distance and I saw
variations around 4-5 dBm there. Would 4 be a better threshold?

-Arman

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Marcel Holtmann]

Hi Arman,

>> I'm not sure if having this so small is good... ie those few devices I have
>> they tend to have ~4 dBm RSSI oscillation when just lying next to each other
>> on the desk.
>> 
>>>              DBG("rssi %d delta %d", rssi, delta);
>>> 
> 
> Yeah, I'm not exactly sure if 2 is the right value to use here, though
> 8 is definitely too large. I basically want to be able to detect a
> user walking away / coming back within a short distance and I saw
> variations around 4-5 dBm there. Would 4 be a better threshold?

the reason why this threshold exists in the first place is that when you show the pairing list in your basic UI that the entries not keep jumping around like crazy. When we added this code 8 dBm made sense for that. That allows that you select an entry in the devices list successfully by touch or mouse. Since sorting close by devices at the top makes more sense than doing things alphabetical.

Regards

Marcel

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Arman Uguray]

Hi Marcel,


> the reason why this threshold exists in the first place is that when you =
show the pairing list in your basic UI that the entries not keep jumping ar=
ound like crazy. When we added this code 8 dBm made sense for that. That al=
lows that you select an entry in the devices list successfully by touch or =
mouse. Since sorting close by devices at the top makes more sense than doin=
g things alphabetical.
>

Looks like we order them according to the order in which they were
discovered in Chrome OS :P Aside from that though, isn't it a bit
strange to set this threshold based on UI a use case? If I'm building
a UI around this and entries are jumping around like crazy, then this
seems like a UI problem and not a bluetooth daemon problem and I can
go and set a new threshold myself in the UI code.

-Arman

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Marcel Holtmann]

Hi Arman,

>> the reason why this threshold exists in the first place is that when you show the pairing list in your basic UI that the entries not keep jumping around like crazy. When we added this code 8 dBm made sense for that. That allows that you select an entry in the devices list successfully by touch or mouse. Since sorting close by devices at the top makes more sense than doing things alphabetical.
>> 
> 
> Looks like we order them according to the order in which they were
> discovered in Chrome OS :P Aside from that though, isn't it a bit
> strange to set this threshold based on UI a use case? If I'm building
> a UI around this and entries are jumping around like crazy, then this
> seems like a UI problem and not a bluetooth daemon problem and I can
> go and set a new threshold myself in the UI code.

the D-Bus API have all been designed with user interaction in mind. So it makes sense to impose this kind of extra thresholds. That is why you can map D-Bus things 1:1 to your UI and it will all nicely work.

Regards

Marcel

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Arman Uguray]

Hi Marcel,

> the D-Bus API have all been designed with user interaction in mind. So it makes sense to impose this kind of extra thresholds. That is why you can map D-Bus things 1:1 to your UI and it will all nicely work.
>

I guess this falls into the overall question of designing an
application API vs an API for system UI. I always look at the D-Bus
API as an application API where system UI is one specific application.
In the end, we should be able to build use cases that require more
fine-tuned RSSI updates.

I don't know if the RSSI property of Device1 is the right thing to use
for this, though I still find it strange that it's hardwired to serve
system UI only.

In any case, I'm open to suggestions. Could we perhaps introduce a new
API for RSSI updates? Or a new property maybe? Perhaps we could expose
a property that sets the RSSI update threshold? Something like:

   int16 RSSIUpdateThreshold  [readwrite]

        Defines the minimum amount of change required (in dBm) for the
RSSI property of a device
        to be updated. Set to 8 dBm by default.

And this would be a property on Adapter1. I'm mostly thinking out loud
here though, so this may not be the best answer.

Cheers,
Arman

Re: [PATCH BlueZ] core: Reduce RSSI delta threshold for RSSI property updates on Device1.

[Marcel Holtmann]

Hi Arman,

>> the D-Bus API have all been designed with user interaction in mind. So it makes sense to impose this kind of extra thresholds. That is why you can map D-Bus things 1:1 to your UI and it will all nicely work.
>> 
> 
> I guess this falls into the overall question of designing an
> application API vs an API for system UI. I always look at the D-Bus
> API as an application API where system UI is one specific application.
> In the end, we should be able to build use cases that require more
> fine-tuned RSSI updates.

the current API was designed for UI frameworks to have an easy and dirt simple way of displaying the core Bluetooth functionality to the user. So this mainly relates to pairing your device and getting extra information about the device. The RSSI property is also only valid if you start a discovery. Which we clearly expected to map 1:1 to someone wanting to display a list of devices and then be able to sort them by approximate distance.

That said, I agree that we should enable applications that want to monitor in range devices. And realistically that is only really possible since Bluetooth LE came into play. With BR/EDR that was extremely power hungry operation even if you used Periodic Inquiry.

> I don't know if the RSSI property of Device1 is the right thing to use
> for this, though I still find it strange that it's hardwired to serve
> system UI only.

Actually on the D-Bus level exposing RSSI is most likely a really bad idea since it has too many controller specific factors. Using pathloss is something that allows for a way better estimation of distance. Just finding devices that include the extra TX Power information on BR/EDR is almost zero. Especially since there are still some devices around that are 2.0 only and these are mainly keyboards and mice. So we opted for RSSI since that is better than nothing. And I am big believer that you should show the most closes devices in the top of your list.

> In any case, I'm open to suggestions. Could we perhaps introduce a new
> API for RSSI updates? Or a new property maybe? Perhaps we could expose
> a property that sets the RSSI update threshold? Something like:
> 
>   int16 RSSIUpdateThreshold  [readwrite]
> 
>        Defines the minimum amount of change required (in dBm) for the
> RSSI property of a device
>        to be updated. Set to 8 dBm by default.
> 
> And this would be a property on Adapter1. I'm mostly thinking out loud
> here though, so this may not be the best answer.

Or we can start smaller and make this a main.conf configurable option.

In addition we really need specific APIs for certain discovery tasks. In a lot of cases you are looking for a specific set of devices. I think that I mentioned this to Scott before. Just tell the daemon what you are looking for and it will scan for you.

The Start Discovery D-Bus API is also the big hammer. It means that it takes over every other operation since it feeds into the Start Discovery mgmt API. If you use it, you block everything else that might be ongoing. That is why I am saying it is tied to UI. Since when you use it, then you expect something visible for the user to happen right away. For this means we have to abort everything else we are doing.

If we say for example an application wants to be notified about xyz devices in range, then it should not block the attempt of a HID device connecting. However the Start Discovery hammer will do exactly that. You should only bring the big tools if you expect immediate feedback to the user.

This goes along the saying, if you only have a hammer, then everything is a nail. That is pretty dangerous approach and will lead down the wrong path. So instead of forcing existing APIs into a new usage model or hacking them to make fit, I prefer that we introduce the appropriate APIs.

On the D-Bus level we are extremely flexible. We can extend the current Adapter1 interface with new methods and signals or we just introduce a new interface.

Regards

Marcel