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;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\))
Subject: Re: [PATCH] doc: Add commands and event for handling device flags
From:   Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <CANFp7mXe3QtkKL8+TgTp=jwvpydABhDkiXks=3h4+op20rWhGQ@mail.gmail.com>
Date:   Mon, 6 Apr 2020 20:36:34 +0200
Cc:     Bluez mailing list <linux-bluetooth@vger.kernel.org>
Content-Transfer-Encoding: 8BIT
Message-Id: <AC887BAA-95ED-456B-84D2-2D1EF6A8339F@holtmann.org>
References: <20200406180331.891822-1-marcel@holtmann.org>
 <CANFp7mXe3QtkKL8+TgTp=jwvpydABhDkiXks=3h4+op20rWhGQ@mail.gmail.com>
To:     Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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