Re: [PATCH] doc: Add commands and event for handling device flags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxx>

Thanks
Abhishek

On Mon, Apr 6, 2020 at 11:36 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> 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
>




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux