Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200406180331.891822-1-marcel@holtmann.org>
In-Reply-To: <20200406180331.891822-1-marcel@holtmann.org>
From:   Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Date:   Mon, 6 Apr 2020 11:24:15 -0700
Message-ID: <CANFp7mXe3QtkKL8+TgTp=jwvpydABhDkiXks=3h4+op20rWhGQ@mail.gmail.com>
Subject: Re: [PATCH] doc: Add commands and event for handling device flags
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Bluez mailing list <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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
>