Hi Manish,
On Thu, Aug 26, 2021 at 8:59 AM Manish Mandlik <mmandlik@xxxxxxxxxx> 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@xxxxxxxxxx
> Signed-off-by: Manish Mandlik <mmandlik@xxxxxxxxxx>
We don't use signed-off-by on userspace.
> ---
>
> src/adapter.c | 8 ++++++++
> src/adv_monitor.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
> src/adv_monitor.h | 2 ++
> 3 files changed, 62 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 84bc5a1b0..3af7d1f1a 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -2872,6 +2872,13 @@ static void clear_discoverable(struct btd_adapter *adapter)
> set_mode(adapter, MGMT_OP_SET_CONNECTABLE, 0x00);
> }
>
> +static void adv_monitor_notify_power_down(struct btd_adapter *adapter)
> +{
> + /* Notify Adv Monitor about power down */
> + if (adapter->adv_monitor_manager)
> + btd_adv_monitor_notify_power_down(adapter->adv_monitor_manager);
> +}
I guess we could just check for NULL inside btd_adv_monitor*, also I
would rename it to btd_adv_monitor_power_down.
> static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
> DBusMessageIter *value,
> GDBusPendingPropertySet id)
> @@ -2912,6 +2919,7 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
> len = sizeof(mode);
>
> if (!mode) {
> + adv_monitor_notify_power_down(adapter);
> clear_discoverable(adapter);
> remove_temporary_devices(adapter);
> }
> diff --git a/src/adv_monitor.c b/src/adv_monitor.c
> index 715ac5904..59f307ae9 100644
> --- a/src/adv_monitor.c
> +++ b/src/adv_monitor.c
> @@ -2011,3 +2011,55 @@ 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("Clearing device lost timer for device %p. "
> + "Calling DeviceLost() on Adv Monitor of "
> + "owner %s at path %s", dev->device,
> + monitor->app->owner, monitor->path);
The function name should already give the information about clearing
device lost timer so I'm not why you have to be so verbose about this
action.
> + 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;
> + }
So you are doing the manager pointer check so it is even more a reason
to not have a wrapper around it.
> + /* 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.rc2.250.ged5fa647cd-goog
>
--
Luiz Augusto von Dentz
