linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
@ 2020-05-21 14:57 Alain Michaud
  2020-05-21 16:26 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Alain Michaud @ 2020-05-21 14:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Alain Michaud

This change adds the definition for the load default parameter command.
In particular, this command is used to load default parameters for
various operations in the kernel. This mechanism is also expandable to
future values that may be necessary.

This will allow bluetoothd to load parameters from a conf file that may
be customized for the specific requirements of each platforms.

---
rebase against current master

 doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 6ee01fed8..c6575e654 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -3223,6 +3223,68 @@ Set Experimental Feature Command
 				Invalid Index
 
 
+Load Default Controller Parameter Command
+=============================
+
+	Command Code:		0x004b
+	Controller Index:	<controller id>
+	Command Parameters:	Parameter_Count (2 Octets)
+				Parameter1 {
+					Parameter_Type (2 Octet)
+					Value_Length (1 Octet)
+					Value (0-255 Octets)
+				}
+				Parameter2 { }
+				...
+	Return Parameters:
+
+	This command is used to feed the kernel a list of default controller
+	parameters.
+
+	Currently defined Parameter_Type values are:
+
+		0x0000	BR/EDR Page Scan Type
+		0x0001	BR/EDR Page Scan Interval
+		0x0002	BR/EDR Page Scan Window
+		0x0003	BR/EDR Inquiry Scan Type
+		0x0004	BR/EDR Inquiry Scan Interval
+		0x0005	BR/EDR Inquiry Scan Window
+		0x0006	BR/EDR Link Supervision Timeout
+		0x0007	BR/EDR Page Timeout
+		0x0008	BR/EDR Min Sniff Interval
+		0x0009	BR/EDR Max Sniff Interval
+		0x000a	LE Advertisement Min Interval
+		0x000b	LE Advertisement Max Interval
+		0x000c	LE Multi Advertisement Rotation Interval
+		0x000d	LE Scanning Interval for auto connect
+		0x000e	LE Scanning Window for auto connect
+		0x000f	LE Scanning Interval for HID only
+		0x0010	LE Scanning Window for HID only
+		0x0012	LE Scanning Interval for wake scenarios
+		0x0013	LE Scanning Window for wake scenarios
+		0x0014	LE Scanning Interval for discovery
+		0x0015	LE Scanning Window for discovery
+		0x0016	LE Scanning Interval for adv monitoring
+		0x0017	LE Scanning Window for adv monitoring
+		0x0018	LE Scanning Interval for connect
+		0x0019	LE Scanning Window for connect
+		0x001a	LE Min Connection Interval
+		0x001b	LE Max Connection Interval
+		0x001c	LE Connection Connection Latency
+		0x001d	LE Connection Supervision Timeout
+
+	This command can be used when the controller is not powered and
+	all settings will be programmed once powered.  Note that these only
+	control the default parameters.  Higher level Apis may influence the
+	effective value used.
+
+	This command generates a Command Complete event on success or
+	a Command Status event on failure.
+
+	Possible errors:	Invalid Parameters
+				Invalid Index
+
+
 Command Complete Event
 ======================
 
-- 
2.26.2.761.g0e0b3e54be-goog


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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
  2020-05-21 14:57 [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op Alain Michaud
@ 2020-05-21 16:26 ` Luiz Augusto von Dentz
       [not found]   ` <CALWDO_UQhU5QxZ1eRNWZtAtkcghWEBM5eVeujxhROSr7PG_P=w@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-05-21 16:26 UTC (permalink / raw)
  To: Alain Michaud; +Cc: linux-bluetooth

Hi Alain,

On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
>
> This change adds the definition for the load default parameter command.
> In particular, this command is used to load default parameters for
> various operations in the kernel. This mechanism is also expandable to
> future values that may be necessary.
>
> This will allow bluetoothd to load parameters from a conf file that may
> be customized for the specific requirements of each platforms.
>
> ---
> rebase against current master
>
>  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 6ee01fed8..c6575e654 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
>                                 Invalid Index
>
>
> +Load Default Controller Parameter Command
> +=============================
> +
> +       Command Code:           0x004b
> +       Controller Index:       <controller id>
> +       Command Parameters:     Parameter_Count (2 Octets)
> +                               Parameter1 {
> +                                       Parameter_Type (2 Octet)
> +                                       Value_Length (1 Octet)
> +                                       Value (0-255 Octets)
> +                               }
> +                               Parameter2 { }
> +                               ...
> +       Return Parameters:
> +
> +       This command is used to feed the kernel a list of default controller
> +       parameters.
> +
> +       Currently defined Parameter_Type values are:
> +
> +               0x0000  BR/EDR Page Scan Type
> +               0x0001  BR/EDR Page Scan Interval
> +               0x0002  BR/EDR Page Scan Window
> +               0x0003  BR/EDR Inquiry Scan Type
> +               0x0004  BR/EDR Inquiry Scan Interval
> +               0x0005  BR/EDR Inquiry Scan Window
> +               0x0006  BR/EDR Link Supervision Timeout
> +               0x0007  BR/EDR Page Timeout
> +               0x0008  BR/EDR Min Sniff Interval
> +               0x0009  BR/EDR Max Sniff Interval
> +               0x000a  LE Advertisement Min Interval
> +               0x000b  LE Advertisement Max Interval
> +               0x000c  LE Multi Advertisement Rotation Interval
> +               0x000d  LE Scanning Interval for auto connect
> +               0x000e  LE Scanning Window for auto connect
> +               0x000f  LE Scanning Interval for HID only
> +               0x0010  LE Scanning Window for HID only

I remember commenting that we don't have profile information on the
kernel so Im not sure how you are planning to detect the when the
device is HID or not?

> +               0x0012  LE Scanning Interval for wake scenarios
> +               0x0013  LE Scanning Window for wake scenarios
> +               0x0014  LE Scanning Interval for discovery
> +               0x0015  LE Scanning Window for discovery
> +               0x0016  LE Scanning Interval for adv monitoring
> +               0x0017  LE Scanning Window for adv monitoring
> +               0x0018  LE Scanning Interval for connect
> +               0x0019  LE Scanning Window for connect
> +               0x001a  LE Min Connection Interval
> +               0x001b  LE Max Connection Interval
> +               0x001c  LE Connection Connection Latency
> +               0x001d  LE Connection Supervision Timeout
> +
> +       This command can be used when the controller is not powered and
> +       all settings will be programmed once powered.  Note that these only
> +       control the default parameters.  Higher level Apis may influence the
> +       effective value used.
> +
> +       This command generates a Command Complete event on success or
> +       a Command Status event on failure.
> +
> +       Possible errors:        Invalid Parameters
> +                               Invalid Index
> +
> +
>  Command Complete Event
>  ======================
>
> --
> 2.26.2.761.g0e0b3e54be-goog
>


-- 
Luiz Augusto von Dentz

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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
       [not found]   ` <CALWDO_UQhU5QxZ1eRNWZtAtkcghWEBM5eVeujxhROSr7PG_P=w@mail.gmail.com>
@ 2020-05-21 22:05     ` Luiz Augusto von Dentz
  2020-05-21 22:27       ` Alain Michaud
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-05-21 22:05 UTC (permalink / raw)
  To: Alain Michaud; +Cc: Alain Michaud, linux-bluetooth

Hi Alain,

On Thu, May 21, 2020 at 1:12 PM Alain Michaud <alainmichaud@google.com> wrote:
>
> Hi Luiz,
>
>
> On Thu, May 21, 2020 at 12:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi Alain,
>>
>> On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
>> >
>> > This change adds the definition for the load default parameter command.
>> > In particular, this command is used to load default parameters for
>> > various operations in the kernel. This mechanism is also expandable to
>> > future values that may be necessary.
>> >
>> > This will allow bluetoothd to load parameters from a conf file that may
>> > be customized for the specific requirements of each platforms.
>> >
>> > ---
>> > rebase against current master
>> >
>> >  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 62 insertions(+)
>> >
>> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> > index 6ee01fed8..c6575e654 100644
>> > --- a/doc/mgmt-api.txt
>> > +++ b/doc/mgmt-api.txt
>> > @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
>> >                                 Invalid Index
>> >
>> >
>> > +Load Default Controller Parameter Command
>> > +=============================
>> > +
>> > +       Command Code:           0x004b
>> > +       Controller Index:       <controller id>
>> > +       Command Parameters:     Parameter_Count (2 Octets)
>> > +                               Parameter1 {
>> > +                                       Parameter_Type (2 Octet)
>> > +                                       Value_Length (1 Octet)
>> > +                                       Value (0-255 Octets)
>> > +                               }
>> > +                               Parameter2 { }
>> > +                               ...
>> > +       Return Parameters:
>> > +
>> > +       This command is used to feed the kernel a list of default controller
>> > +       parameters.
>> > +
>> > +       Currently defined Parameter_Type values are:
>> > +
>> > +               0x0000  BR/EDR Page Scan Type
>> > +               0x0001  BR/EDR Page Scan Interval
>> > +               0x0002  BR/EDR Page Scan Window
>> > +               0x0003  BR/EDR Inquiry Scan Type
>> > +               0x0004  BR/EDR Inquiry Scan Interval
>> > +               0x0005  BR/EDR Inquiry Scan Window
>> > +               0x0006  BR/EDR Link Supervision Timeout
>> > +               0x0007  BR/EDR Page Timeout
>> > +               0x0008  BR/EDR Min Sniff Interval
>> > +               0x0009  BR/EDR Max Sniff Interval
>> > +               0x000a  LE Advertisement Min Interval
>> > +               0x000b  LE Advertisement Max Interval
>> > +               0x000c  LE Multi Advertisement Rotation Interval
>> > +               0x000d  LE Scanning Interval for auto connect
>> > +               0x000e  LE Scanning Window for auto connect
>> > +               0x000f  LE Scanning Interval for HID only
>> > +               0x0010  LE Scanning Window for HID only
>>
>> I remember commenting that we don't have profile information on the
>> kernel so Im not sure how you are planning to detect the when the
>> device is HID or not?
>
> I expect we'll need to add an Add Device flag to carry that information down into the kernel.  The hid profile already sends an op, so it's relatively trivial to carry that information in so we can apply specific scanning requirements for that case (which are less aggressive and therefore have less of an impact to Wifi and BT performance).

Well if we will need to feed that information of the profile type we
might as well just feed the intervals directly, that way any profile
that needs some different parameters than the default can all use the
same command and we don't have to grow this list when a new profile
want to use different parameters.

>>
>>
>> > +               0x0012  LE Scanning Interval for wake scenarios
>> > +               0x0013  LE Scanning Window for wake scenarios
>> > +               0x0014  LE Scanning Interval for discovery
>> > +               0x0015  LE Scanning Window for discovery
>> > +               0x0016  LE Scanning Interval for adv monitoring
>> > +               0x0017  LE Scanning Window for adv monitoring
>> > +               0x0018  LE Scanning Interval for connect
>> > +               0x0019  LE Scanning Window for connect
>> > +               0x001a  LE Min Connection Interval
>> > +               0x001b  LE Max Connection Interval
>> > +               0x001c  LE Connection Connection Latency
>> > +               0x001d  LE Connection Supervision Timeout
>> > +
>> > +       This command can be used when the controller is not powered and
>> > +       all settings will be programmed once powered.  Note that these only
>> > +       control the default parameters.  Higher level Apis may influence the
>> > +       effective value used.
>> > +
>> > +       This command generates a Command Complete event on success or
>> > +       a Command Status event on failure.
>> > +
>> > +       Possible errors:        Invalid Parameters
>> > +                               Invalid Index
>> > +
>> > +
>> >  Command Complete Event
>> >  ======================
>> >
>> > --
>> > 2.26.2.761.g0e0b3e54be-goog
>> >
>>
>>
>> --
>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
  2020-05-21 22:05     ` Luiz Augusto von Dentz
@ 2020-05-21 22:27       ` Alain Michaud
  2020-05-21 23:18         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Alain Michaud @ 2020-05-21 22:27 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Alain Michaud, linux-bluetooth

Hi Luiz


On Thu, May 21, 2020 at 6:05 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Alain,
>
> On Thu, May 21, 2020 at 1:12 PM Alain Michaud <alainmichaud@google.com> wrote:
> >
> > Hi Luiz,
> >
> >
> > On Thu, May 21, 2020 at 12:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> >>
> >> Hi Alain,
> >>
> >> On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
> >> >
> >> > This change adds the definition for the load default parameter command.
> >> > In particular, this command is used to load default parameters for
> >> > various operations in the kernel. This mechanism is also expandable to
> >> > future values that may be necessary.
> >> >
> >> > This will allow bluetoothd to load parameters from a conf file that may
> >> > be customized for the specific requirements of each platforms.
> >> >
> >> > ---
> >> > rebase against current master
> >> >
> >> >  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 62 insertions(+)
> >> >
> >> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> >> > index 6ee01fed8..c6575e654 100644
> >> > --- a/doc/mgmt-api.txt
> >> > +++ b/doc/mgmt-api.txt
> >> > @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
> >> >                                 Invalid Index
> >> >
> >> >
> >> > +Load Default Controller Parameter Command
> >> > +=============================
> >> > +
> >> > +       Command Code:           0x004b
> >> > +       Controller Index:       <controller id>
> >> > +       Command Parameters:     Parameter_Count (2 Octets)
> >> > +                               Parameter1 {
> >> > +                                       Parameter_Type (2 Octet)
> >> > +                                       Value_Length (1 Octet)
> >> > +                                       Value (0-255 Octets)
> >> > +                               }
> >> > +                               Parameter2 { }
> >> > +                               ...
> >> > +       Return Parameters:
> >> > +
> >> > +       This command is used to feed the kernel a list of default controller
> >> > +       parameters.
> >> > +
> >> > +       Currently defined Parameter_Type values are:
> >> > +
> >> > +               0x0000  BR/EDR Page Scan Type
> >> > +               0x0001  BR/EDR Page Scan Interval
> >> > +               0x0002  BR/EDR Page Scan Window
> >> > +               0x0003  BR/EDR Inquiry Scan Type
> >> > +               0x0004  BR/EDR Inquiry Scan Interval
> >> > +               0x0005  BR/EDR Inquiry Scan Window
> >> > +               0x0006  BR/EDR Link Supervision Timeout
> >> > +               0x0007  BR/EDR Page Timeout
> >> > +               0x0008  BR/EDR Min Sniff Interval
> >> > +               0x0009  BR/EDR Max Sniff Interval
> >> > +               0x000a  LE Advertisement Min Interval
> >> > +               0x000b  LE Advertisement Max Interval
> >> > +               0x000c  LE Multi Advertisement Rotation Interval
> >> > +               0x000d  LE Scanning Interval for auto connect
> >> > +               0x000e  LE Scanning Window for auto connect
> >> > +               0x000f  LE Scanning Interval for HID only
> >> > +               0x0010  LE Scanning Window for HID only
> >>
> >> I remember commenting that we don't have profile information on the
> >> kernel so Im not sure how you are planning to detect the when the
> >> device is HID or not?
> >
> > I expect we'll need to add an Add Device flag to carry that information down into the kernel.  The hid profile already sends an op, so it's relatively trivial to carry that information in so we can apply specific scanning requirements for that case (which are less aggressive and therefore have less of an impact to Wifi and BT performance).
>
> Well if we will need to feed that information of the profile type we
> might as well just feed the intervals directly, that way any profile
> that needs some different parameters than the default can all use the
> same command and we don't have to grow this list when a new profile
> want to use different parameters.
>
That might be another option indeed.  I wonder how far you'd want to
take this to have the profile provide all the values.  At some point,
you'd need some logic that would pick the value that meets all the
profile requirements so if one wants w:100, i:200 and another wants
w:200: i400, you may need to scan at 100% to meet the requirements of
all profiles...  I think that with the current proposal, while less
flexible, may avoid this complexity/condition..

> >>
> >>
> >> > +               0x0012  LE Scanning Interval for wake scenarios
> >> > +               0x0013  LE Scanning Window for wake scenarios
> >> > +               0x0014  LE Scanning Interval for discovery
> >> > +               0x0015  LE Scanning Window for discovery
> >> > +               0x0016  LE Scanning Interval for adv monitoring
> >> > +               0x0017  LE Scanning Window for adv monitoring
> >> > +               0x0018  LE Scanning Interval for connect
> >> > +               0x0019  LE Scanning Window for connect
> >> > +               0x001a  LE Min Connection Interval
> >> > +               0x001b  LE Max Connection Interval
> >> > +               0x001c  LE Connection Connection Latency
> >> > +               0x001d  LE Connection Supervision Timeout
> >> > +
> >> > +       This command can be used when the controller is not powered and
> >> > +       all settings will be programmed once powered.  Note that these only
> >> > +       control the default parameters.  Higher level Apis may influence the
> >> > +       effective value used.
> >> > +
> >> > +       This command generates a Command Complete event on success or
> >> > +       a Command Status event on failure.
> >> > +
> >> > +       Possible errors:        Invalid Parameters
> >> > +                               Invalid Index
> >> > +
> >> > +
> >> >  Command Complete Event
> >> >  ======================
> >> >
> >> > --
> >> > 2.26.2.761.g0e0b3e54be-goog
> >> >
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
  2020-05-21 22:27       ` Alain Michaud
@ 2020-05-21 23:18         ` Luiz Augusto von Dentz
       [not found]           ` <CALWDO_XX-xKJ4Se6HwfyroD9uQHj32xf5ZAFkEEA_QjV5MsVNg@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-05-21 23:18 UTC (permalink / raw)
  To: Alain Michaud; +Cc: Alain Michaud, linux-bluetooth

Hi Alain,

On Thu, May 21, 2020 at 3:28 PM Alain Michaud <alainmichaud@google.com> wrote:
>
> Hi Luiz
>
>
> On Thu, May 21, 2020 at 6:05 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Alain,
> >
> > On Thu, May 21, 2020 at 1:12 PM Alain Michaud <alainmichaud@google.com> wrote:
> > >
> > > Hi Luiz,
> > >
> > >
> > > On Thu, May 21, 2020 at 12:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> > >>
> > >> Hi Alain,
> > >>
> > >> On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
> > >> >
> > >> > This change adds the definition for the load default parameter command.
> > >> > In particular, this command is used to load default parameters for
> > >> > various operations in the kernel. This mechanism is also expandable to
> > >> > future values that may be necessary.
> > >> >
> > >> > This will allow bluetoothd to load parameters from a conf file that may
> > >> > be customized for the specific requirements of each platforms.
> > >> >
> > >> > ---
> > >> > rebase against current master
> > >> >
> > >> >  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >> >  1 file changed, 62 insertions(+)
> > >> >
> > >> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> > >> > index 6ee01fed8..c6575e654 100644
> > >> > --- a/doc/mgmt-api.txt
> > >> > +++ b/doc/mgmt-api.txt
> > >> > @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
> > >> >                                 Invalid Index
> > >> >
> > >> >
> > >> > +Load Default Controller Parameter Command
> > >> > +=============================
> > >> > +
> > >> > +       Command Code:           0x004b
> > >> > +       Controller Index:       <controller id>
> > >> > +       Command Parameters:     Parameter_Count (2 Octets)
> > >> > +                               Parameter1 {
> > >> > +                                       Parameter_Type (2 Octet)
> > >> > +                                       Value_Length (1 Octet)
> > >> > +                                       Value (0-255 Octets)
> > >> > +                               }
> > >> > +                               Parameter2 { }
> > >> > +                               ...
> > >> > +       Return Parameters:
> > >> > +
> > >> > +       This command is used to feed the kernel a list of default controller
> > >> > +       parameters.
> > >> > +
> > >> > +       Currently defined Parameter_Type values are:
> > >> > +
> > >> > +               0x0000  BR/EDR Page Scan Type
> > >> > +               0x0001  BR/EDR Page Scan Interval
> > >> > +               0x0002  BR/EDR Page Scan Window
> > >> > +               0x0003  BR/EDR Inquiry Scan Type
> > >> > +               0x0004  BR/EDR Inquiry Scan Interval
> > >> > +               0x0005  BR/EDR Inquiry Scan Window
> > >> > +               0x0006  BR/EDR Link Supervision Timeout
> > >> > +               0x0007  BR/EDR Page Timeout
> > >> > +               0x0008  BR/EDR Min Sniff Interval
> > >> > +               0x0009  BR/EDR Max Sniff Interval
> > >> > +               0x000a  LE Advertisement Min Interval
> > >> > +               0x000b  LE Advertisement Max Interval
> > >> > +               0x000c  LE Multi Advertisement Rotation Interval
> > >> > +               0x000d  LE Scanning Interval for auto connect
> > >> > +               0x000e  LE Scanning Window for auto connect
> > >> > +               0x000f  LE Scanning Interval for HID only
> > >> > +               0x0010  LE Scanning Window for HID only
> > >>
> > >> I remember commenting that we don't have profile information on the
> > >> kernel so Im not sure how you are planning to detect the when the
> > >> device is HID or not?
> > >
> > > I expect we'll need to add an Add Device flag to carry that information down into the kernel.  The hid profile already sends an op, so it's relatively trivial to carry that information in so we can apply specific scanning requirements for that case (which are less aggressive and therefore have less of an impact to Wifi and BT performance).
> >
> > Well if we will need to feed that information of the profile type we
> > might as well just feed the intervals directly, that way any profile
> > that needs some different parameters than the default can all use the
> > same command and we don't have to grow this list when a new profile
> > want to use different parameters.
> >
> That might be another option indeed.  I wonder how far you'd want to
> take this to have the profile provide all the values.  At some point,
> you'd need some logic that would pick the value that meets all the
> profile requirements so if one wants w:100, i:200 and another wants
> w:200: i400, you may need to scan at 100% to meet the requirements of
> all profiles...  I think that with the current proposal, while less
> flexible, may avoid this complexity/condition..

Not sure I follow you here, what I had in mind was that the profile
provides its own settings for scanning but perhaps you are saying that
in case there are multiple profiles involved we would have to
aggregate the parameters, well that has to be done either way even if
we use simple types, in which case there can be multiple defaults (one
for each profile) that the kernel must aggregate or we do aggregate
them at userspace and just provide the end result. In the end I guess
this means that the connect scanning parameters should probably be per
device rather than a using a globals with per profile exceptions, much
like connection parameters are per device as well.

> > >>
> > >>
> > >> > +               0x0012  LE Scanning Interval for wake scenarios
> > >> > +               0x0013  LE Scanning Window for wake scenarios
> > >> > +               0x0014  LE Scanning Interval for discovery
> > >> > +               0x0015  LE Scanning Window for discovery
> > >> > +               0x0016  LE Scanning Interval for adv monitoring
> > >> > +               0x0017  LE Scanning Window for adv monitoring
> > >> > +               0x0018  LE Scanning Interval for connect
> > >> > +               0x0019  LE Scanning Window for connect
> > >> > +               0x001a  LE Min Connection Interval
> > >> > +               0x001b  LE Max Connection Interval
> > >> > +               0x001c  LE Connection Connection Latency
> > >> > +               0x001d  LE Connection Supervision Timeout
> > >> > +
> > >> > +       This command can be used when the controller is not powered and
> > >> > +       all settings will be programmed once powered.  Note that these only
> > >> > +       control the default parameters.  Higher level Apis may influence the
> > >> > +       effective value used.
> > >> > +
> > >> > +       This command generates a Command Complete event on success or
> > >> > +       a Command Status event on failure.
> > >> > +
> > >> > +       Possible errors:        Invalid Parameters
> > >> > +                               Invalid Index
> > >> > +
> > >> > +
> > >> >  Command Complete Event
> > >> >  ======================
> > >> >
> > >> > --
> > >> > 2.26.2.761.g0e0b3e54be-goog
> > >> >
> > >>
> > >>
> > >> --
> > >> Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
       [not found]           ` <CALWDO_XX-xKJ4Se6HwfyroD9uQHj32xf5ZAFkEEA_QjV5MsVNg@mail.gmail.com>
@ 2020-05-22  0:14             ` Luiz Augusto von Dentz
       [not found]               ` <CALWDO_Uhid28nvX5NEngfTa2s76zKpQMK+8sS9Wm2cOf4weHuw@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-05-22  0:14 UTC (permalink / raw)
  To: Alain Michaud; +Cc: Alain Michaud, BlueZ

Hi Alain,

On Thu, May 21, 2020 at 4:44 PM Alain Michaud <alainmichaud@google.com> wrote:
>
> Hi Luiz
>
> On Thu., May 21, 2020, 7:19 p.m. Luiz Augusto von Dentz, <luiz.dentz@gmail.com> wrote:
>>
>> Hi Alain,
>>
>> On Thu, May 21, 2020 at 3:28 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >
>> > Hi Luiz
>> >
>> >
>> > On Thu, May 21, 2020 at 6:05 PM Luiz Augusto von Dentz
>> > <luiz.dentz@gmail.com> wrote:
>> > >
>> > > Hi Alain,
>> > >
>> > > On Thu, May 21, 2020 at 1:12 PM Alain Michaud <alainmichaud@google.com> wrote:
>> > > >
>> > > > Hi Luiz,
>> > > >
>> > > >
>> > > > On Thu, May 21, 2020 at 12:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> > > >>
>> > > >> Hi Alain,
>> > > >>
>> > > >> On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
>> > > >> >
>> > > >> > This change adds the definition for the load default parameter command.
>> > > >> > In particular, this command is used to load default parameters for
>> > > >> > various operations in the kernel. This mechanism is also expandable to
>> > > >> > future values that may be necessary.
>> > > >> >
>> > > >> > This will allow bluetoothd to load parameters from a conf file that may
>> > > >> > be customized for the specific requirements of each platforms.
>> > > >> >
>> > > >> > ---
>> > > >> > rebase against current master
>> > > >> >
>> > > >> >  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
>> > > >> >  1 file changed, 62 insertions(+)
>> > > >> >
>> > > >> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> > > >> > index 6ee01fed8..c6575e654 100644
>> > > >> > --- a/doc/mgmt-api.txt
>> > > >> > +++ b/doc/mgmt-api.txt
>> > > >> > @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
>> > > >> >                                 Invalid Index
>> > > >> >
>> > > >> >
>> > > >> > +Load Default Controller Parameter Command
>> > > >> > +=============================
>> > > >> > +
>> > > >> > +       Command Code:           0x004b
>> > > >> > +       Controller Index:       <controller id>
>> > > >> > +       Command Parameters:     Parameter_Count (2 Octets)
>> > > >> > +                               Parameter1 {
>> > > >> > +                                       Parameter_Type (2 Octet)
>> > > >> > +                                       Value_Length (1 Octet)
>> > > >> > +                                       Value (0-255 Octets)
>> > > >> > +                               }
>> > > >> > +                               Parameter2 { }
>> > > >> > +                               ...
>> > > >> > +       Return Parameters:
>> > > >> > +
>> > > >> > +       This command is used to feed the kernel a list of default controller
>> > > >> > +       parameters.
>> > > >> > +
>> > > >> > +       Currently defined Parameter_Type values are:
>> > > >> > +
>> > > >> > +               0x0000  BR/EDR Page Scan Type
>> > > >> > +               0x0001  BR/EDR Page Scan Interval
>> > > >> > +               0x0002  BR/EDR Page Scan Window
>> > > >> > +               0x0003  BR/EDR Inquiry Scan Type
>> > > >> > +               0x0004  BR/EDR Inquiry Scan Interval
>> > > >> > +               0x0005  BR/EDR Inquiry Scan Window
>> > > >> > +               0x0006  BR/EDR Link Supervision Timeout
>> > > >> > +               0x0007  BR/EDR Page Timeout
>> > > >> > +               0x0008  BR/EDR Min Sniff Interval
>> > > >> > +               0x0009  BR/EDR Max Sniff Interval
>> > > >> > +               0x000a  LE Advertisement Min Interval
>> > > >> > +               0x000b  LE Advertisement Max Interval
>> > > >> > +               0x000c  LE Multi Advertisement Rotation Interval
>> > > >> > +               0x000d  LE Scanning Interval for auto connect
>> > > >> > +               0x000e  LE Scanning Window for auto connect
>> > > >> > +               0x000f  LE Scanning Interval for HID only
>> > > >> > +               0x0010  LE Scanning Window for HID only
>> > > >>
>> > > >> I remember commenting that we don't have profile information on the
>> > > >> kernel so Im not sure how you are planning to detect the when the
>> > > >> device is HID or not?
>> > > >
>> > > > I expect we'll need to add an Add Device flag to carry that information down into the kernel.  The hid profile already sends an op, so it's relatively trivial to carry that information in so we can apply specific scanning requirements for that case (which are less aggressive and therefore have less of an impact to Wifi and BT performance).
>> > >
>> > > Well if we will need to feed that information of the profile type we
>> > > might as well just feed the intervals directly, that way any profile
>> > > that needs some different parameters than the default can all use the
>> > > same command and we don't have to grow this list when a new profile
>> > > want to use different parameters.
>> > >
>> > That might be another option indeed.  I wonder how far you'd want to
>> > take this to have the profile provide all the values.  At some point,
>> > you'd need some logic that would pick the value that meets all the
>> > profile requirements so if one wants w:100, i:200 and another wants
>> > w:200: i400, you may need to scan at 100% to meet the requirements of
>> > all profiles...  I think that with the current proposal, while less
>> > flexible, may avoid this complexity/condition..
>>
>> Not sure I follow you here, what I had in mind was that the profile
>> provides its own settings for scanning but perhaps you are saying that
>> in case there are multiple profiles involved we would have to
>> aggregate the parameters, well that has to be done either way even if
>> we use simple types, in which case there can be multiple defaults (one
>> for each profile) that the kernel must aggregate or we do aggregate
>> them at userspace and just provide the end result. In the end I guess
>> this means that the connect scanning parameters should probably be per
>> device rather than a using a globals with per profile exceptions, much
>> like connection parameters are per device as well.
>
>
> You got the idea. Aggregation would be needed, but with simple types, we can have simple logic such as if only hid is added (via the flag, we use the smaller duty cycle provided by the hid parameters defined in this patch), else, we use the passive scanning parameters as we do today. In essence, with this logic, no aggregation of settings is necessary since it is up to the platform who set the various values via their .conf file.

Well you will need to know when to apply the hid params, so to me it
still seems a per device policy, what Im missing?

> Scan parameters are global, they can't be per connection like the connection parameters.

Not when you are scanning to connect to specifica device, perhaps you
want to expand on where each scanning parameter set takes place then.

>>
>>
>> > > >>
>> > > >>
>> > > >> > +               0x0012  LE Scanning Interval for wake scenarios
>> > > >> > +               0x0013  LE Scanning Window for wake scenarios
>> > > >> > +               0x0014  LE Scanning Interval for discovery
>> > > >> > +               0x0015  LE Scanning Window for discovery
>> > > >> > +               0x0016  LE Scanning Interval for adv monitoring
>> > > >> > +               0x0017  LE Scanning Window for adv monitoring
>> > > >> > +               0x0018  LE Scanning Interval for connect
>> > > >> > +               0x0019  LE Scanning Window for connect
>> > > >> > +               0x001a  LE Min Connection Interval
>> > > >> > +               0x001b  LE Max Connection Interval
>> > > >> > +               0x001c  LE Connection Connection Latency
>> > > >> > +               0x001d  LE Connection Supervision Timeout
>> > > >> > +
>> > > >> > +       This command can be used when the controller is not powered and
>> > > >> > +       all settings will be programmed once powered.  Note that these only
>> > > >> > +       control the default parameters.  Higher level Apis may influence the
>> > > >> > +       effective value used.
>> > > >> > +
>> > > >> > +       This command generates a Command Complete event on success or
>> > > >> > +       a Command Status event on failure.
>> > > >> > +
>> > > >> > +       Possible errors:        Invalid Parameters
>> > > >> > +                               Invalid Index
>> > > >> > +
>> > > >> > +
>> > > >> >  Command Complete Event
>> > > >> >  ======================
>> > > >> >
>> > > >> > --
>> > > >> > 2.26.2.761.g0e0b3e54be-goog
>> > > >> >
>> > > >>
>> > > >>
>> > > >> --
>> > > >> Luiz Augusto von Dentz
>> > >
>> > >
>> > >
>> > > --
>> > > Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
       [not found]               ` <CALWDO_Uhid28nvX5NEngfTa2s76zKpQMK+8sS9Wm2cOf4weHuw@mail.gmail.com>
@ 2020-05-22 18:04                 ` Luiz Augusto von Dentz
       [not found]                   ` <CALWDO_XiuiB6viANARXZ9hhSKkfW+stwwr44pMJbTBOUzxUDew@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-05-22 18:04 UTC (permalink / raw)
  To: Alain Michaud; +Cc: Alain Michaud, BlueZ

Hi Alain,

On Thu, May 21, 2020 at 6:04 PM Alain Michaud <alainmichaud@google.com> wrote:
>
>
>
> On Thu., May 21, 2020, 8:14 p.m. Luiz Augusto von Dentz, <luiz.dentz@gmail.com> wrote:
>>
>> Hi Alain,
>>
>> On Thu, May 21, 2020 at 4:44 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >
>> > Hi Luiz
>> >
>> > On Thu., May 21, 2020, 7:19 p.m. Luiz Augusto von Dentz, <luiz.dentz@gmail.com> wrote:
>> >>
>> >> Hi Alain,
>> >>
>> >> On Thu, May 21, 2020 at 3:28 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >> >
>> >> > Hi Luiz
>> >> >
>> >> >
>> >> > On Thu, May 21, 2020 at 6:05 PM Luiz Augusto von Dentz
>> >> > <luiz.dentz@gmail.com> wrote:
>> >> > >
>> >> > > Hi Alain,
>> >> > >
>> >> > > On Thu, May 21, 2020 at 1:12 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >> > > >
>> >> > > > Hi Luiz,
>> >> > > >
>> >> > > >
>> >> > > > On Thu, May 21, 2020 at 12:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> >> > > >>
>> >> > > >> Hi Alain,
>> >> > > >>
>> >> > > >> On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
>> >> > > >> >
>> >> > > >> > This change adds the definition for the load default parameter command.
>> >> > > >> > In particular, this command is used to load default parameters for
>> >> > > >> > various operations in the kernel. This mechanism is also expandable to
>> >> > > >> > future values that may be necessary.
>> >> > > >> >
>> >> > > >> > This will allow bluetoothd to load parameters from a conf file that may
>> >> > > >> > be customized for the specific requirements of each platforms.
>> >> > > >> >
>> >> > > >> > ---
>> >> > > >> > rebase against current master
>> >> > > >> >
>> >> > > >> >  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >> > > >> >  1 file changed, 62 insertions(+)
>> >> > > >> >
>> >> > > >> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> >> > > >> > index 6ee01fed8..c6575e654 100644
>> >> > > >> > --- a/doc/mgmt-api.txt
>> >> > > >> > +++ b/doc/mgmt-api.txt
>> >> > > >> > @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
>> >> > > >> >                                 Invalid Index
>> >> > > >> >
>> >> > > >> >
>> >> > > >> > +Load Default Controller Parameter Command
>> >> > > >> > +=============================
>> >> > > >> > +
>> >> > > >> > +       Command Code:           0x004b
>> >> > > >> > +       Controller Index:       <controller id>
>> >> > > >> > +       Command Parameters:     Parameter_Count (2 Octets)
>> >> > > >> > +                               Parameter1 {
>> >> > > >> > +                                       Parameter_Type (2 Octet)
>> >> > > >> > +                                       Value_Length (1 Octet)
>> >> > > >> > +                                       Value (0-255 Octets)
>> >> > > >> > +                               }
>> >> > > >> > +                               Parameter2 { }
>> >> > > >> > +                               ...
>> >> > > >> > +       Return Parameters:
>> >> > > >> > +
>> >> > > >> > +       This command is used to feed the kernel a list of default controller
>> >> > > >> > +       parameters.
>> >> > > >> > +
>> >> > > >> > +       Currently defined Parameter_Type values are:
>> >> > > >> > +
>> >> > > >> > +               0x0000  BR/EDR Page Scan Type
>> >> > > >> > +               0x0001  BR/EDR Page Scan Interval
>> >> > > >> > +               0x0002  BR/EDR Page Scan Window
>> >> > > >> > +               0x0003  BR/EDR Inquiry Scan Type
>> >> > > >> > +               0x0004  BR/EDR Inquiry Scan Interval
>> >> > > >> > +               0x0005  BR/EDR Inquiry Scan Window
>> >> > > >> > +               0x0006  BR/EDR Link Supervision Timeout
>> >> > > >> > +               0x0007  BR/EDR Page Timeout
>> >> > > >> > +               0x0008  BR/EDR Min Sniff Interval
>> >> > > >> > +               0x0009  BR/EDR Max Sniff Interval
>> >> > > >> > +               0x000a  LE Advertisement Min Interval
>> >> > > >> > +               0x000b  LE Advertisement Max Interval
>> >> > > >> > +               0x000c  LE Multi Advertisement Rotation Interval
>> >> > > >> > +               0x000d  LE Scanning Interval for auto connect
>> >> > > >> > +               0x000e  LE Scanning Window for auto connect
>> >> > > >> > +               0x000f  LE Scanning Interval for HID only
>> >> > > >> > +               0x0010  LE Scanning Window for HID only
>> >> > > >>
>> >> > > >> I remember commenting that we don't have profile information on the
>> >> > > >> kernel so Im not sure how you are planning to detect the when the
>> >> > > >> device is HID or not?
>> >> > > >
>> >> > > > I expect we'll need to add an Add Device flag to carry that information down into the kernel.  The hid profile already sends an op, so it's relatively trivial to carry that information in so we can apply specific scanning requirements for that case (which are less aggressive and therefore have less of an impact to Wifi and BT performance).
>> >> > >
>> >> > > Well if we will need to feed that information of the profile type we
>> >> > > might as well just feed the intervals directly, that way any profile
>> >> > > that needs some different parameters than the default can all use the
>> >> > > same command and we don't have to grow this list when a new profile
>> >> > > want to use different parameters.
>> >> > >
>> >> > That might be another option indeed.  I wonder how far you'd want to
>> >> > take this to have the profile provide all the values.  At some point,
>> >> > you'd need some logic that would pick the value that meets all the
>> >> > profile requirements so if one wants w:100, i:200 and another wants
>> >> > w:200: i400, you may need to scan at 100% to meet the requirements of
>> >> > all profiles...  I think that with the current proposal, while less
>> >> > flexible, may avoid this complexity/condition..
>> >>
>> >> Not sure I follow you here, what I had in mind was that the profile
>> >> provides its own settings for scanning but perhaps you are saying that
>> >> in case there are multiple profiles involved we would have to
>> >> aggregate the parameters, well that has to be done either way even if
>> >> we use simple types, in which case there can be multiple defaults (one
>> >> for each profile) that the kernel must aggregate or we do aggregate
>> >> them at userspace and just provide the end result. In the end I guess
>> >> this means that the connect scanning parameters should probably be per
>> >> device rather than a using a globals with per profile exceptions, much
>> >> like connection parameters are per device as well.
>> >
>> >
>> > You got the idea. Aggregation would be needed, but with simple types, we can have simple logic such as if only hid is added (via the flag, we use the smaller duty cycle provided by the hid parameters defined in this patch), else, we use the passive scanning parameters as we do today. In essence, with this logic, no aggregation of settings is necessary since it is up to the platform who set the various values via their .conf file.
>>
>> Well you will need to know when to apply the hid params, so to me it
>> still seems a per device policy, what Im missing?
>
> It's effectively passive scanning based on what requested it.
>>
>>
>> > Scan parameters are global, they can't be per connection like the connection parameters.
>>
>> Not when you are scanning to connect to specifica device, perhaps you
>> want to expand on where each scanning parameter set takes place then.
>
>
> Today, it's all passive scanning. This would introduce the concept of hid only passive scanning where we can have a lower scanning duty cycle and save bandwidth and power.

So the intend is to switch passive scanning 'modes', perhaps Im
missing an API or policy to do that, or is the indend to do this
automagically? It is a little bit hard to follow given that a system
can have many different peripherals, each with different scanning
requirements, or the idea is to apply HID mode only if there are HID
devices paired?

>>
>> >>
>> >>
>> >> > > >>
>> >> > > >>
>> >> > > >> > +               0x0012  LE Scanning Interval for wake scenarios
>> >> > > >> > +               0x0013  LE Scanning Window for wake scenarios
>> >> > > >> > +               0x0014  LE Scanning Interval for discovery
>> >> > > >> > +               0x0015  LE Scanning Window for discovery
>> >> > > >> > +               0x0016  LE Scanning Interval for adv monitoring
>> >> > > >> > +               0x0017  LE Scanning Window for adv monitoring
>> >> > > >> > +               0x0018  LE Scanning Interval for connect
>> >> > > >> > +               0x0019  LE Scanning Window for connect
>> >> > > >> > +               0x001a  LE Min Connection Interval
>> >> > > >> > +               0x001b  LE Max Connection Interval
>> >> > > >> > +               0x001c  LE Connection Connection Latency
>> >> > > >> > +               0x001d  LE Connection Supervision Timeout
>> >> > > >> > +
>> >> > > >> > +       This command can be used when the controller is not powered and
>> >> > > >> > +       all settings will be programmed once powered.  Note that these only
>> >> > > >> > +       control the default parameters.  Higher level Apis may influence the
>> >> > > >> > +       effective value used.
>> >> > > >> > +
>> >> > > >> > +       This command generates a Command Complete event on success or
>> >> > > >> > +       a Command Status event on failure.
>> >> > > >> > +
>> >> > > >> > +       Possible errors:        Invalid Parameters
>> >> > > >> > +                               Invalid Index
>> >> > > >> > +
>> >> > > >> > +
>> >> > > >> >  Command Complete Event
>> >> > > >> >  ======================
>> >> > > >> >
>> >> > > >> > --
>> >> > > >> > 2.26.2.761.g0e0b3e54be-goog
>> >> > > >> >
>> >> > > >>
>> >> > > >>
>> >> > > >> --
>> >> > > >> Luiz Augusto von Dentz
>> >> > >
>> >> > >
>> >> > >
>> >> > > --
>> >> > > Luiz Augusto von Dentz
>> >>
>> >>
>> >>
>> >> --
>> >> Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

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

* Re: [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op
       [not found]                   ` <CALWDO_XiuiB6viANARXZ9hhSKkfW+stwwr44pMJbTBOUzxUDew@mail.gmail.com>
@ 2020-05-26 13:47                     ` Alain Michaud
  0 siblings, 0 replies; 8+ messages in thread
From: Alain Michaud @ 2020-05-26 13:47 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Alain Michaud, BlueZ

Hi Luiz, Marcel.

Would you feel more comfortable if I remove the HID only mode
reference from this for now until it is officially introduced in the
kernel?

I'd like to make progress on the rest in the meantime.

Thanks,
Alain


On Fri, May 22, 2020 at 8:47 PM Alain Michaud <alainmichaud@google.com> wrote:
>
> Hi Luiz,
>
> On Fri., May 22, 2020, 2:04 p.m. Luiz Augusto von Dentz, <luiz.dentz@gmail.com> wrote:
>>
>> Hi Alain,
>>
>> On Thu, May 21, 2020 at 6:04 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >
>> >
>> >
>> > On Thu., May 21, 2020, 8:14 p.m. Luiz Augusto von Dentz, <luiz.dentz@gmail.com> wrote:
>> >>
>> >> Hi Alain,
>> >>
>> >> On Thu, May 21, 2020 at 4:44 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >> >
>> >> > Hi Luiz
>> >> >
>> >> > On Thu., May 21, 2020, 7:19 p.m. Luiz Augusto von Dentz, <luiz.dentz@gmail.com> wrote:
>> >> >>
>> >> >> Hi Alain,
>> >> >>
>> >> >> On Thu, May 21, 2020 at 3:28 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >> >> >
>> >> >> > Hi Luiz
>> >> >> >
>> >> >> >
>> >> >> > On Thu, May 21, 2020 at 6:05 PM Luiz Augusto von Dentz
>> >> >> > <luiz.dentz@gmail.com> wrote:
>> >> >> > >
>> >> >> > > Hi Alain,
>> >> >> > >
>> >> >> > > On Thu, May 21, 2020 at 1:12 PM Alain Michaud <alainmichaud@google.com> wrote:
>> >> >> > > >
>> >> >> > > > Hi Luiz,
>> >> >> > > >
>> >> >> > > >
>> >> >> > > > On Thu, May 21, 2020 at 12:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> >> >> > > >>
>> >> >> > > >> Hi Alain,
>> >> >> > > >>
>> >> >> > > >> On Thu, May 21, 2020 at 8:05 AM Alain Michaud <alainm@chromium.org> wrote:
>> >> >> > > >> >
>> >> >> > > >> > This change adds the definition for the load default parameter command.
>> >> >> > > >> > In particular, this command is used to load default parameters for
>> >> >> > > >> > various operations in the kernel. This mechanism is also expandable to
>> >> >> > > >> > future values that may be necessary.
>> >> >> > > >> >
>> >> >> > > >> > This will allow bluetoothd to load parameters from a conf file that may
>> >> >> > > >> > be customized for the specific requirements of each platforms.
>> >> >> > > >> >
>> >> >> > > >> > ---
>> >> >> > > >> > rebase against current master
>> >> >> > > >> >
>> >> >> > > >> >  doc/mgmt-api.txt | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >> >> > > >> >  1 file changed, 62 insertions(+)
>> >> >> > > >> >
>> >> >> > > >> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> >> >> > > >> > index 6ee01fed8..c6575e654 100644
>> >> >> > > >> > --- a/doc/mgmt-api.txt
>> >> >> > > >> > +++ b/doc/mgmt-api.txt
>> >> >> > > >> > @@ -3223,6 +3223,68 @@ Set Experimental Feature Command
>> >> >> > > >> >                                 Invalid Index
>> >> >> > > >> >
>> >> >> > > >> >
>> >> >> > > >> > +Load Default Controller Parameter Command
>> >> >> > > >> > +=============================
>> >> >> > > >> > +
>> >> >> > > >> > +       Command Code:           0x004b
>> >> >> > > >> > +       Controller Index:       <controller id>
>> >> >> > > >> > +       Command Parameters:     Parameter_Count (2 Octets)
>> >> >> > > >> > +                               Parameter1 {
>> >> >> > > >> > +                                       Parameter_Type (2 Octet)
>> >> >> > > >> > +                                       Value_Length (1 Octet)
>> >> >> > > >> > +                                       Value (0-255 Octets)
>> >> >> > > >> > +                               }
>> >> >> > > >> > +                               Parameter2 { }
>> >> >> > > >> > +                               ...
>> >> >> > > >> > +       Return Parameters:
>> >> >> > > >> > +
>> >> >> > > >> > +       This command is used to feed the kernel a list of default controller
>> >> >> > > >> > +       parameters.
>> >> >> > > >> > +
>> >> >> > > >> > +       Currently defined Parameter_Type values are:
>> >> >> > > >> > +
>> >> >> > > >> > +               0x0000  BR/EDR Page Scan Type
>> >> >> > > >> > +               0x0001  BR/EDR Page Scan Interval
>> >> >> > > >> > +               0x0002  BR/EDR Page Scan Window
>> >> >> > > >> > +               0x0003  BR/EDR Inquiry Scan Type
>> >> >> > > >> > +               0x0004  BR/EDR Inquiry Scan Interval
>> >> >> > > >> > +               0x0005  BR/EDR Inquiry Scan Window
>> >> >> > > >> > +               0x0006  BR/EDR Link Supervision Timeout
>> >> >> > > >> > +               0x0007  BR/EDR Page Timeout
>> >> >> > > >> > +               0x0008  BR/EDR Min Sniff Interval
>> >> >> > > >> > +               0x0009  BR/EDR Max Sniff Interval
>> >> >> > > >> > +               0x000a  LE Advertisement Min Interval
>> >> >> > > >> > +               0x000b  LE Advertisement Max Interval
>> >> >> > > >> > +               0x000c  LE Multi Advertisement Rotation Interval
>> >> >> > > >> > +               0x000d  LE Scanning Interval for auto connect
>> >> >> > > >> > +               0x000e  LE Scanning Window for auto connect
>> >> >> > > >> > +               0x000f  LE Scanning Interval for HID only
>> >> >> > > >> > +               0x0010  LE Scanning Window for HID only
>> >> >> > > >>
>> >> >> > > >> I remember commenting that we don't have profile information on the
>> >> >> > > >> kernel so Im not sure how you are planning to detect the when the
>> >> >> > > >> device is HID or not?
>> >> >> > > >
>> >> >> > > > I expect we'll need to add an Add Device flag to carry that information down into the kernel.  The hid profile already sends an op, so it's relatively trivial to carry that information in so we can apply specific scanning requirements for that case (which are less aggressive and therefore have less of an impact to Wifi and BT performance).
>> >> >> > >
>> >> >> > > Well if we will need to feed that information of the profile type we
>> >> >> > > might as well just feed the intervals directly, that way any profile
>> >> >> > > that needs some different parameters than the default can all use the
>> >> >> > > same command and we don't have to grow this list when a new profile
>> >> >> > > want to use different parameters.
>> >> >> > >
>> >> >> > That might be another option indeed.  I wonder how far you'd want to
>> >> >> > take this to have the profile provide all the values.  At some point,
>> >> >> > you'd need some logic that would pick the value that meets all the
>> >> >> > profile requirements so if one wants w:100, i:200 and another wants
>> >> >> > w:200: i400, you may need to scan at 100% to meet the requirements of
>> >> >> > all profiles...  I think that with the current proposal, while less
>> >> >> > flexible, may avoid this complexity/condition..
>> >> >>
>> >> >> Not sure I follow you here, what I had in mind was that the profile
>> >> >> provides its own settings for scanning but perhaps you are saying that
>> >> >> in case there are multiple profiles involved we would have to
>> >> >> aggregate the parameters, well that has to be done either way even if
>> >> >> we use simple types, in which case there can be multiple defaults (one
>> >> >> for each profile) that the kernel must aggregate or we do aggregate
>> >> >> them at userspace and just provide the end result. In the end I guess
>> >> >> this means that the connect scanning parameters should probably be per
>> >> >> device rather than a using a globals with per profile exceptions, much
>> >> >> like connection parameters are per device as well.
>> >> >
>> >> >
>> >> > You got the idea. Aggregation would be needed, but with simple types, we can have simple logic such as if only hid is added (via the flag, we use the smaller duty cycle provided by the hid parameters defined in this patch), else, we use the passive scanning parameters as we do today. In essence, with this logic, no aggregation of settings is necessary since it is up to the platform who set the various values via their .conf file.
>> >>
>> >> Well you will need to know when to apply the hid params, so to me it
>> >> still seems a per device policy, what Im missing?
>> >
>> > It's effectively passive scanning based on what requested it.
>> >>
>> >>
>> >> > Scan parameters are global, they can't be per connection like the connection parameters.
>> >>
>> >> Not when you are scanning to connect to specifica device, perhaps you
>> >> want to expand on where each scanning parameter set takes place then.
>> >
>> >
>> > Today, it's all passive scanning. This would introduce the concept of hid only passive scanning where we can have a lower scanning duty cycle and save bandwidth and power.
>>
>> So the intend is to switch passive scanning 'modes', perhaps Im
>> missing an API or policy to do that, or is the indend to do this
>> automagically? It is a little bit hard to follow given that a system
>> can have many different peripherals, each with different scanning
>> requirements, or the idea is to apply HID mode only if there are HID
>> devices paired?
>>
>> Yes, the idea is to use a hid only mode if there are only hid devices. Note that there is a precedent for this already (see suspend mode that was recently added).
>
>
>> >>
>> >> >>
>> >> >>
>> >> >> > > >>
>> >> >> > > >>
>> >> >> > > >> > +               0x0012  LE Scanning Interval for wake scenarios
>> >> >> > > >> > +               0x0013  LE Scanning Window for wake scenarios
>> >> >> > > >> > +               0x0014  LE Scanning Interval for discovery
>> >> >> > > >> > +               0x0015  LE Scanning Window for discovery
>> >> >> > > >> > +               0x0016  LE Scanning Interval for adv monitoring
>> >> >> > > >> > +               0x0017  LE Scanning Window for adv monitoring
>> >> >> > > >> > +               0x0018  LE Scanning Interval for connect
>> >> >> > > >> > +               0x0019  LE Scanning Window for connect
>> >> >> > > >> > +               0x001a  LE Min Connection Interval
>> >> >> > > >> > +               0x001b  LE Max Connection Interval
>> >> >> > > >> > +               0x001c  LE Connection Connection Latency
>> >> >> > > >> > +               0x001d  LE Connection Supervision Timeout
>> >> >> > > >> > +
>> >> >> > > >> > +       This command can be used when the controller is not powered and
>> >> >> > > >> > +       all settings will be programmed once powered.  Note that these only
>> >> >> > > >> > +       control the default parameters.  Higher level Apis may influence the
>> >> >> > > >> > +       effective value used.
>> >> >> > > >> > +
>> >> >> > > >> > +       This command generates a Command Complete event on success or
>> >> >> > > >> > +       a Command Status event on failure.
>> >> >> > > >> > +
>> >> >> > > >> > +       Possible errors:        Invalid Parameters
>> >> >> > > >> > +                               Invalid Index
>> >> >> > > >> > +
>> >> >> > > >> > +
>> >> >> > > >> >  Command Complete Event
>> >> >> > > >> >  ======================
>> >> >> > > >> >
>> >> >> > > >> > --
>> >> >> > > >> > 2.26.2.761.g0e0b3e54be-goog
>> >> >> > > >> >
>> >> >> > > >>
>> >> >> > > >>
>> >> >> > > >> --
>> >> >> > > >> Luiz Augusto von Dentz
>> >> >> > >
>> >> >> > >
>> >> >> > >
>> >> >> > > --
>> >> >> > > Luiz Augusto von Dentz
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Luiz Augusto von Dentz
>> >>
>> >>
>> >>
>> >> --
>> >> Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz

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

end of thread, other threads:[~2020-05-26 13:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 14:57 [BlueZ PATCH v3] doc:adding definitions for load default params mgmt op Alain Michaud
2020-05-21 16:26 ` Luiz Augusto von Dentz
     [not found]   ` <CALWDO_UQhU5QxZ1eRNWZtAtkcghWEBM5eVeujxhROSr7PG_P=w@mail.gmail.com>
2020-05-21 22:05     ` Luiz Augusto von Dentz
2020-05-21 22:27       ` Alain Michaud
2020-05-21 23:18         ` Luiz Augusto von Dentz
     [not found]           ` <CALWDO_XX-xKJ4Se6HwfyroD9uQHj32xf5ZAFkEEA_QjV5MsVNg@mail.gmail.com>
2020-05-22  0:14             ` Luiz Augusto von Dentz
     [not found]               ` <CALWDO_Uhid28nvX5NEngfTa2s76zKpQMK+8sS9Wm2cOf4weHuw@mail.gmail.com>
2020-05-22 18:04                 ` Luiz Augusto von Dentz
     [not found]                   ` <CALWDO_XiuiB6viANARXZ9hhSKkfW+stwwr44pMJbTBOUzxUDew@mail.gmail.com>
2020-05-26 13:47                     ` Alain Michaud

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).