Re: [PATCH] Add Support for LE Ping feature

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Spoorthi,

On 27 Mar 2019, at 5.43, SpoorthiX K <spoorthix.k@intel.com> wrote:
> +#define HCI_OP_WRITE_AUTH_PAYLOAD_TO    0x0c7c
> +struct hci_cp_write_auth_payload_to {
> +	__le16	handle;
> +	__u16	timeout;

As Marcel (and now also the build test robot) pointed out this should be __le16.

> #if IS_ENABLED(CONFIG_BT_LEDS)
> 	struct led_trigger	*power_led;
> @@ -500,6 +501,7 @@ struct hci_conn {
> 	__u8		remote_id;
> 
> 	unsigned int	sent;
> +	bool		auth_payload_timeout_set;

Would it make sense to add this to the flags instead of increasing the hci_conn struct size?

> +	/* Set the default Authenticated Payload Timeout after
> +	 * an LE Link is established.

The test further below also includes BR/EDR links, i.e. the above comment doesn’t match that.

> +	 * Section 3.3, the HCI command WRITE_AUTH_PAYLOAD_TIMEOUT should be
> +	 * sent when the link is active and Encryption is enabled, the conn
> +	 * type can be either LE or ACL and controller must support LMP Ping.
> +	 * Ensure for AES-CCM encryption as well.
> +	 */
> +	if ((conn->type == LE_LINK || conn->type == ACL_LINK) &

That ‘&’ the end should probably be a ‘&&’, shouldn’t it?

> +	} else {
> +		conn->state = BT_CONNECTED;
> +		hci_connect_cfm(conn, ev->status);
> +	}

Why is this being added? Such a transition to BT_CONNECTED state at this point in the code didn’t exist previously, and this doesn’t seem to be related to the new feature.

Johan