Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CABmPvSHYi7WeWqgqxSe60omSyVXj_EezMhGi1GMXBBWM-TWeBw@mail.gmail.com>
 <CABBYNZJDbPd83cx0KWLGMpE_KaA9xUbuMCDsuQVTNxV7xrowiw@mail.gmail.com>
In-Reply-To: <CABBYNZJDbPd83cx0KWLGMpE_KaA9xUbuMCDsuQVTNxV7xrowiw@mail.gmail.com>
From:   Miao-chen Chou <mcchou@chromium.org>
Date:   Mon, 21 Sep 2020 12:20:54 -0700
Message-ID: <CABmPvSEgnWoyijsA=ZsNeJoJdbz5YJTZHtEGWp6JTj3dKxxMrQ@mail.gmail.com>
Subject: Re: Modifying Makefile.am to facilitate test-adv-monitor and future
 unit tests.
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     Bluetooth Kernel Mailing List <linux-bluetooth@vger.kernel.org>,
        Marcel Holtmann <marcel@holtmann.org>,
        Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
        Alain Michaud <alainm@chromium.org>,
        Manish Mandlik <mmandlik@chromium.org>,
        Howard Chung <howardchung@google.com>,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Luiz,

On Fri, Sep 18, 2020 at 3:12 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Miao,
>
> On Fri, Sep 18, 2020 at 12:44 PM Miao-chen Chou <mcchou@chromium.org> wrote:
> >
> > Hi Luiz and Marcel,
> >
> > Unlike the rest of the existing unit tests in BlueZ, the logic blocks
> > tested in test-adv-monitor require dependencies of not only
> > src/adv_monitor.c but also all the dependency tree of
> > src/adv_monitor.c. The current convention in Makefile.am is to add all
> > the extra dependencies one by one. However, the maintenance cost is
> > high and not suitable in the case of test-adv-monitor. Therefore, we'd
> > like to propose changes in Makefile.am to make the source of
> > bluetoothd as a static library and link it for bluetoothd target and
> > the unit test target. It would be great if you can provide feedback on
> > this idea before the implementation. Thanks in advance!
>
> Then we should have had the code move to src/shared for unit testing,
> but how exactly are you planning to do that? For testing the kernel
> interface it normally done via a dedicated tester, but that again can
> be done with shared library.
>
In series https://patchwork.kernel.org/project/bluetooth/list/?series=351021,
we introduced some helper functions in adv_monitor.h to perform unit
testing and test-adv-monitor to facilitate the unit tests of
adv_monitor. We are encountering an expected build check failure on
this series.

There are two categories in test-adv-monitor, content filtering and
RSSI tracking, and content filter is easy to be moved to a standalone
shared component while RSSI tracking involves the use of timer, D-Bus
method calls and adv_monitor's internal structures, and that makes it
strongly coupled with the adv_monitor implementation which require a
tree of dependencies apart from adv_monitor.

There are two options to resolve the build failures in our case.
(1) Reorganize Makefile.am
This option is to make the sources (except src/main.c) into a static
lib and link this lib in bluetoothd executable target and whichever
unit test the sources are required.
(2) Create src/shared/am to facilitate helpers and core logic
This option is to create a new source under src/shared/ to facilitate
helper functions and core logic for src/adv_monitor. The interface of
src/shared/am may have the following functions.
- Create/destroy functions of struct adv_monitor
- Create/destroy functions of struct adv_monitor_device
- Helper function for monitor content matching.
- Helper function for RSSI tracking
However, the logic of RSSI tracking is hard to be ripped off and moved
to src/shared/am. One example would be the use of timer in RSSI
tracking, and there is currently no previous example of the timer use
in the shared library.

Series https://patchwork.kernel.org/project/bluetooth/list/?series=351021
is up for review. Our next step here would be ripped off the unit test
for now and submit v5 of the series. Once we reach an conclusion on
advmon unit test, we can submit a separate series including the
refactoring and unit tests. Looking forward to any feedbacks. Thanks!

Regards,
Miao