Hi Manish,
On Mon, Oct 18, 2021 at 7:53 AM Manish Mandlik <mmandlik@xxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> On Fri, Oct 15, 2021 at 1:09 AM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
>>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>
>> This adds proper packet definitions for command and response of MSFT
>> extension.
>> ---
>> monitor/msft.h | 148 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 148 insertions(+)
>>
>> diff --git a/monitor/msft.h b/monitor/msft.h
>> index a268f4bc7..90a64117a 100644
>> --- a/monitor/msft.h
>> +++ b/monitor/msft.h
>> @@ -24,6 +24,154 @@
>>
>> #include <stdint.h>
>>
>> +#define MSFT_SUBCMD_READ_SUPPORTED_FEATURES 0x00
>> +
>> +struct msft_cmd_read_supported_features {
>> + uint8_t subcmd;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_read_supported_features {
>> + uint8_t status;
>> + uint8_t subcmd;
>> + uint8_t features[8];
>> + uint8_t evt_prefix_len;
>> + uint8_t evt_prefix[];
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_MONITOR_RSSI 0x01
>> +
>> +struct msft_cmd_monitor_rssi {
>> + uint8_t subcmd;
>> + uint16_t handle;
>> + int8_t rssi_high;
>> + int8_t rssi_low;
>> + uint8_t rssi_low_interval;
>> + uint8_t rssi_period;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_monitor_rssi {
>> + uint8_t status;
>> + uint8_t subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_CANCEL_MONITOR_RSSI 0x02
>> +
>> +struct msft_cmd_cancel_monitor_rssi {
>> + uint8_t subcmd;
>> + uint16_t handle;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_cancel_monitor_rssi {
>> + uint8_t status;
>> + uint8_t subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_MONITOR_ADV 0x03
>> +
>> +struct msft_le_monitor_pattern {
>> + uint8_t len;
>> + uint8_t type;
>> + uint8_t start;
>> + uint8_t data[];
>> +} __attribute__((packed));
>> +
>
>
> + #define MSFT_COND_LE_MONITOR_ADV_PATTERN 0x01
>>
>> +struct msft_le_monitor_adv_pattern_type {
>> + uint8_t num_patterns;
>> + struct msft_le_monitor_pattern data[];
>> +} __attribute__((packed));
>> +
>
>
> + #define MSFT_COND_LE_MONITOR_ADV_UUID 0x02
>>
>> +struct msft_le_monitor_adv_uuid_type {
>> + uint8_t type;
>> + union {
>> + uint16_t u16;
>> + uint32_t u32;
>> + uint8_t u128[8];
>> + } value;
>> +} __attribute__((packed));
>> +
>
>
> + #define MSFT_COND_LE_MONITOR_ADV_IRK 0x03
>>
>> +struct msft_le_monitor_adv_irk_type {
>> + uint8_t irk[8];
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_MONITOR_ADV_ADDR 0x04
>
> I think this is not a subcommand. Instead, it is a condition type. So we can rename this to something else, e.g. MSFT_COND_LE_MONITOR_ADV_ADDR.
> Similarly, we'll have to define other three condition types as well for msft_le_monitor_adv_pattern_type, msft_le_monitor_adv_uuid_type and msft_le_monitor_adv_irk_type as mentioned above.
Right I will fix it since the intent was to have it as conditions,
thanks for reviewing.
>> +struct msft_le_monitor_adv_addr {
>> + uint8_t type;
>> + uint8_t addr[6];
>> +} __attribute__((packed));
>> +
>> +struct msft_cmd_le_monitor_adv {
>> + uint8_t subcmd;
>> + int8_t rssi_low;
>> + int8_t rssi_high;
>
> Order should be:
> + int8_t rssi_high;
> + int8_t rssi_low;
>>
>> + uint8_t rssi_low_interval;
>> + uint8_t rssi_period;
>> + uint8_t type;
>> + uint8_t data[];
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_le_monitor_adv {
>> + uint8_t status;
>> + uint8_t subcmd;
>> + uint8_t handle;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_CANCEL_MONITOR_ADV 0x04
>> +
>> +struct msft_cmd_le_cancel_monitor_adv {
>> + uint8_t subcmd;
>> + uint8_t handle;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_le_cancel_monitor_adv {
>> + uint8_t status;
>> + uint8_t subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_MONITOR_ADV_ENABLE 0x05
>> +
>> +struct msft_cmd_le_monitor_adv_enable {
>> + uint8_t subcmd;
>> + uint8_t enable;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_le_monitor_adv_enable {
>> + uint8_t status;
>> + uint8_t subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_READ_ABS_RSSI 0x06
>> +
>> +struct msft_cmd_read_abs_rssi {
>> + uint8_t subcmd;
>> + uint16_t handle;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_read_abs_rssi {
>> + uint8_t status;
>> + uint8_t subcmd;
>> + uint16_t handle;
>> + uint8_t rssi;
Ack.
> 'int8_t rssi' instead of 'uint8_t rssi'
>
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBEVT_RSSI 0x01
>> +
>> +struct msft_evt_rssi {
>> + uint8_t subevt;
>> + uint8_t status;
>> + uint16_t handle;
>> + uint8_t rssi;
>
> same as above - 'int8_t rssi' instead of 'uint8_t rssi'
Ack.
>
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBEVT_MONITOR_DEVICE 0x02
>> +
>> +struct msft_evt_monitor_device {
>> + uint8_t subevt;
>> + uint8_t addr_type;
>> + uint8_t addr[6];
>> + uint8_t handle;
>> + uint8_t state;
>> +} __attribute__((packed));
>> +
>> struct vendor_ocf;
>> struct vendor_evt;
>>
>> --
>> 2.31.1
>>
>
> Rest of the changes look good to me.
>
> Thanks,
> Manish.
--
Luiz Augusto von Dentz
