Hi,
On Mon, Oct 18, 2021 at 9:24 AM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> 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
Pushed after making the suggested changes.
--
Luiz Augusto von Dentz
