| Message ID | 20201116233910.4128702-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,BlueZ,1/2] avdtp: Fix connecting using streaming mode with signalling channel | 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=385541
---Test result---
##############################
Test: CheckPatch - FAIL
Output:
main.conf: Add option to configure AVDP session/stream channel modes
WARNING:TYPO_SPELLING: 'prefered' may be misspelled - perhaps 'preferred'?
#7:
This adds a new group AVDTP where platform can confure the prefered
WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
#101: FILE: src/main.c:141:
+static const char *avdtp_options[] = {
- total: 0 errors, 2 warnings, 150 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] main.conf: Add option to configure AVDP session/stream" has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
---
Regards,
Linux Bluetooth
On Mon, Nov 16, 2020 at 7:26 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This adds a new group AVDTP where platform can confure the prefered > modes L2CAP channel for both session (signalling) and stream > (transport). For better backward compatibility the default modes of > boths Reviewed-by: Alain Michaud <alainm@chromium.org> Tested-by: Alain Michaud <alainm@chromium.org> > --- > profiles/audio/a2dp.c | 5 +---- > profiles/audio/avdtp.c | 14 ++----------- > src/btd.h | 7 +++++++ > src/main.c | 45 ++++++++++++++++++++++++++++++++++++++++++ > src/main.conf | 13 ++++++++++++ > 5 files changed, 68 insertions(+), 16 deletions(-) > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > index 626f61d34..59d11a0aa 100644 > --- a/profiles/audio/a2dp.c > +++ b/profiles/audio/a2dp.c > @@ -2324,10 +2324,7 @@ static bool a2dp_server_listen(struct a2dp_server *server) > if (server->io) > return true; > > - if (btd_opts.mps == MPS_OFF) > - mode = BT_IO_MODE_BASIC; > - else > - mode = BT_IO_MODE_STREAMING; > + mode = btd_opts.avdtp.session_mode; > > server->io = bt_io_listen(NULL, confirm_cb, server, NULL, &err, > BT_IO_OPT_SOURCE_BDADDR, > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c > index 619b94e29..ff0a124c3 100644 > --- a/profiles/audio/avdtp.c > +++ b/profiles/audio/avdtp.c > @@ -2603,12 +2603,7 @@ static int send_req(struct avdtp *session, gboolean priority, > int err, timeout; > > if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) { > - BtIOMode mode; > - > - if (btd_opts.mps == MPS_OFF) > - mode = BT_IO_MODE_BASIC; > - else > - mode = BT_IO_MODE_ERTM; > + BtIOMode mode = btd_opts.avdtp.session_mode; > > session->io = l2cap_connect(session, mode); > if (!session->io) { > @@ -2807,12 +2802,7 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre > struct seid_rej *resp, int size) > { > struct avdtp_local_sep *sep = stream->lsep; > - BtIOMode mode; > - > - if (btd_opts.mps == MPS_OFF) > - mode = BT_IO_MODE_BASIC; > - else > - mode = BT_IO_MODE_STREAMING; > + BtIOMode mode = btd_opts.avdtp.stream_mode; > > stream->io = l2cap_connect(session, mode); > if (!stream->io) { > diff --git a/src/btd.h b/src/btd.h > index c98414e35..a3247e4fd 100644 > --- a/src/btd.h > +++ b/src/btd.h > @@ -84,6 +84,11 @@ struct btd_defaults { > struct btd_le_defaults le; > }; > > +struct btd_avdtp_opts { > + uint8_t session_mode; > + uint8_t stream_mode; > +}; > + > struct btd_opts { > char *name; > uint32_t class; > @@ -112,6 +117,8 @@ struct btd_opts { > uint8_t gatt_channels; > enum mps_mode_t mps; > > + struct btd_avdtp_opts avdtp; > + > uint8_t key_size; > > enum jw_repairing_t jw_repairing; > diff --git a/src/main.c b/src/main.c > index e6c4d861e..33c0f0d15 100644 > --- a/src/main.c > +++ b/src/main.c > @@ -34,6 +34,7 @@ > #include "lib/sdp.h" > > #include "gdbus/gdbus.h" > +#include "btio/btio.h" > > #include "log.h" > #include "backtrace.h" > @@ -137,6 +138,12 @@ static const char *gatt_options[] = { > NULL > }; > > +static const char *avdtp_options[] = { > + "SessionMode", > + "StreamMode", > + NULL > +}; > + > static const struct group_table { > const char *name; > const char **options; > @@ -146,6 +153,7 @@ static const struct group_table { > { "LE", le_options }, > { "Policy", policy_options }, > { "GATT", gatt_options }, > + { "AVDTP", avdtp_options }, > { } > }; > > @@ -744,6 +752,40 @@ static void parse_config(GKeyFile *config) > btd_opts.gatt_channels = val; > } > > + str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err); > + if (err) { > + DBG("%s", err->message); > + g_clear_error(&err); > + } else { > + DBG("SessionMode=%s", str); > + > + if (!strcmp(str, "basic")) > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC; > + else if (!strcmp(str, "ertm")) > + btd_opts.avdtp.session_mode = BT_IO_MODE_ERTM; > + else { > + DBG("Invalid mode option: %s", str); > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC; > + } > + } > + > + val = g_key_file_get_integer(config, "AVDTP", "StreamMode", &err); > + if (err) { > + DBG("%s", err->message); > + g_clear_error(&err); > + } else { > + DBG("StreamMode=%s", str); > + > + if (!strcmp(str, "basic")) > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC; > + else if (!strcmp(str, "streaming")) > + btd_opts.avdtp.stream_mode = BT_IO_MODE_STREAMING; > + else { > + DBG("Invalid mode option: %s", str); > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC; > + } > + } > + > parse_br_config(config); > parse_le_config(config); > } > @@ -780,6 +822,9 @@ static void init_defaults(void) > btd_opts.gatt_cache = BT_GATT_CACHE_ALWAYS; > btd_opts.gatt_mtu = BT_ATT_MAX_LE_MTU; > btd_opts.gatt_channels = 3; > + > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC; > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC; > } > > static void log_handler(const gchar *log_domain, GLogLevelFlags log_level, > diff --git a/src/main.conf b/src/main.conf > index 54f6a36bd..d3bc61441 100644 > --- a/src/main.conf > +++ b/src/main.conf > @@ -200,6 +200,19 @@ > # Default to 3 > #Channels = 3 > > +[AVDTP] > +# AVDTP L2CAP Signalling Channel Mode. > +# Possible values: > +# basic: Use L2CAP Basic Mode > +# ertm: Use L2CAP Enhanced Retransmission Mode > +#SessionMode = basic > + > +# AVDTP L2CAP Transport Channel Mode. > +# Possible values: > +# basic: Use L2CAP Basic Mode > +# streaming: Use L2CAP Streaming Mode > +#StreamMode = basic > + > [Policy] > # > # The ReconnectUUIDs defines the set of remote services that should try > -- > 2.26.2 >
On Mon, Nov 16, 2020 at 7:22 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > Signalling channel shall only use Basic or ERTM modes. Reviewed-by: Alain Michaud <alainm@chromium.org> Tested-by: Alain Michaud <alainm@chromium.org> > --- > profiles/audio/avdtp.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c > index 16fa20bba..619b94e29 100644 > --- a/profiles/audio/avdtp.c > +++ b/profiles/audio/avdtp.c > @@ -2429,20 +2429,13 @@ uint16_t avdtp_get_version(struct avdtp *session) > return session->version; > } > > -static GIOChannel *l2cap_connect(struct avdtp *session) > +static GIOChannel *l2cap_connect(struct avdtp *session, BtIOMode mode) > { > GError *err = NULL; > GIOChannel *io; > const bdaddr_t *src; > - BtIOMode mode; > - > src = btd_adapter_get_address(device_get_adapter(session->device)); > > - if (btd_opts.mps == MPS_OFF) > - mode = BT_IO_MODE_BASIC; > - else > - mode = BT_IO_MODE_STREAMING; > - > if (session->phy) > io = bt_io_connect(avdtp_connect_cb, session, > NULL, &err, > @@ -2610,7 +2603,14 @@ static int send_req(struct avdtp *session, gboolean priority, > int err, timeout; > > if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) { > - session->io = l2cap_connect(session); > + BtIOMode mode; > + > + if (btd_opts.mps == MPS_OFF) > + mode = BT_IO_MODE_BASIC; > + else > + mode = BT_IO_MODE_ERTM; > + > + session->io = l2cap_connect(session, mode); > if (!session->io) { > /* Report disconnection anyways, as the other layers > * are using this state for cleanup. > @@ -2807,8 +2807,14 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre > struct seid_rej *resp, int size) > { > struct avdtp_local_sep *sep = stream->lsep; > + BtIOMode mode; > + > + if (btd_opts.mps == MPS_OFF) > + mode = BT_IO_MODE_BASIC; > + else > + mode = BT_IO_MODE_STREAMING; > > - stream->io = l2cap_connect(session); > + stream->io = l2cap_connect(session, mode); > if (!stream->io) { > avdtp_sep_set_state(session, sep, AVDTP_STATE_IDLE); > return FALSE; > -- > 2.26.2 >
Hi Alain, On Tue, Nov 17, 2020 at 6:43 AM Alain Michaud <alainmichaud@google.com> wrote: > > On Mon, Nov 16, 2020 at 7:26 PM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > This adds a new group AVDTP where platform can confure the prefered > > modes L2CAP channel for both session (signalling) and stream > > (transport). For better backward compatibility the default modes of > > boths > > Reviewed-by: Alain Michaud <alainm@chromium.org> > Tested-by: Alain Michaud <alainm@chromium.org> > > > --- > > profiles/audio/a2dp.c | 5 +---- > > profiles/audio/avdtp.c | 14 ++----------- > > src/btd.h | 7 +++++++ > > src/main.c | 45 ++++++++++++++++++++++++++++++++++++++++++ > > src/main.conf | 13 ++++++++++++ > > 5 files changed, 68 insertions(+), 16 deletions(-) > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > > index 626f61d34..59d11a0aa 100644 > > --- a/profiles/audio/a2dp.c > > +++ b/profiles/audio/a2dp.c > > @@ -2324,10 +2324,7 @@ static bool a2dp_server_listen(struct a2dp_server *server) > > if (server->io) > > return true; > > > > - if (btd_opts.mps == MPS_OFF) > > - mode = BT_IO_MODE_BASIC; > > - else > > - mode = BT_IO_MODE_STREAMING; > > + mode = btd_opts.avdtp.session_mode; > > > > server->io = bt_io_listen(NULL, confirm_cb, server, NULL, &err, > > BT_IO_OPT_SOURCE_BDADDR, > > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c > > index 619b94e29..ff0a124c3 100644 > > --- a/profiles/audio/avdtp.c > > +++ b/profiles/audio/avdtp.c > > @@ -2603,12 +2603,7 @@ static int send_req(struct avdtp *session, gboolean priority, > > int err, timeout; > > > > if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) { > > - BtIOMode mode; > > - > > - if (btd_opts.mps == MPS_OFF) > > - mode = BT_IO_MODE_BASIC; > > - else > > - mode = BT_IO_MODE_ERTM; > > + BtIOMode mode = btd_opts.avdtp.session_mode; > > > > session->io = l2cap_connect(session, mode); > > if (!session->io) { > > @@ -2807,12 +2802,7 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre > > struct seid_rej *resp, int size) > > { > > struct avdtp_local_sep *sep = stream->lsep; > > - BtIOMode mode; > > - > > - if (btd_opts.mps == MPS_OFF) > > - mode = BT_IO_MODE_BASIC; > > - else > > - mode = BT_IO_MODE_STREAMING; > > + BtIOMode mode = btd_opts.avdtp.stream_mode; > > > > stream->io = l2cap_connect(session, mode); > > if (!stream->io) { > > diff --git a/src/btd.h b/src/btd.h > > index c98414e35..a3247e4fd 100644 > > --- a/src/btd.h > > +++ b/src/btd.h > > @@ -84,6 +84,11 @@ struct btd_defaults { > > struct btd_le_defaults le; > > }; > > > > +struct btd_avdtp_opts { > > + uint8_t session_mode; > > + uint8_t stream_mode; > > +}; > > + > > struct btd_opts { > > char *name; > > uint32_t class; > > @@ -112,6 +117,8 @@ struct btd_opts { > > uint8_t gatt_channels; > > enum mps_mode_t mps; > > > > + struct btd_avdtp_opts avdtp; > > + > > uint8_t key_size; > > > > enum jw_repairing_t jw_repairing; > > diff --git a/src/main.c b/src/main.c > > index e6c4d861e..33c0f0d15 100644 > > --- a/src/main.c > > +++ b/src/main.c > > @@ -34,6 +34,7 @@ > > #include "lib/sdp.h" > > > > #include "gdbus/gdbus.h" > > +#include "btio/btio.h" > > > > #include "log.h" > > #include "backtrace.h" > > @@ -137,6 +138,12 @@ static const char *gatt_options[] = { > > NULL > > }; > > > > +static const char *avdtp_options[] = { > > + "SessionMode", > > + "StreamMode", > > + NULL > > +}; > > + > > static const struct group_table { > > const char *name; > > const char **options; > > @@ -146,6 +153,7 @@ static const struct group_table { > > { "LE", le_options }, > > { "Policy", policy_options }, > > { "GATT", gatt_options }, > > + { "AVDTP", avdtp_options }, > > { } > > }; > > > > @@ -744,6 +752,40 @@ static void parse_config(GKeyFile *config) > > btd_opts.gatt_channels = val; > > } > > > > + str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err); > > + if (err) { > > + DBG("%s", err->message); > > + g_clear_error(&err); > > + } else { > > + DBG("SessionMode=%s", str); > > + > > + if (!strcmp(str, "basic")) > > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC; > > + else if (!strcmp(str, "ertm")) > > + btd_opts.avdtp.session_mode = BT_IO_MODE_ERTM; > > + else { > > + DBG("Invalid mode option: %s", str); > > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC; > > + } > > + } > > + > > + val = g_key_file_get_integer(config, "AVDTP", "StreamMode", &err); > > + if (err) { > > + DBG("%s", err->message); > > + g_clear_error(&err); > > + } else { > > + DBG("StreamMode=%s", str); > > + > > + if (!strcmp(str, "basic")) > > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC; > > + else if (!strcmp(str, "streaming")) > > + btd_opts.avdtp.stream_mode = BT_IO_MODE_STREAMING; > > + else { > > + DBG("Invalid mode option: %s", str); > > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC; > > + } > > + } > > + > > parse_br_config(config); > > parse_le_config(config); > > } > > @@ -780,6 +822,9 @@ static void init_defaults(void) > > btd_opts.gatt_cache = BT_GATT_CACHE_ALWAYS; > > btd_opts.gatt_mtu = BT_ATT_MAX_LE_MTU; > > btd_opts.gatt_channels = 3; > > + > > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC; > > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC; > > } > > > > static void log_handler(const gchar *log_domain, GLogLevelFlags log_level, > > diff --git a/src/main.conf b/src/main.conf > > index 54f6a36bd..d3bc61441 100644 > > --- a/src/main.conf > > +++ b/src/main.conf > > @@ -200,6 +200,19 @@ > > # Default to 3 > > #Channels = 3 > > > > +[AVDTP] > > +# AVDTP L2CAP Signalling Channel Mode. > > +# Possible values: > > +# basic: Use L2CAP Basic Mode > > +# ertm: Use L2CAP Enhanced Retransmission Mode > > +#SessionMode = basic > > + > > +# AVDTP L2CAP Transport Channel Mode. > > +# Possible values: > > +# basic: Use L2CAP Basic Mode > > +# streaming: Use L2CAP Streaming Mode > > +#StreamMode = basic > > + > > [Policy] > > # > > # The ReconnectUUIDs defines the set of remote services that should try > > -- > > 2.26.2 > > Pushed. -- Luiz Augusto von Dentz
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c index 16fa20bba..619b94e29 100644 --- a/profiles/audio/avdtp.c +++ b/profiles/audio/avdtp.c @@ -2429,20 +2429,13 @@ uint16_t avdtp_get_version(struct avdtp *session) return session->version; } -static GIOChannel *l2cap_connect(struct avdtp *session) +static GIOChannel *l2cap_connect(struct avdtp *session, BtIOMode mode) { GError *err = NULL; GIOChannel *io; const bdaddr_t *src; - BtIOMode mode; - src = btd_adapter_get_address(device_get_adapter(session->device)); - if (btd_opts.mps == MPS_OFF) - mode = BT_IO_MODE_BASIC; - else - mode = BT_IO_MODE_STREAMING; - if (session->phy) io = bt_io_connect(avdtp_connect_cb, session, NULL, &err, @@ -2610,7 +2603,14 @@ static int send_req(struct avdtp *session, gboolean priority, int err, timeout; if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) { - session->io = l2cap_connect(session); + BtIOMode mode; + + if (btd_opts.mps == MPS_OFF) + mode = BT_IO_MODE_BASIC; + else + mode = BT_IO_MODE_ERTM; + + session->io = l2cap_connect(session, mode); if (!session->io) { /* Report disconnection anyways, as the other layers * are using this state for cleanup. @@ -2807,8 +2807,14 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre struct seid_rej *resp, int size) { struct avdtp_local_sep *sep = stream->lsep; + BtIOMode mode; + + if (btd_opts.mps == MPS_OFF) + mode = BT_IO_MODE_BASIC; + else + mode = BT_IO_MODE_STREAMING; - stream->io = l2cap_connect(session); + stream->io = l2cap_connect(session, mode); if (!stream->io) { avdtp_sep_set_state(session, sep, AVDTP_STATE_IDLE); return FALSE;
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Signalling channel shall only use Basic or ERTM modes. --- profiles/audio/avdtp.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)