| Message ID | 20210708142059.Bluez.v1.5.I699a3837a5a18d9f889635300f6c717879f19dd2@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [Bluez,v1,01/14] lib: add hash functions for bt_uuid_t | expand |
Hi Howard, On Wed, Jul 7, 2021 at 11:23 PM Howard Chung <howardchung@google.com> wrote: > > From: Yun-Hao Chung <howardchung@chromium.org> > > This implements functions to register/unregister state changed callback > functions, the functions will be called when a device's state changed. > Currently the state only shows initializing, available and removing, > which is enough for plugins to register dbus objects upon device > creation and unregister it upon device removal. > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org> > --- > > src/device.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/device.h | 13 +++++++++++ > 2 files changed, 77 insertions(+) > > diff --git a/src/device.c b/src/device.c > index e1d82eab0988..0d7444706336 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -81,6 +81,13 @@ > > static DBusConnection *dbus_conn = NULL; > static unsigned service_state_cb_id; > +static GSList *device_state_callbacks; Use a struct queue instead of GSList. > + > +struct device_state_callback { > + btd_device_state_cb cb; > + void *user_data; > + unsigned int id; > +}; > > struct btd_disconnect_data { > guint id; > @@ -272,6 +279,8 @@ struct btd_device { > > GIOChannel *att_io; > guint store_id; > + > + enum btd_device_state_t state; > }; > > static const uint16_t uuid_list[] = { > @@ -4095,6 +4104,23 @@ static void gatt_service_removed(struct gatt_db_attribute *attr, > gatt_services_changed(device); > } > > +static void device_change_state(struct btd_device *device, > + enum btd_device_state_t new_state) > +{ > + GSList *l; > + struct device_state_callback *cb_data; > + > + if (device->state == new_state) > + return; > + > + for (l = device_state_callbacks; l != NULL; l = g_slist_next(l)) { > + cb_data = l->data; > + cb_data->cb(device, new_state, cb_data->user_data); > + } > + > + device->state = new_state; > +} > + > static struct btd_device *device_new(struct btd_adapter *adapter, > const char *address) > { > @@ -4158,6 +4184,8 @@ static struct btd_device *device_new(struct btd_adapter *adapter, > > device->refresh_discovery = btd_opts.refresh_discovery; > > + device_change_state(device, BTD_DEVICE_STATE_AVAILABLE); > + > return btd_device_ref(device); > } > > @@ -6839,6 +6867,7 @@ void btd_device_unref(struct btd_device *device) > > DBG("Freeing device %s", device->path); > > + device_change_state(device, BTD_DEVICE_STATE_REMOVING); > g_dbus_unregister_interface(dbus_conn, device->path, DEVICE_INTERFACE); > } > > @@ -6980,3 +7009,38 @@ void btd_device_cleanup(void) > { > btd_service_remove_state_cb(service_state_cb_id); > } > + > +unsigned int btd_device_add_state_cb(btd_device_state_cb cb, void *user_data) > +{ > + struct device_state_callback *cb_data; > + static unsigned int id; > + > + cb_data = g_new0(struct device_state_callback, 1); > + cb_data->cb = cb; > + cb_data->user_data = user_data; > + cb_data->id = ++id; > + > + device_state_callbacks = g_slist_append(device_state_callbacks, > + cb_data); > + > + return cb_data->id; > +} > + > +bool btd_device_remove_state_cb(unsigned int id) > +{ > + GSList *l; > + > + for (l = device_state_callbacks; l != NULL; l = g_slist_next(l)) { > + struct device_state_callback *cb_data = l->data; > + > + if (cb_data && cb_data->id == id) { > + device_state_callbacks = g_slist_remove( > + device_state_callbacks, > + cb_data); > + g_free(cb_data); > + return true; > + } > + } > + > + return false; > +} > diff --git a/src/device.h b/src/device.h > index 5f615cb4b6b2..a8424aa4f098 100644 > --- a/src/device.h > +++ b/src/device.h > @@ -11,8 +11,18 @@ > > #define DEVICE_INTERFACE "org.bluez.Device1" > > +enum btd_device_state_t { > + BTD_DEVICE_STATE_INITIALIZING, /* Device object is creating */ > + BTD_DEVICE_STATE_AVAILABLE, /* Device object is registered */ > + BTD_DEVICE_STATE_REMOVING, /* Device object is being removed */ > +}; I got a bad feeling about adding this sort of state, are you trying to cover some use case that we can't do with btd_service_add_state_cb? It does seem a lot like it but at device level. > + > struct btd_device; > > +typedef void (*btd_device_state_cb) (struct btd_device *device, > + enum btd_device_state_t new_state, > + void *user_data); > + > struct btd_device *device_create(struct btd_adapter *adapter, > const bdaddr_t *address, uint8_t bdaddr_type); > struct btd_device *device_create_from_storage(struct btd_adapter *adapter, > @@ -179,3 +189,6 @@ bool btd_device_all_services_allowed(struct btd_device *dev); > void btd_device_update_allowed_services(struct btd_device *dev); > void btd_device_init(void); > void btd_device_cleanup(void); > + > +unsigned int btd_device_add_state_cb(btd_device_state_cb cb, void *user_data); > +bool btd_device_remove_state_cb(unsigned int id); > -- > 2.32.0.93.g670b81a890-goog >
On Fri, Jul 9, 2021 at 1:34 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Howard, > > On Wed, Jul 7, 2021 at 11:23 PM Howard Chung <howardchung@google.com> wrote: > > > > From: Yun-Hao Chung <howardchung@chromium.org> > > > > This implements functions to register/unregister state changed callback > > functions, the functions will be called when a device's state changed. > > Currently the state only shows initializing, available and removing, > > which is enough for plugins to register dbus objects upon device > > creation and unregister it upon device removal. > > > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org> > > --- > > > > src/device.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > src/device.h | 13 +++++++++++ > > 2 files changed, 77 insertions(+) > > > > diff --git a/src/device.c b/src/device.c > > index e1d82eab0988..0d7444706336 100644 > > --- a/src/device.c > > +++ b/src/device.c > > @@ -81,6 +81,13 @@ > > > > static DBusConnection *dbus_conn = NULL; > > static unsigned service_state_cb_id; > > +static GSList *device_state_callbacks; > > Use a struct queue instead of GSList. Will do. > > > + > > +struct device_state_callback { > > + btd_device_state_cb cb; > > + void *user_data; > > + unsigned int id; > > +}; > > > > struct btd_disconnect_data { > > guint id; > > @@ -272,6 +279,8 @@ struct btd_device { > > > > GIOChannel *att_io; > > guint store_id; > > + > > + enum btd_device_state_t state; > > }; > > > > static const uint16_t uuid_list[] = { > > @@ -4095,6 +4104,23 @@ static void gatt_service_removed(struct gatt_db_attribute *attr, > > gatt_services_changed(device); > > } > > > > +static void device_change_state(struct btd_device *device, > > + enum btd_device_state_t new_state) > > +{ > > + GSList *l; > > + struct device_state_callback *cb_data; > > + > > + if (device->state == new_state) > > + return; > > + > > + for (l = device_state_callbacks; l != NULL; l = g_slist_next(l)) { > > + cb_data = l->data; > > + cb_data->cb(device, new_state, cb_data->user_data); > > + } > > + > > + device->state = new_state; > > +} > > + > > static struct btd_device *device_new(struct btd_adapter *adapter, > > const char *address) > > { > > @@ -4158,6 +4184,8 @@ static struct btd_device *device_new(struct btd_adapter *adapter, > > > > device->refresh_discovery = btd_opts.refresh_discovery; > > > > + device_change_state(device, BTD_DEVICE_STATE_AVAILABLE); > > + > > return btd_device_ref(device); > > } > > > > @@ -6839,6 +6867,7 @@ void btd_device_unref(struct btd_device *device) > > > > DBG("Freeing device %s", device->path); > > > > + device_change_state(device, BTD_DEVICE_STATE_REMOVING); > > g_dbus_unregister_interface(dbus_conn, device->path, DEVICE_INTERFACE); > > } > > > > @@ -6980,3 +7009,38 @@ void btd_device_cleanup(void) > > { > > btd_service_remove_state_cb(service_state_cb_id); > > } > > + > > +unsigned int btd_device_add_state_cb(btd_device_state_cb cb, void *user_data) > > +{ > > + struct device_state_callback *cb_data; > > + static unsigned int id; > > + > > + cb_data = g_new0(struct device_state_callback, 1); > > + cb_data->cb = cb; > > + cb_data->user_data = user_data; > > + cb_data->id = ++id; > > + > > + device_state_callbacks = g_slist_append(device_state_callbacks, > > + cb_data); > > + > > + return cb_data->id; > > +} > > + > > +bool btd_device_remove_state_cb(unsigned int id) > > +{ > > + GSList *l; > > + > > + for (l = device_state_callbacks; l != NULL; l = g_slist_next(l)) { > > + struct device_state_callback *cb_data = l->data; > > + > > + if (cb_data && cb_data->id == id) { > > + device_state_callbacks = g_slist_remove( > > + device_state_callbacks, > > + cb_data); > > + g_free(cb_data); > > + return true; > > + } > > + } > > + > > + return false; > > +} > > diff --git a/src/device.h b/src/device.h > > index 5f615cb4b6b2..a8424aa4f098 100644 > > --- a/src/device.h > > +++ b/src/device.h > > @@ -11,8 +11,18 @@ > > > > #define DEVICE_INTERFACE "org.bluez.Device1" > > > > +enum btd_device_state_t { > > + BTD_DEVICE_STATE_INITIALIZING, /* Device object is creating */ > > + BTD_DEVICE_STATE_AVAILABLE, /* Device object is registered */ > > + BTD_DEVICE_STATE_REMOVING, /* Device object is being removed */ > > +}; > > I got a bad feeling about adding this sort of state, are you trying to > cover some use case that we can't do with btd_service_add_state_cb? It > does seem a lot like it but at device level. I added this so that the admin plugin can know whenever a device object is created or removed. I just learned that we can create a dbus client to listen for new device objects. If it sounds better, I'll do it in this way. > > > + > > struct btd_device; > > > > +typedef void (*btd_device_state_cb) (struct btd_device *device, > > + enum btd_device_state_t new_state, > > + void *user_data); > > + > > struct btd_device *device_create(struct btd_adapter *adapter, > > const bdaddr_t *address, uint8_t bdaddr_type); > > struct btd_device *device_create_from_storage(struct btd_adapter *adapter, > > @@ -179,3 +189,6 @@ bool btd_device_all_services_allowed(struct btd_device *dev); > > void btd_device_update_allowed_services(struct btd_device *dev); > > void btd_device_init(void); > > void btd_device_cleanup(void); > > + > > +unsigned int btd_device_add_state_cb(btd_device_state_cb cb, void *user_data); > > +bool btd_device_remove_state_cb(unsigned int id); > > -- > > 2.32.0.93.g670b81a890-goog > > > > > -- > Luiz Augusto von Dentz
diff --git a/src/device.c b/src/device.c index e1d82eab0988..0d7444706336 100644 --- a/src/device.c +++ b/src/device.c @@ -81,6 +81,13 @@ static DBusConnection *dbus_conn = NULL; static unsigned service_state_cb_id; +static GSList *device_state_callbacks; + +struct device_state_callback { + btd_device_state_cb cb; + void *user_data; + unsigned int id; +}; struct btd_disconnect_data { guint id; @@ -272,6 +279,8 @@ struct btd_device { GIOChannel *att_io; guint store_id; + + enum btd_device_state_t state; }; static const uint16_t uuid_list[] = { @@ -4095,6 +4104,23 @@ static void gatt_service_removed(struct gatt_db_attribute *attr, gatt_services_changed(device); } +static void device_change_state(struct btd_device *device, + enum btd_device_state_t new_state) +{ + GSList *l; + struct device_state_callback *cb_data; + + if (device->state == new_state) + return; + + for (l = device_state_callbacks; l != NULL; l = g_slist_next(l)) { + cb_data = l->data; + cb_data->cb(device, new_state, cb_data->user_data); + } + + device->state = new_state; +} + static struct btd_device *device_new(struct btd_adapter *adapter, const char *address) { @@ -4158,6 +4184,8 @@ static struct btd_device *device_new(struct btd_adapter *adapter, device->refresh_discovery = btd_opts.refresh_discovery; + device_change_state(device, BTD_DEVICE_STATE_AVAILABLE); + return btd_device_ref(device); } @@ -6839,6 +6867,7 @@ void btd_device_unref(struct btd_device *device) DBG("Freeing device %s", device->path); + device_change_state(device, BTD_DEVICE_STATE_REMOVING); g_dbus_unregister_interface(dbus_conn, device->path, DEVICE_INTERFACE); } @@ -6980,3 +7009,38 @@ void btd_device_cleanup(void) { btd_service_remove_state_cb(service_state_cb_id); } + +unsigned int btd_device_add_state_cb(btd_device_state_cb cb, void *user_data) +{ + struct device_state_callback *cb_data; + static unsigned int id; + + cb_data = g_new0(struct device_state_callback, 1); + cb_data->cb = cb; + cb_data->user_data = user_data; + cb_data->id = ++id; + + device_state_callbacks = g_slist_append(device_state_callbacks, + cb_data); + + return cb_data->id; +} + +bool btd_device_remove_state_cb(unsigned int id) +{ + GSList *l; + + for (l = device_state_callbacks; l != NULL; l = g_slist_next(l)) { + struct device_state_callback *cb_data = l->data; + + if (cb_data && cb_data->id == id) { + device_state_callbacks = g_slist_remove( + device_state_callbacks, + cb_data); + g_free(cb_data); + return true; + } + } + + return false; +} diff --git a/src/device.h b/src/device.h index 5f615cb4b6b2..a8424aa4f098 100644 --- a/src/device.h +++ b/src/device.h @@ -11,8 +11,18 @@ #define DEVICE_INTERFACE "org.bluez.Device1" +enum btd_device_state_t { + BTD_DEVICE_STATE_INITIALIZING, /* Device object is creating */ + BTD_DEVICE_STATE_AVAILABLE, /* Device object is registered */ + BTD_DEVICE_STATE_REMOVING, /* Device object is being removed */ +}; + struct btd_device; +typedef void (*btd_device_state_cb) (struct btd_device *device, + enum btd_device_state_t new_state, + void *user_data); + struct btd_device *device_create(struct btd_adapter *adapter, const bdaddr_t *address, uint8_t bdaddr_type); struct btd_device *device_create_from_storage(struct btd_adapter *adapter, @@ -179,3 +189,6 @@ bool btd_device_all_services_allowed(struct btd_device *dev); void btd_device_update_allowed_services(struct btd_device *dev); void btd_device_init(void); void btd_device_cleanup(void); + +unsigned int btd_device_add_state_cb(btd_device_state_cb cb, void *user_data); +bool btd_device_remove_state_cb(unsigned int id);