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: <20200107091833.14921-1-szymon.czapracki@codecoup.pl>
 <20200107091833.14921-2-szymon.czapracki@codecoup.pl> <CABBYNZLwzscUwUwFgpUu4TjbY7XVCVOCXXccvFFtZiU7yLEjcQ@mail.gmail.com>
 <1770049.2qRCXdMzth@ix>
In-Reply-To: <1770049.2qRCXdMzth@ix>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Wed, 8 Jan 2020 10:22:16 -0800
Message-ID: <CABBYNZLL_zB7O+hsvh1Yx1MZ_=H4A7E0wwuk=8RQd4KsrxUneg@mail.gmail.com>
Subject: Re: [PATCH 2/8] monitor: Decode LE Periodic Advertising Sync Transfer
 Received Event
To:     Szymon Janc <szymon.janc@codecoup.pl>
Cc:     Szymon Czapracki <szymon.czapracki@codecoup.pl>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Szymon,

On Wed, Jan 8, 2020 at 1:53 AM Szymon Janc <szymon.janc@codecoup.pl> wrote:
>
> Hi,
>
> On Wednesday, 8 January 2020 00:43:44 CET Luiz Augusto von Dentz wrote:
> > Hi Szymon,
> >
> > On Tue, Jan 7, 2020 at 1:20 AM Szymon Czapracki
> >
> > <szymon.czapracki@codecoup.pl> wrote:
> > > Change-Id: I5a7c2d36ca5aee61441c2aab6adeb16058ab062f
> > > ---
> > >
> > >  monitor/bt.h     | 14 ++++++++++++++
> > >  monitor/packet.c | 27 +++++++++++++++++++++++++++
> > >  2 files changed, 41 insertions(+)
> > >
> > > diff --git a/monitor/bt.h b/monitor/bt.h
> > > index ecf3782c9..e14c1771f 100644
> > > --- a/monitor/bt.h
> > > +++ b/monitor/bt.h
> > > @@ -3140,6 +3140,20 @@ struct bt_hci_evt_le_chan_select_alg {
> > >
> > >         uint8_t  algorithm;
> > >
> > >  } __attribute__ ((packed));
> > >
> > > +#define BT_HCI_EVT_LE_PER_ADV_SYNC_TRANS_REC           0x18
> > > +struct bt_hci_evt_le_per_adv_sync_trans_rec {
> > > +       uint8_t  status;
> > > +       uint16_t handle;
> > > +       uint16_t service_data;
> > > +       uint16_t sync_handle;
> > > +       uint8_t  sid;
> > > +       uint8_t  addr_type;
> > > +       uint8_t  addr[6];
> > > +       uint8_t  phy;
> > > +       uint16_t interval;
> > > +       uint8_t  clock_accuracy;
> > > +} __attribute__ ((packed));
> > > +
> > >
> > >  #define BT_HCI_ERR_SUCCESS                     0x00
> > >  #define BT_HCI_ERR_UNKNOWN_COMMAND             0x01
> > >  #define BT_HCI_ERR_UNKNOWN_CONN_ID             0x02
> > >
> > > diff --git a/monitor/packet.c b/monitor/packet.c
> > > index 64f75cf8e..76bb9f239 100644
> > > --- a/monitor/packet.c
> > > +++ b/monitor/packet.c
> > > @@ -600,6 +600,12 @@ static void print_addr_type(const char *label,
> > > uint8_t addr_type)>
> > >         case 0x01:
> > >                 str = "Random";
> > >                 break;
> > >
> > > +       case 0x02:
> > > +               str = "Public Identity Address";
> > > +               break;
> > > +       case 0x03:
> > > +               str = "Random Identity Address";
> > > +               break;
> >
> > Usually the term Address is already part of the label so we don't need
> > to repeat here, Random Identity also doesn't sound right, it should
> > probably be Static Random or perhaps have Resolved Public and Resolved
> > Static Random to indicate they are actually resolved address from RPA.
>
> Those names are from spec actually but I agree that those are somewhat odd.
> How about just using print_peer_addr_type() to print this?
> Then we have: public, random, resolved public and resolved Random. This is
> short and also makes output consistent with other events.

Yep, lets use that then.

> >
> > >         default:
> > >                 str = "Reserved";
> > >                 break;
> > >
> > > @@ -9788,6 +9794,24 @@ static void le_chan_select_alg_evt(const void
> > > *data, uint8_t size)>
> > >         print_field("Algorithm: %s (0x%2.2x)", str, evt->algorithm);
> > >
> > >  }
> > >
> > > +static void le_per_adv_sync_trans_rec_evt(const void *data, uint8_t size)
> > > +{
> > > +       const struct bt_hci_evt_le_per_adv_sync_trans_rec *evt = data;
> > > +
> > > +       print_status(evt->status);
> > > +       print_field("Handle: %d", evt->handle);
> > > +       print_field("Connection handle: %d", evt->handle);
> > > +       print_field("Service data: 0x%4.4x", evt->service_data);
> > > +       print_field("Sync handle: %d", evt->sync_handle);
> > > +       print_field("SID: 0x%2.2x", evt->sid);
> > > +       print_addr_type("Address type:", evt->addr_type);
> > > +       print_addr("Addres:", evt->addr, evt->addr_type);
> > > +       print_le_phy("PHY:", evt->phy);
> > > +       print_field("Periodic advertising Interval: %.3f",
> > > +                                                       1.25 *
> > > evt->interval); +       print_clock_accuracy(evt->clock_accuracy);
> > > +}
> > > +
> > >
> > >  struct subevent_data {
> > >
> > >         uint8_t subevent;
> > >         const char *str;
> > >
> > > @@ -9871,6 +9895,9 @@ static const struct subevent_data
> > > le_meta_event_table[] = {>
> > >                                 le_scan_req_received_evt, 8, true},
> > >
> > >         { 0x14, "LE Channel Selection Algorithm",
> > >
> > >                                 le_chan_select_alg_evt, 3, true},
> > >
> > > +       { 0x18, "LE Periodic Advertising Sync Transfer Received",
> > > +                                       le_per_adv_sync_trans_rec_evt, 19,
> > > +                                       true},
> > >
> > >         { }
> > >
> > >  };
> > >
> > > --
> > > 2.24.1
>
>
> --
> pozdrawiam
> Szymon Janc
>
>


-- 
Luiz Augusto von Dentz