| Message ID | 20210826085906.BlueZ.v1.1.Iae4b26a8036d47ca4d0db470f2bb23247f6cac7d@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [BlueZ,v1] adv_monitor: Clear any running DeviceLost timers on power down | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=537875 ---Test result--- Test Summary: CheckPatch PASS 0.36 seconds GitLint PASS 0.12 seconds Prep - Setup ELL PASS 45.94 seconds Build - Prep PASS 0.17 seconds Build - Configure PASS 8.13 seconds Build - Make PASS 197.05 seconds Make Check PASS 9.18 seconds Make Distcheck PASS 233.12 seconds Build w/ext ELL - Configure PASS 8.12 seconds Build w/ext ELL - Make PASS 185.92 seconds Details ############################## Test: CheckPatch - PASS Desc: Run checkpatch.pl script with rule in .checkpatch.conf ############################## Test: GitLint - PASS Desc: Run gitlint with rule in .gitlint ############################## Test: Prep - Setup ELL - PASS Desc: Clone, build, and install ELL ############################## Test: Build - Prep - PASS Desc: Prepare environment for build ############################## Test: Build - Configure - PASS Desc: Configure the BlueZ source tree ############################## Test: Build - Make - PASS Desc: Build the BlueZ source tree ############################## Test: Make Check - PASS Desc: Run 'make check' ############################## Test: Make Distcheck - PASS Desc: Run distcheck to check the distribution ############################## Test: Build w/ext ELL - Configure - PASS Desc: Configure BlueZ source with '--enable-external-ell' configuration ############################## Test: Build w/ext ELL - Make - PASS Desc: Build BlueZ source with '--enable-external-ell' configuration --- Regards, Linux Bluetooth
Hi Manish, On Thu, Aug 26, 2021 at 8:59 AM 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 > Signed-off-by: Manish Mandlik <mmandlik@google.com> 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 >
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); +} + 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); + + 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 */