| Message ID | e856ad3174024bb61113217cb889005a0bf0ad1c.1678902782.git.pav@iki.fi (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (82>80): "[BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties" |
| tedd_an/BuildEll | fail | Build ELL FAIL: writing RSA key writing RSA key writing RSA key writing RSA key writing RSA key make[1]: *** [Makefile:3277: unit/cert-intca.pem] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1265: all] Error 2 |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | fail | Make External ELL FAIL: configure.ac:21: installing './compile' configure.ac:36: installing './config.guess' configure.ac:36: installing './config.sub' configure.ac:5: installing './install-sh' configure.ac:5: installing './missing' Makefile.am: installing './depcomp' parallel-tests: installing './test-driver' configure: error: Embedded Linux library >= 0.39 is required |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
Hi Pauli, On Wed, Mar 15, 2023 at 10:54 AM Pauli Virtanen <pav@iki.fi> wrote: > > Add CIG, CIS, and PHY properties to BAP transport. The other QoS > properties are there, and these may also be useful to clients, e.g. to > manage CIG/CIS allocation as client. > > Hide transport QoS properties when they are not configured. > --- > profiles/audio/transport.c | 67 ++++++++++++++++++++++++++++++++++---- > 1 file changed, 61 insertions(+), 6 deletions(-) > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c > index 457590746..53bf13175 100644 > --- a/profiles/audio/transport.c > +++ b/profiles/audio/transport.c > @@ -811,6 +811,38 @@ static const GDBusPropertyTable a2dp_properties[] = { > { } > }; > > +static gboolean qos_exists(const GDBusPropertyTable *property, void *data) > +{ > + struct media_transport *transport = data; > + struct bap_transport *bap = transport->data; > + > + return bap->qos.phy != 0x00; > +} > + > +static gboolean get_cig(const GDBusPropertyTable *property, > + DBusMessageIter *iter, void *data) > +{ > + struct media_transport *transport = data; > + struct bap_transport *bap = transport->data; > + > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, > + &bap->qos.cig_id); > + > + return TRUE; > +} > + > +static gboolean get_cis(const GDBusPropertyTable *property, > + DBusMessageIter *iter, void *data) > +{ > + struct media_transport *transport = data; > + struct bap_transport *bap = transport->data; > + > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, > + &bap->qos.cis_id); > + > + return TRUE; > +} > + > static gboolean get_interval(const GDBusPropertyTable *property, > DBusMessageIter *iter, void *data) > { > @@ -835,6 +867,17 @@ static gboolean get_framing(const GDBusPropertyTable *property, > return TRUE; > } > > +static gboolean get_phy(const GDBusPropertyTable *property, > + DBusMessageIter *iter, void *data) > +{ > + struct media_transport *transport = data; > + struct bap_transport *bap = transport->data; > + > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.phy); > + > + return TRUE; > +} > + > static gboolean get_sdu(const GDBusPropertyTable *property, > DBusMessageIter *iter, void *data) > { > @@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] = { > { "Codec", "y", get_codec }, > { "Configuration", "ay", get_configuration }, > { "State", "s", get_state }, > - { "Interval", "u", get_interval }, > - { "Framing", "b", get_framing }, > - { "SDU", "q", get_sdu }, > - { "Retransmissions", "y", get_retransmissions }, > - { "Latency", "q", get_latency }, > - { "Delay", "u", get_delay }, > + { "CIG", "y", get_cig, NULL, qos_exists }, > + { "CIS", "y", get_cis, NULL, qos_exists }, > + { "Interval", "u", get_interval, NULL, qos_exists }, > + { "Framing", "b", get_framing, NULL, qos_exists }, > + { "PHY", "y", get_phy, NULL, qos_exists }, > + { "SDU", "q", get_sdu, NULL, qos_exists }, > + { "Retransmissions", "y", get_retransmissions, NULL, qos_exists }, > + { "Latency", "q", get_latency, NULL, qos_exists }, > + { "Delay", "u", get_delay, NULL, qos_exists }, > { "Endpoint", "o", get_endpoint, NULL, endpoint_exists }, > { "Location", "u", get_location }, > { "Metadata", "ay", get_metadata }, > @@ -1191,12 +1237,21 @@ static void bap_update_qos(const struct media_transport *transport) > > bap->qos = *qos; > > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > + transport->path, MEDIA_TRANSPORT_INTERFACE, > + "CIG"); > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > + transport->path, MEDIA_TRANSPORT_INTERFACE, > + "CIS"); > g_dbus_emit_property_changed(btd_get_dbus_connection(), > transport->path, MEDIA_TRANSPORT_INTERFACE, > "Interval"); > g_dbus_emit_property_changed(btd_get_dbus_connection(), > transport->path, MEDIA_TRANSPORT_INTERFACE, > "Framing"); > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > + transport->path, MEDIA_TRANSPORT_INTERFACE, > + "PHY"); > g_dbus_emit_property_changed(btd_get_dbus_connection(), > transport->path, MEDIA_TRANSPORT_INTERFACE, > "SDU"); > -- > 2.39.2 I'm fine adding these but you could also have used BT_ISO_QOS socketopt to read it directly from the socket in case you want to use it on pipewire.
Hi Luiz, ke, 2023-03-15 kello 11:19 -0700, Luiz Augusto von Dentz kirjoitti: > Hi Pauli, > > On Wed, Mar 15, 2023 at 10:54 AM Pauli Virtanen <pav@iki.fi> wrote: > > > > Add CIG, CIS, and PHY properties to BAP transport. The other QoS > > properties are there, and these may also be useful to clients, e.g. to > > manage CIG/CIS allocation as client. > > > > Hide transport QoS properties when they are not configured. > > --- > > profiles/audio/transport.c | 67 ++++++++++++++++++++++++++++++++++---- > > 1 file changed, 61 insertions(+), 6 deletions(-) > > > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c > > index 457590746..53bf13175 100644 > > --- a/profiles/audio/transport.c > > +++ b/profiles/audio/transport.c > > @@ -811,6 +811,38 @@ static const GDBusPropertyTable a2dp_properties[] = { > > { } > > }; > > > > +static gboolean qos_exists(const GDBusPropertyTable *property, void *data) > > +{ > > + struct media_transport *transport = data; > > + struct bap_transport *bap = transport->data; > > + > > + return bap->qos.phy != 0x00; > > +} > > + > > +static gboolean get_cig(const GDBusPropertyTable *property, > > + DBusMessageIter *iter, void *data) > > +{ > > + struct media_transport *transport = data; > > + struct bap_transport *bap = transport->data; > > + > > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, > > + &bap->qos.cig_id); > > + > > + return TRUE; > > +} > > + > > +static gboolean get_cis(const GDBusPropertyTable *property, > > + DBusMessageIter *iter, void *data) > > +{ > > + struct media_transport *transport = data; > > + struct bap_transport *bap = transport->data; > > + > > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, > > + &bap->qos.cis_id); > > + > > + return TRUE; > > +} > > + > > static gboolean get_interval(const GDBusPropertyTable *property, > > DBusMessageIter *iter, void *data) > > { > > @@ -835,6 +867,17 @@ static gboolean get_framing(const GDBusPropertyTable *property, > > return TRUE; > > } > > > > +static gboolean get_phy(const GDBusPropertyTable *property, > > + DBusMessageIter *iter, void *data) > > +{ > > + struct media_transport *transport = data; > > + struct bap_transport *bap = transport->data; > > + > > + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.phy); > > + > > + return TRUE; > > +} > > + > > static gboolean get_sdu(const GDBusPropertyTable *property, > > DBusMessageIter *iter, void *data) > > { > > @@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] = { > > { "Codec", "y", get_codec }, > > { "Configuration", "ay", get_configuration }, > > { "State", "s", get_state }, > > - { "Interval", "u", get_interval }, > > - { "Framing", "b", get_framing }, > > - { "SDU", "q", get_sdu }, > > - { "Retransmissions", "y", get_retransmissions }, > > - { "Latency", "q", get_latency }, > > - { "Delay", "u", get_delay }, > > + { "CIG", "y", get_cig, NULL, qos_exists }, > > + { "CIS", "y", get_cis, NULL, qos_exists }, > > + { "Interval", "u", get_interval, NULL, qos_exists }, > > + { "Framing", "b", get_framing, NULL, qos_exists }, > > + { "PHY", "y", get_phy, NULL, qos_exists }, > > + { "SDU", "q", get_sdu, NULL, qos_exists }, > > + { "Retransmissions", "y", get_retransmissions, NULL, qos_exists }, > > + { "Latency", "q", get_latency, NULL, qos_exists }, > > + { "Delay", "u", get_delay, NULL, qos_exists }, > > { "Endpoint", "o", get_endpoint, NULL, endpoint_exists }, > > { "Location", "u", get_location }, > > { "Metadata", "ay", get_metadata }, > > @@ -1191,12 +1237,21 @@ static void bap_update_qos(const struct media_transport *transport) > > > > bap->qos = *qos; > > > > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > > + transport->path, MEDIA_TRANSPORT_INTERFACE, > > + "CIG"); > > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > > + transport->path, MEDIA_TRANSPORT_INTERFACE, > > + "CIS"); > > g_dbus_emit_property_changed(btd_get_dbus_connection(), > > transport->path, MEDIA_TRANSPORT_INTERFACE, > > "Interval"); > > g_dbus_emit_property_changed(btd_get_dbus_connection(), > > transport->path, MEDIA_TRANSPORT_INTERFACE, > > "Framing"); > > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > > + transport->path, MEDIA_TRANSPORT_INTERFACE, > > + "PHY"); > > g_dbus_emit_property_changed(btd_get_dbus_connection(), > > transport->path, MEDIA_TRANSPORT_INTERFACE, > > "SDU"); > > -- > > 2.39.2 > > I'm fine adding these but you could also have used BT_ISO_QOS > socketopt to read it directly from the socket in case you want to use > it on pipewire. Yes, we can do that once we have the fd. But if client wants to do CIG+CIS allocation in its SelectProperties, it needs to know already reserved CIG+CIS. Acquiring the fd moves ASEs out from qos state, which is not wanted for this. Also, no Acquire call will return until all CIS in the same CIG have been connected. To acquire the right transports (only those in the same CIG), we need to know CIGs of all transports before starting to acquire any. So we don't have any fds at that point. Managing that probably should be responsibility of BlueZ, it could connect all CIS in the same CIG when one of the transports is acquired. Or, try address it in kernel but maybe that has more constraints. But I think these properties can be added regardless of the above, given that they have some uses also otherwise.
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=730450 ---Test result--- Test Summary: CheckPatch PASS 0.90 seconds GitLint FAIL 0.88 seconds BuildEll FAIL 21.98 seconds BluezMake PASS 863.86 seconds MakeCheck PASS 11.04 seconds MakeDistcheck PASS 151.92 seconds CheckValgrind PASS 248.09 seconds CheckSmatch PASS 329.13 seconds bluezmakeextell FAIL 7.32 seconds IncrementalBuild PASS 1432.04 seconds ScanBuild PASS 1036.93 seconds Details ############################## Test: GitLint - FAIL Desc: Run gitlint Output: [BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (82>80): "[BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties" ############################## Test: BuildEll - FAIL Desc: Build and Install ELL Output: writing RSA key writing RSA key writing RSA key writing RSA key writing RSA key make[1]: *** [Makefile:3277: unit/cert-intca.pem] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1265: all] Error 2 ############################## Test: bluezmakeextell - FAIL Desc: Build Bluez with External ELL Output: configure.ac:21: installing './compile' configure.ac:36: installing './config.guess' configure.ac:36: installing './config.sub' configure.ac:5: installing './install-sh' configure.ac:5: installing './missing' Makefile.am: installing './depcomp' parallel-tests: installing './test-driver' configure: error: Embedded Linux library >= 0.39 is required --- Regards, Linux Bluetooth
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c index 457590746..53bf13175 100644 --- a/profiles/audio/transport.c +++ b/profiles/audio/transport.c @@ -811,6 +811,38 @@ static const GDBusPropertyTable a2dp_properties[] = { { } }; +static gboolean qos_exists(const GDBusPropertyTable *property, void *data) +{ + struct media_transport *transport = data; + struct bap_transport *bap = transport->data; + + return bap->qos.phy != 0x00; +} + +static gboolean get_cig(const GDBusPropertyTable *property, + DBusMessageIter *iter, void *data) +{ + struct media_transport *transport = data; + struct bap_transport *bap = transport->data; + + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, + &bap->qos.cig_id); + + return TRUE; +} + +static gboolean get_cis(const GDBusPropertyTable *property, + DBusMessageIter *iter, void *data) +{ + struct media_transport *transport = data; + struct bap_transport *bap = transport->data; + + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, + &bap->qos.cis_id); + + return TRUE; +} + static gboolean get_interval(const GDBusPropertyTable *property, DBusMessageIter *iter, void *data) { @@ -835,6 +867,17 @@ static gboolean get_framing(const GDBusPropertyTable *property, return TRUE; } +static gboolean get_phy(const GDBusPropertyTable *property, + DBusMessageIter *iter, void *data) +{ + struct media_transport *transport = data; + struct bap_transport *bap = transport->data; + + dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.phy); + + return TRUE; +} + static gboolean get_sdu(const GDBusPropertyTable *property, DBusMessageIter *iter, void *data) { @@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] = { { "Codec", "y", get_codec }, { "Configuration", "ay", get_configuration }, { "State", "s", get_state }, - { "Interval", "u", get_interval }, - { "Framing", "b", get_framing }, - { "SDU", "q", get_sdu }, - { "Retransmissions", "y", get_retransmissions }, - { "Latency", "q", get_latency }, - { "Delay", "u", get_delay }, + { "CIG", "y", get_cig, NULL, qos_exists }, + { "CIS", "y", get_cis, NULL, qos_exists }, + { "Interval", "u", get_interval, NULL, qos_exists }, + { "Framing", "b", get_framing, NULL, qos_exists }, + { "PHY", "y", get_phy, NULL, qos_exists }, + { "SDU", "q", get_sdu, NULL, qos_exists }, + { "Retransmissions", "y", get_retransmissions, NULL, qos_exists }, + { "Latency", "q", get_latency, NULL, qos_exists }, + { "Delay", "u", get_delay, NULL, qos_exists }, { "Endpoint", "o", get_endpoint, NULL, endpoint_exists }, { "Location", "u", get_location }, { "Metadata", "ay", get_metadata }, @@ -1191,12 +1237,21 @@ static void bap_update_qos(const struct media_transport *transport) bap->qos = *qos; + g_dbus_emit_property_changed(btd_get_dbus_connection(), + transport->path, MEDIA_TRANSPORT_INTERFACE, + "CIG"); + g_dbus_emit_property_changed(btd_get_dbus_connection(), + transport->path, MEDIA_TRANSPORT_INTERFACE, + "CIS"); g_dbus_emit_property_changed(btd_get_dbus_connection(), transport->path, MEDIA_TRANSPORT_INTERFACE, "Interval"); g_dbus_emit_property_changed(btd_get_dbus_connection(), transport->path, MEDIA_TRANSPORT_INTERFACE, "Framing"); + g_dbus_emit_property_changed(btd_get_dbus_connection(), + transport->path, MEDIA_TRANSPORT_INTERFACE, + "PHY"); g_dbus_emit_property_changed(btd_get_dbus_connection(), transport->path, MEDIA_TRANSPORT_INTERFACE, "SDU");