Hi Jakub,
> On Thu, Mar 5, 2015 at 11:38 AM, Jakub Pawlowski <jpawlowski@xxxxxxxxxx> wrote:
> This patch proposes new method, SetDiscoveryFilter to D-Bus Adapter
> API for desktop bluetoothd. It will allow to set per-client discovery
> filter that would limit devices being discovered.
>
I think this looks good overall. See my comments below; I mostly
commented on grammar and phrasing.
> Signed-off-by: Jakub Pawlowski <jpawlowski@xxxxxxxxxx>
> ---
> doc/adapter-api.txt | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 74d235a..528f0ba 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -43,6 +43,65 @@ Methods void StartDiscovery()
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.Failed
>
> + void SetDiscoveryFilter(dict filter) [Experimental]
> +
> + This method sets the device discovery filter for
> + current discovery session. When this method is called
> + with no filter parameter, filter is removed.
> +
Say "This method sets the device discovery filter for the caller" to
make it clear that there is an individual filter for each application.
> + Parameters that can be set in filter dictionary include
> + the following:
> +
nit: "...set in the filter dictionary..."
> + 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
nit: s/device/remote device/
nit: s/advertise/advertises/
> + 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
> +
> + If "le" or "bredr" transport is requested, and device
> + doesn't support it, org.bluez.Error.Failed error will
by "device doesn't support it" I think you mean "the controller
doesn't support it". Let's not say "device" since we've been using it
to refer to remote devices. Use "adapter"/"controller" to refer to the
local system to avoid ambiguities.
> + be returned. If "auto" transport is requested, scan
> + will use LE, BREDR, or both, depending on what's
> + currently enabled on controller.
nit: s/controller/the controller/
> +
> + When discovery filter is set, Device objects will be
> + created as new devices matching criteria are
nit: s/devices matching criteria/devices with matching criteria/
> + discovered. It will also emit PropertiesChanged signal
nit: "PropertiesChanged signals will be emitted"
> + for already existing Device objects, with updated RSSI
> + value. This is different from plain StartDiscovery
> + behaviour, that keeps imposing the delta-threshold. As
> + soon as there's one or more filters set, this type of
> + filtering is removed.
The documentation for StartDiscovery doesn't mention any threshold so
this sentence here will be confusing for someone who is new to the
code base. I would add a line to StartDiscovery that mentions the
current delta threshold and rephrase the last two sentences, probably
in a new paragraph: "If one or more discovery filters have been set,
the RSSI delta-threshold, that is imposed by StartDiscovery by
default, will not be applied."
> +
> + When multiple clients call StartDiscoveryFilter, their
nit: s/StartDiscoveryFilter/SetDiscoveryFilter/
> + filters are internally merged, and notifications about
> + new devices are sent to all clients. Therfore every
nit: s/Therfore/Therefore/
> + client must check wether Devices that he was notificed
nit: s/wether/whether/. Also, who is "he" :P? Rephrase: "Therefore,
each client must check that device updates actually match its filter."
> + about are matching it's filter.
> +
> + When SetDiscoveryFilter is called multiple times, last
> + filter passed will be active for given client.
Say "When SetDiscoveryFilter is called multiple times by the same
client", since I think that's what you're referring to here.
> +
> + SetDiscoveryFilter can be called before StartDiscovery.
> + It is useful when client will create first discovery
> + session, to ensure that proper scan will be started
> + right after call to StartDiscovery.
> +
> + Possible errors: org.bluez.Error.NotReady
> + org.bluez.Error.Failed
> +
> Properties string Address [readonly]
>
> The Bluetooth device address.
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
Arman
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
