| Message ID | 20200406180331.891822-1-marcel@holtmann.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | doc: Add commands and event for handling device flags | expand |
Hi Marcel, On Mon, Apr 6, 2020 at 11:03 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > --- > doc/mgmt-api.txt | 96 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 96 insertions(+) > > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt > index 39f23c456080..ac5b6c97d64a 100644 > --- a/doc/mgmt-api.txt > +++ b/doc/mgmt-api.txt > @@ -3138,6 +3138,74 @@ Read Security Information Command > Invalid Index > > > +Get Device Flags Command > +======================== > + > + Command Code: 0x0049 > + Controller Index: <controller id> > + Command Parameters: Address (6 Octets) > + Address_Type (1 Octet) > + Return Parameters: Address (6 Octets) > + Address_Type (1 Octet) > + Supported_Flags (4 Octets) > + Current_Flags (4 Octets) > + > + This command is used to retrieve additional flags and settings > + for devices that are added via Add Device command. > + > + Possible values for the Address_Type parameter: > + 0 BR/EDR > + 1 LE Public > + 2 LE Random > + > + The Flags parameters are a bitmask with currently the following > + available bits: > + > + 0 Remote Wakeup enabled > + > + This command generates a Command Complete event on success > + or a Command Status event on failure. > + > + Possible errors: Invalid Parameters > + Invalid Index > + > + Get device flags looks good to me. > +Set Device Flags Command > +======================== > + > + Command Code: 0x004a > + Controller Index: <controller id> > + Command Parameters: Address (6 Octets) > + Address_Type (1 Octet) > + Current_Flags (4 Octets) I would prefer to use a mask and value rather than current_flags here. > + Return Parameters: Address (6 Octets) > + Address_Type (1 Octet) Prefer to also return an updated_mask and current_flags. This simplifies completion for userspace. Otherwise, we would need to keep a "pending flags" value on the device structure. > + > + This command is used to configure additional flags and settings > + for devices that are added via Add Device command. > + > + Possible values for the Address_Type parameter: > + 0 BR/EDR > + 1 LE Public > + 2 LE Random > + > + The list of supported Flags can be retrieved via the Get Device > + Flags or Device Flags Changed command. Selecting unsupported flags > + will result in an Invalid Parameter error; > + > + Refer to the Get Device Flags command for a detailed description > + of the Flags parameters. > + > + This command can be used when the controller is not powered and > + all settings will be programmed once powered. > + > + 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 > ====================== > > @@ -4004,6 +4072,7 @@ Extended Controller Information Changed Event > The event will only be sent to management sockets other than the > one through which the change was triggered. > > + > PHY Configuration Changed Event > =============================== > > @@ -4020,3 +4089,30 @@ PHY Configuration Changed Event > one through which the change was triggered. > > Refer Get PHY Configuration command for PHYs parameter. > + > + > +Device Flags Changed Event > +========================== > + > + Event Code: 0x0027 > + Controller Index: <controller id> > + Event Parameters: Address (6 Octets) > + Address_Type (1 Octet) > + Supported_Flags (4 Octets) > + Current_Flags (4 Octets) > + > + This event indicates that the device flags have been changed via > + the Set Device Flags command or that a new device has been added > + via the Add Device command. In the latter case it is send right > + after the Device Added event. > + > + Possible values for the Address_Type parameter: > + 0 BR/EDR > + 1 LE Public > + 2 LE Random > + > + The event will only be sent to management sockets other than the > + one through which the command was sent. > + > + In case this event is triggered by Add Device then it is sent to > + all management sockets. Event looks good to me. > -- > 2.25.1 >
Hi Abhishek, >> --- >> doc/mgmt-api.txt | 96 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 96 insertions(+) >> >> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt >> index 39f23c456080..ac5b6c97d64a 100644 >> --- a/doc/mgmt-api.txt >> +++ b/doc/mgmt-api.txt >> @@ -3138,6 +3138,74 @@ Read Security Information Command >> Invalid Index >> >> >> +Get Device Flags Command >> +======================== >> + >> + Command Code: 0x0049 >> + Controller Index: <controller id> >> + Command Parameters: Address (6 Octets) >> + Address_Type (1 Octet) >> + Return Parameters: Address (6 Octets) >> + Address_Type (1 Octet) >> + Supported_Flags (4 Octets) >> + Current_Flags (4 Octets) >> + >> + This command is used to retrieve additional flags and settings >> + for devices that are added via Add Device command. >> + >> + Possible values for the Address_Type parameter: >> + 0 BR/EDR >> + 1 LE Public >> + 2 LE Random >> + >> + The Flags parameters are a bitmask with currently the following >> + available bits: >> + >> + 0 Remote Wakeup enabled >> + >> + This command generates a Command Complete event on success >> + or a Command Status event on failure. >> + >> + Possible errors: Invalid Parameters >> + Invalid Index >> + >> + > > Get device flags looks good to me. > >> +Set Device Flags Command >> +======================== >> + >> + Command Code: 0x004a >> + Controller Index: <controller id> >> + Command Parameters: Address (6 Octets) >> + Address_Type (1 Octet) >> + Current_Flags (4 Octets) > > I would prefer to use a mask and value rather than current_flags here. > >> + Return Parameters: Address (6 Octets) >> + Address_Type (1 Octet) > > Prefer to also return an updated_mask and current_flags. This > simplifies completion for userspace. Otherwise, we would need to keep > a "pending flags" value on the device structure. I saw your “mask” proposal and I am not a fan of that. I want to keep the design of the API somewhat consistent. Hence the Device Flags Changed event should be send after Add Device completed and also after Device Added has been sent out. One other option is to keep Get Device Flags as is and then instead of adding Set Device Flags, add one command per flag and rename Device Flags Changed to New Device Flags. Set Device Wakeable Command =========================== Command Code: 0x004a Controller Index: <controller id> Command Parameters: Address (6 Octets) Address_Type (1 Octet) Wakeable (1 Octet) Return Parameters: Address (6 Octets) Address_Type (1 Octet) Current_Flags (4 Octets) Maybe this is not a bad idea either and is similar on how we handle settings. I need to sleep about this. Regards Marcel
Hey Marcel, Coming back to this device flags idea, I think I would prefer the original design over adding new management commands for each flag. Bluez will just have to maintain the current flags and pending flags (i.e. mgmt call) to decide when to emit property changed events for the device WakeAllowed property. 0x0049 and 0x004A are now taken for experimental features but you have my reviewed-by for the next available values for the original patch. Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> Thanks Abhishek On Mon, Apr 6, 2020 at 11:36 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Abhishek, > > >> --- > >> doc/mgmt-api.txt | 96 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 96 insertions(+) > >> > >> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt > >> index 39f23c456080..ac5b6c97d64a 100644 > >> --- a/doc/mgmt-api.txt > >> +++ b/doc/mgmt-api.txt > >> @@ -3138,6 +3138,74 @@ Read Security Information Command > >> Invalid Index > >> > >> > >> +Get Device Flags Command > >> +======================== > >> + > >> + Command Code: 0x0049 > >> + Controller Index: <controller id> > >> + Command Parameters: Address (6 Octets) > >> + Address_Type (1 Octet) > >> + Return Parameters: Address (6 Octets) > >> + Address_Type (1 Octet) > >> + Supported_Flags (4 Octets) > >> + Current_Flags (4 Octets) > >> + > >> + This command is used to retrieve additional flags and settings > >> + for devices that are added via Add Device command. > >> + > >> + Possible values for the Address_Type parameter: > >> + 0 BR/EDR > >> + 1 LE Public > >> + 2 LE Random > >> + > >> + The Flags parameters are a bitmask with currently the following > >> + available bits: > >> + > >> + 0 Remote Wakeup enabled > >> + > >> + This command generates a Command Complete event on success > >> + or a Command Status event on failure. > >> + > >> + Possible errors: Invalid Parameters > >> + Invalid Index > >> + > >> + > > > > Get device flags looks good to me. > > > >> +Set Device Flags Command > >> +======================== > >> + > >> + Command Code: 0x004a > >> + Controller Index: <controller id> > >> + Command Parameters: Address (6 Octets) > >> + Address_Type (1 Octet) > >> + Current_Flags (4 Octets) > > > > I would prefer to use a mask and value rather than current_flags here. > > > >> + Return Parameters: Address (6 Octets) > >> + Address_Type (1 Octet) > > > > Prefer to also return an updated_mask and current_flags. This > > simplifies completion for userspace. Otherwise, we would need to keep > > a "pending flags" value on the device structure. > > I saw your “mask” proposal and I am not a fan of that. I want to keep the design of the API somewhat consistent. Hence the Device Flags Changed event should be send after Add Device completed and also after Device Added has been sent out. > > One other option is to keep Get Device Flags as is and then instead of adding Set Device Flags, add one command per flag and rename Device Flags Changed to New Device Flags. > > Set Device Wakeable Command > =========================== > > Command Code: 0x004a > Controller Index: <controller id> > Command Parameters: Address (6 Octets) > Address_Type (1 Octet) > Wakeable (1 Octet) > Return Parameters: Address (6 Octets) > Address_Type (1 Octet) > Current_Flags (4 Octets) > > Maybe this is not a bad idea either and is similar on how we handle settings. I need to sleep about this. > > Regards > > Marcel >
Hi Abhishek, > Coming back to this device flags idea, I think I would prefer the > original design over adding new management commands for each flag. > Bluez will just have to maintain the current flags and pending flags > (i.e. mgmt call) to decide when to emit property changed events for > the device WakeAllowed property. > > 0x0049 and 0x004A are now taken for experimental features but you have > my reviewed-by for the next available values for the original patch. I updated the patch and applied it. Regards Marcel
Thanks Marcel. Do you want to handle the implementation in the kernel or shall I send up a patch? Abhishek On Wed, Jun 10, 2020 at 10:23 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Abhishek, > > > Coming back to this device flags idea, I think I would prefer the > > original design over adding new management commands for each flag. > > Bluez will just have to maintain the current flags and pending flags > > (i.e. mgmt call) to decide when to emit property changed events for > > the device WakeAllowed property. > > > > 0x0049 and 0x004A are now taken for experimental features but you have > > my reviewed-by for the next available values for the original patch. > > I updated the patch and applied it. > > Regards > > Marcel >
Hi Abhishek, > Do you want to handle the implementation in the kernel or shall I send > up a patch? if you can adopt one of your earlier patches, that would be great. I am currently busy with a few other patches. So it would be bottom of my queue. Regards Marcel
diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt index 39f23c456080..ac5b6c97d64a 100644 --- a/doc/mgmt-api.txt +++ b/doc/mgmt-api.txt @@ -3138,6 +3138,74 @@ Read Security Information Command Invalid Index +Get Device Flags Command +======================== + + Command Code: 0x0049 + Controller Index: <controller id> + Command Parameters: Address (6 Octets) + Address_Type (1 Octet) + Return Parameters: Address (6 Octets) + Address_Type (1 Octet) + Supported_Flags (4 Octets) + Current_Flags (4 Octets) + + This command is used to retrieve additional flags and settings + for devices that are added via Add Device command. + + Possible values for the Address_Type parameter: + 0 BR/EDR + 1 LE Public + 2 LE Random + + The Flags parameters are a bitmask with currently the following + available bits: + + 0 Remote Wakeup enabled + + This command generates a Command Complete event on success + or a Command Status event on failure. + + Possible errors: Invalid Parameters + Invalid Index + + +Set Device Flags Command +======================== + + Command Code: 0x004a + Controller Index: <controller id> + Command Parameters: Address (6 Octets) + Address_Type (1 Octet) + Current_Flags (4 Octets) + Return Parameters: Address (6 Octets) + Address_Type (1 Octet) + + This command is used to configure additional flags and settings + for devices that are added via Add Device command. + + Possible values for the Address_Type parameter: + 0 BR/EDR + 1 LE Public + 2 LE Random + + The list of supported Flags can be retrieved via the Get Device + Flags or Device Flags Changed command. Selecting unsupported flags + will result in an Invalid Parameter error; + + Refer to the Get Device Flags command for a detailed description + of the Flags parameters. + + This command can be used when the controller is not powered and + all settings will be programmed once powered. + + 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 ====================== @@ -4004,6 +4072,7 @@ Extended Controller Information Changed Event The event will only be sent to management sockets other than the one through which the change was triggered. + PHY Configuration Changed Event =============================== @@ -4020,3 +4089,30 @@ PHY Configuration Changed Event one through which the change was triggered. Refer Get PHY Configuration command for PHYs parameter. + + +Device Flags Changed Event +========================== + + Event Code: 0x0027 + Controller Index: <controller id> + Event Parameters: Address (6 Octets) + Address_Type (1 Octet) + Supported_Flags (4 Octets) + Current_Flags (4 Octets) + + This event indicates that the device flags have been changed via + the Set Device Flags command or that a new device has been added + via the Add Device command. In the latter case it is send right + after the Device Added event. + + Possible values for the Address_Type parameter: + 0 BR/EDR + 1 LE Public + 2 LE Random + + The event will only be sent to management sockets other than the + one through which the command was sent. + + In case this event is triggered by Add Device then it is sent to + all management sockets.