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: <20210909190503.BlueZ.v2.1.Iae4b26a8036d47ca4d0db470f2bb23247f6cac7d@changeid>
 <CAGPPCLDpXtXfhMHp39ABmYB8=OduvJmR0VkN2nh9E8Sqg7V_gw@mail.gmail.com>
In-Reply-To: <CAGPPCLDpXtXfhMHp39ABmYB8=OduvJmR0VkN2nh9E8Sqg7V_gw@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 20 Sep 2021 14:54:35 -0700
Message-ID: <CABBYNZLMunjUB1MR8kGNDPjRxLSft7m5ujODkhssybOmNwbc3A@mail.gmail.com>
Subject: Re: [BlueZ PATCH v2] adv_monitor: Clear any running DeviceLost timers
 on power down
To:     Manish Mandlik <mmandlik@google.com>
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>,
        Miao-chen Chou <mcchou@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Manish,

On Mon, Sep 20, 2021 at 8:53 AM Manish Mandlik <mmandlik@google.com> wrote:
>
> Friendly reminder to review this patch.
>
> Thanks,
> Manish.
>
>
> On Thu, Sep 9, 2021 at 7:05 PM Manish Mandlik <mmandlik@google.com> wrote:
>>
>> This patch clears any running Adv Monitor DeviceLost timers on bt power
>> down. It'll also invoke DeviceLost event if the device is already found
>> and is being tracked for the DeviceLost event.
>>
>> Verified this by adding a monitor using bluetoothctl and confirming that
>> the DeviceLost event is getting triggered for already found device in
>> case of bt power down.
>>
>> Reviewed-by: mcchou@google.com
>> ---
>>
>> Changes in v2:
>> - Removed the wrapper in adapter.c, updated the debug log.
>>
>>  src/adapter.c     |  2 ++
>>  src/adv_monitor.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>>  src/adv_monitor.h |  2 ++
>>  3 files changed, 55 insertions(+)
>>
>> diff --git a/src/adapter.c b/src/adapter.c
>> index 84bc5a1b0..5ddc5eee6 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -2912,6 +2912,8 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
>>                 len = sizeof(mode);
>>
>>                 if (!mode) {
>> +                       btd_adv_monitor_notify_power_down(
>> +                                       adapter->adv_monitor_manager);
>>                         clear_discoverable(adapter);
>>                         remove_temporary_devices(adapter);
>>                 }
>> diff --git a/src/adv_monitor.c b/src/adv_monitor.c
>> index 715ac5904..39094c10d 100644
>> --- a/src/adv_monitor.c
>> +++ b/src/adv_monitor.c
>> @@ -2011,3 +2011,54 @@ static void adv_monitor_filter_rssi(struct adv_monitor *monitor,
>>                                             NULL);
>>         }
>>  }
>> +
>> +/* Clears running DeviceLost timer for a given device */
>> +static void clear_device_lost_timer(void *data, void *user_data)
>> +{
>> +       struct adv_monitor_device *dev = data;
>> +       struct adv_monitor *monitor = NULL;
>> +
>> +       if (dev->lost_timer) {
>> +               timeout_remove(dev->lost_timer);
>> +               dev->lost_timer = 0;
>> +
>> +               monitor = dev->monitor;
>> +
>> +               DBG("Calling DeviceLost() for device %p on Adv Monitor "
>> +                               "of owner %s at path %s", dev->device,
>> +                               monitor->app->owner, monitor->path);
>> +
>> +               g_dbus_proxy_method_call(monitor->proxy, "DeviceLost",
>> +                               report_device_state_setup,
>> +                               NULL, dev->device, NULL);
>> +       }
>> +}
>> +
>> +/* Clears running DeviceLost timers from each monitor */
>> +static void clear_lost_timers_from_monitor(void *data, void *user_data)
>> +{
>> +       struct adv_monitor *monitor = data;
>> +
>> +       queue_foreach(monitor->devices, clear_device_lost_timer, NULL);
>> +}
>> +
>> +/* Clears running DeviceLost timers from each app */
>> +static void clear_lost_timers_from_app(void *data, void *user_data)
>> +{
>> +       struct adv_monitor_app *app = data;
>> +
>> +       queue_foreach(app->monitors, clear_lost_timers_from_monitor, NULL);
>> +}
>> +
>> +/* Handles bt power down scenario */
>> +void btd_adv_monitor_notify_power_down(struct btd_adv_monitor_manager *manager)
>> +{
>> +       if (!manager) {
>> +               error("Unexpected NULL btd_adv_monitor_manager object upon "
>> +                               "power down");
>> +               return;
>> +       }
>> +
>> +       /* Clear any running DeviceLost timers in case of power down */
>> +       queue_foreach(manager->apps, clear_lost_timers_from_app, NULL);
>> +}
>> diff --git a/src/adv_monitor.h b/src/adv_monitor.h
>> index 2b4f68abf..20da012d4 100644
>> --- a/src/adv_monitor.h
>> +++ b/src/adv_monitor.h
>> @@ -38,4 +38,6 @@ void btd_adv_monitor_notify_monitors(struct btd_adv_monitor_manager *manager,
>>  void btd_adv_monitor_device_remove(struct btd_adv_monitor_manager *manager,
>>                                    struct btd_device *device);
>>
>> +void btd_adv_monitor_notify_power_down(struct btd_adv_monitor_manager *manager);
>> +
>>  #endif /* __ADV_MONITOR_H */
>> --
>> 2.33.0.309.g3052b89438-goog
>>

It has already been pushed some 10 days ago:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=cec1ef63ff3f74d98c9fa6b480c7416726927870


-- 
Luiz Augusto von Dentz