Hi, On Wed, May 28, 2025 at 1:21 AM Ye He <ye.he@xxxxxxxxxxx> wrote: > > Hi Luiz, > > > [ EXTERNAL EMAIL ] > > > > From: Luiz Augusto von Dentz<luiz.von.dentz@xxxxxxxxx> > > > > Some attributes may need to handle offset other than 0 in case read long > > procedure is used, so this properly handles that for PAC_SINK_CHRC_UUID > > and PAC_SOURCE_CHRC_UUID. > > > > In addition to PAC record this also uses > > gatt_db_attribute_set_fixed_length for attributes that are considered of > > fixed size so gatt_db can handle offset directly. > > > > Fixes:https://github.com/bluez/bluez/issues/1294 > > --- > > src/shared/bap.c | 28 ++++++++++++++++++++++++---- > > 1 file changed, 24 insertions(+), 4 deletions(-) > > > > diff --git a/src/shared/bap.c b/src/shared/bap.c > > index 4bb6e08b5379..f0c6f64855c4 100644 > > --- a/src/shared/bap.c > > +++ b/src/shared/bap.c > > @@ -516,8 +516,15 @@ static void pacs_sink_read(struct gatt_db_attribute *attrib, > > queue_foreach(bdb->sinks, pac_foreach, &iov); > > queue_foreach(bdb->broadcast_sinks, pac_foreach, &iov); > > > > - gatt_db_attribute_read_result(attrib, id, 0, iov.iov_base, > > - iov.iov_len); > > + if (offset > iov.iov_len) { > > + gatt_db_attribute_read_result(attrib, id, > > + BT_ATT_ERROR_INVALID_OFFSET, > > + NULL, 0); > > + return; > > + } > > + > > + gatt_db_attribute_read_result(attrib, id, 0, iov.iov_base + offset, > > + iov.iov_len - offset); > > } > > > > static void pacs_sink_loc_read(struct gatt_db_attribute *attrib, > > @@ -549,8 +556,15 @@ static void pacs_source_read(struct gatt_db_attribute *attrib, > > > > queue_foreach(bdb->sources, pac_foreach, &iov); > > > > - gatt_db_attribute_read_result(attrib, id, 0, iov.iov_base, > > - iov.iov_len); > > + if (offset > iov.iov_len) { > > + gatt_db_attribute_read_result(attrib, id, > > + BT_ATT_ERROR_INVALID_OFFSET, > > + NULL, 0); > > + return; > > + } > > + > > + gatt_db_attribute_read_result(attrib, id, 0, iov.iov_base + offset, > > + iov.iov_len - offset); > > } > > > > static void pacs_source_loc_read(struct gatt_db_attribute *attrib, > > @@ -627,6 +641,7 @@ static struct bt_pacs *pacs_new(struct gatt_db *db) > > BT_GATT_CHRC_PROP_NOTIFY, > > pacs_sink_loc_read, NULL, > > pacs); > > + gatt_db_attribute_set_fixed_length(pacs->sink_loc, sizeof(uint32_t)); > > > > pacs->sink_loc_ccc = gatt_db_service_add_ccc(pacs->service, > > BT_ATT_PERM_READ | BT_ATT_PERM_WRITE); > > @@ -649,6 +664,7 @@ static struct bt_pacs *pacs_new(struct gatt_db *db) > > BT_GATT_CHRC_PROP_NOTIFY, > > pacs_source_loc_read, NULL, > > pacs); > > + gatt_db_attribute_set_fixed_length(pacs->source_loc, sizeof(uint32_t)); > > > > pacs->source_loc_ccc = gatt_db_service_add_ccc(pacs->service, > > BT_ATT_PERM_READ | BT_ATT_PERM_WRITE); > > @@ -659,6 +675,8 @@ static struct bt_pacs *pacs_new(struct gatt_db *db) > > BT_GATT_CHRC_PROP_READ | > > BT_GATT_CHRC_PROP_NOTIFY, > > pacs_context_read, NULL, pacs); > > + gatt_db_attribute_set_fixed_length(pacs->context, > > + sizeof(struct bt_pacs_context)); > > > > pacs->context_ccc = gatt_db_service_add_ccc(pacs->service, > > BT_ATT_PERM_READ | BT_ATT_PERM_WRITE); > > @@ -671,6 +689,8 @@ static struct bt_pacs *pacs_new(struct gatt_db *db) > > BT_GATT_CHRC_PROP_NOTIFY, > > pacs_supported_context_read, NULL, > > pacs); > > + gatt_db_attribute_set_fixed_length(pacs->supported_context, > > + sizeof(struct bt_pacs_context)); > > > > pacs->supported_context_ccc = gatt_db_service_add_ccc(pacs->service, > > BT_ATT_PERM_READ | BT_ATT_PERM_WRITE); > > -- > > 2.49.0 > > > > > I am concerned about this solution. It can indeed solve the problem of > using blob procedure to read sink pacs or source pacs, but if blob > procedure is used to read other services, will the problem occur again? > I think we should fix the issue at its root. Well Ive already explained that doesn't work since the offset is already part of the interface, so both internal and external attribute are expected to return that data following the offset: https://github.com/bluez/bluez/blob/master/doc/org.bluez.GattCharacteristic.rst#arraybyte-readvaluedict-options Also note that although normally the offset means the offset of data read with a read long operation I wouldn't be surprised if the vendor does use for something else, e.g. as control point index or something like that. What is perhaps more efficient is to cache the read operations so we can detect when it is a read long operation taking place we just use the cache to respond, that said Read Blob that doesn't continue from previous offset shall still be sent to the callback to respond. -- Luiz Augusto von Dentz
