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;
From:   Szymon Janc <szymon.janc@codecoup.pl>
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     Szymon Czapracki <szymon.czapracki@codecoup.pl>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH 2/8] monitor: Decode LE Periodic Advertising Sync Transfer Received Event
Date:   Wed, 08 Jan 2020 10:53:14 +0100
Message-ID: <1770049.2qRCXdMzth@ix>
Organization: CODECOUP
In-Reply-To: <CABBYNZLwzscUwUwFgpUu4TjbY7XVCVOCXXccvFFtZiU7yLEjcQ@mail.gmail.com>
References: <20200107091833.14921-1-szymon.czapracki@codecoup.pl> <20200107091833.14921-2-szymon.czapracki@codecoup.pl> <CABBYNZLwzscUwUwFgpUu4TjbY7XVCVOCXXccvFFtZiU7yLEjcQ@mail.gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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.

> 
> >         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