Bluetooth: Classic: AVRCP: Support vendor dependent cmd #96273

makeshi · 2025-09-19T09:39:19Z

This patch introduces support for AVRCP vendor-dependent commands and responses, including full handling of fragmented messages.

MarkWangChinese

Only reviewed part of the codes.

subsys/bluetooth/host/classic/avrcp_internal.h

MarkWangChinese · 2025-09-23T09:50:58Z

subsys/bluetooth/host/classic/avrcp.c

+				  sizeof(struct bt_l2cap_hdr) +
+				  sizeof(struct bt_avctp_header_start) +
+				  sizeof(struct bt_avrcp_header) +
+				  sizeof(struct bt_avrcp_avc_pdu));


From I see, the bt_avrcp_avc_pdu is for vendor dependent msg. Is bt_avrcp_create_pdu only for vendor dependent msg buf creation?

Currently all bt_avrcp_avc_pdu for all AV command buffer alloc included browing buffer, the API's message is the AVRCP payload, so I need to reserve space for the maximum possible AVRCP header inside it, the sizeof(struct bt_avrcp_header) + sizeof(struct bt_avrcp_avc_pdu) is maximum possible AVRCP header.

Do you consider to divide bt_avrcp_avc_pdu() into two part. On part for pass through. Another part for vendor independent?

Have split two api for net_buf headrom saving

MarkWangChinese · 2025-09-23T12:40:21Z

include/zephyr/bluetooth/classic/avrcp.h

 	 *             @ref bt_avrcp_set_browsed_player_rsp. Note that the data is encoded in
 	 *             big-endian format.
 	 */
+


meaningless blank line.

MarkWangChinese · 2025-09-23T12:46:49Z

include/zephyr/bluetooth/classic/avrcp.h

+	 *  @param tid The transaction label of the request.
+	 *  @param cap_id The capability ID requested.
+	 */
+	void (*get_cap_req)(struct bt_avrcp_tg *tg, uint8_t tid, uint8_t cap_id);


why the get_caps_rsp uses caps?

I want to standardize the naming of this API so that it's easy to trace from the spec document → PDU ID → command/response buffer name → API and callback.
The reason I renamed it to caps is because of the BT_AVRCP_PDU_ID_GET_CAPS.

MarkWangChinese · 2025-09-23T13:13:00Z

subsys/bluetooth/host/classic/avrcp_internal.h

+	uint16_t received_len;			/**< Length already received */
+};
+
+struct bt_avrcp_tg_tx {


From my see, the struct is only used for sending the vendor dependent response, suggest to change as bt_avrcp_tg_vd_rsp_tx

MarkWangChinese · 2025-09-23T13:14:30Z

subsys/bluetooth/host/classic/avrcp.c

+	struct k_sem lock;
+
+	/* TX work */
+	struct k_work_delayable tx_work;


From my see, it is only used for the vendor dependent response. Suggest to change the name to indicate the usage.

gzh-terry · 2025-09-24T04:15:26Z

include/zephyr/bluetooth/classic/avrcp.h

+	 *
+	 * @note This is an AV/C Response semantics (not an AVRCP Error Status).
+	 *       When the CT-side callback reports REJECTED, the application MUST parse
+	 *       the net buffer to extract the 1-octet Error Status from the PDU payload.


Can we follow the spec definitions?
BTW, the range of these responses should be in 0x8-0xf

I'm trying to extend the status field. I want to append the response codes (ranging from 0x8 to 0xF) to the end of the status, starting from 0xFF bt_avrcp_rsp_t.
The response definitions will still remain as they are. pls see bt_avrcp_rsp_t
Now I'm facing a situation where a vendor-dependent Transaction Group (TG) reply requires both a response code and a status. For example, when the TG replies with an AV/C REJECTED response, it also needs to include a reason in the status.
At the same time, I feel that keeping both the response code and the status for the application might introduce redundant information, making the function somewhat awkward.
take example, if not extend the status field, we need
int bt_avrcp_tg_send_add_to_now_playing_rsp(struct bt_avrcp_tg *tg, uint8_t tid, uint8_t rsp_code, uint8_t status);
If we the status field, application only need take care the status, then stack will parse it to rsp_code and status.

detail see spec:
6.15.3 Status and error codes
The responses of AVRCP Specific Browsing Commands contain a status field to indicate success/failure
and an error status code is added to the AV/C REJECTED response if the TG rejected an AVRCP Specific AV/C command.
It is useful for CT to know why the command was rejected by the TG. Table 6.49 shows the status and error codes to
be used with both AVRCP Specific Browsing Commands and AVRCP Specific AV/C Commands.

lylezhu2012 · 2025-09-29T04:05:29Z

include/zephyr/bluetooth/classic/avrcp.h

+/** @brief AVRCP Media Attribute structure */
+struct bt_avrcp_media_attr {
+	uint32_t attr_id;    /**< Media attribute ID, see @ref bt_avrcp_media_attr_t */
+	uint16_t charset_id; /**< Character set ID, see @ref bt_avrcp_charset_t */
+	uint16_t attr_len;   /**< Length of attribute value */
+	uint8_t attr_val[];  /**< Attribute value data */
+} __packed;
+
+/** @brief GetElementAttributes command request structure */
+struct bt_avrcp_get_element_attrs_cmd {
+	uint8_t identifier[8]; /**< Element identifier (0x0 for currently playing) */
+	uint8_t num_attrs;     /**< Number of attributes requested (0 = all) */
+	uint32_t attr_ids[];   /**< Array of requested attribute IDs */
+} __packed;


Suggested change

/** @brief AVRCP Media Attribute structure */

struct bt_avrcp_media_attr {

uint32_t attr_id; /**< Media attribute ID, see @ref bt_avrcp_media_attr_t */

uint16_t charset_id; /**< Character set ID, see @ref bt_avrcp_charset_t */

uint16_t attr_len; /**< Length of attribute value */

uint8_t attr_val[]; /**< Attribute value data */

} __packed;

/** @brief GetElementAttributes command request structure */

struct bt_avrcp_get_element_attrs_cmd {

uint8_t identifier[8]; /**< Element identifier (0x0 for currently playing) */

uint8_t num_attrs; /**< Number of attributes requested (0 = all) */

uint32_t attr_ids[]; /**< Array of requested attribute IDs */

} __packed;

/** @brief GetElementAttributes command request structure */

struct bt_avrcp_get_element_attrs_cmd {

uint8_t identifier[8]; /**< Element identifier (0x0 for currently playing) */

uint8_t num_attrs; /**< Number of attributes requested (0 = all) */

uint32_t attr_ids[]; /**< Array of requested attribute IDs */

} __packed

/** @brief AVRCP Media Attribute structure */

struct bt_avrcp_media_attr {

uint32_t attr_id; /**< Media attribute ID, see @ref bt_avrcp_media_attr_t */

uint16_t charset_id; /**< Character set ID, see @ref bt_avrcp_charset_t */

uint16_t attr_len; /**< Length of attribute value */

uint8_t attr_val[]; /**< Attribute value data */

} __packed;

lylezhu2012 · 2025-09-29T04:06:00Z

include/zephyr/bluetooth/classic/avrcp.h

+struct bt_avrcp_get_play_status_rsp {
+	uint32_t song_length;    /**< Total length of the song in milliseconds */
+	uint32_t song_position;  /**< Current position in the song in milliseconds */
+	uint8_t play_status;     /**< Play status: @ref bt_avrcp_playback_status_t*/


Suggested change

uint8_t play_status; /**< Play status: @ref bt_avrcp_playback_status_t*/

uint8_t play_status; /**< Play status: @ref bt_avrcp_playback_status_t */

lylezhu2012 · 2025-09-29T04:07:14Z

include/zephyr/bluetooth/classic/avrcp.h

+
+/** @brief RegisterNotification command request */
+struct bt_avrcp_register_notification_cmd {
+    uint8_t event_id;        /**< Event ID to register for */


Where is the event ID defined?

the event ID is bt_avrcp_evt_t, but struct bt_avrcp_register_notification_cmd have been removed because it is not needed anymore

lylezhu2012 · 2025-09-29T05:38:51Z

subsys/bluetooth/host/classic/avrcp.c

+	{  BT_AVRCP_PDU_ID_GET_CAPS                          , BT_AVRCP_CTYPE_STATUS },
+	{  BT_AVRCP_PDU_ID_LIST_PLAYER_APP_SETTING_ATTRS     , BT_AVRCP_CTYPE_STATUS },
+	{  BT_AVRCP_PDU_ID_LIST_PLAYER_APP_SETTING_VALS      , BT_AVRCP_CTYPE_STATUS},
+	{  BT_AVRCP_PDU_ID_GET_CURR_PLAYER_APP_SETTING_VAL   , BT_AVRCP_CTYPE_STATUS},
+	{  BT_AVRCP_PDU_ID_SET_PLAYER_APP_SETTING_VAL        , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_GET_PLAYER_APP_SETTING_ATTR_TEXT  , BT_AVRCP_CTYPE_STATUS},
+	{  BT_AVRCP_PDU_ID_GET_PLAYER_APP_SETTING_VAL_TEXT   , BT_AVRCP_CTYPE_STATUS},
+	{  BT_AVRCP_PDU_ID_INFORM_DISPLAYABLE_CHAR_SET	     , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_INFORM_BATT_STATUS_OF_CT	     , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_GET_ELEMENT_ATTRS                 , BT_AVRCP_CTYPE_STATUS},
+	{  BT_AVRCP_PDU_ID_GET_PLAY_STATUS                   , BT_AVRCP_CTYPE_STATUS},
+	{  BT_AVRCP_PDU_ID_REGISTER_NOTIFICATION	     , BT_AVRCP_CTYPE_NOTIFY},
+	{  BT_AVRCP_PDU_ID_SET_ABSOLUTE_VOLUME               , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_SET_ADDRESSED_PLAYER              , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_PLAY_ITEM                         , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_ADD_TO_NOW_PLAYING                , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_REQ_CONTINUING_RSP                , BT_AVRCP_CTYPE_CONTROL},
+	{  BT_AVRCP_PDU_ID_ABORT_CONTINUING_RSP              , BT_AVRCP_CTYPE_CONTROL},


Some lines have space before the brace, and others do not.

fixed, and table is removed

lylezhu2012 · 2025-09-29T05:48:03Z

subsys/bluetooth/host/classic/avrcp.c

+struct avrcp_pdu_vendor_handler  {
+	bt_avrcp_pdu_id_t pdu_id;
+	uint8_t min_len;
+	void (*func)(struct bt_avrcp *avrcp, uint8_t tid, uint8_t result, struct net_buf *buf);
+};
+
+typedef struct {
+	uint8_t pdu_id;
+	bt_avrcp_ctype_t cmd_type;
+} bt_avrcp_pdu_cmd_map_t;


Do you consider that these two structures can be merged into one?

Good suggestion, updated

lylezhu2012 · 2025-09-29T10:51:12Z

include/zephyr/bluetooth/classic/avrcp.h

+	BT_AVRCP_STATUS_REJECTED = 0xf8,
+
+	/** Interim response.
+	 * Indicates a temporary state; typically used for @ref BT_AVRCP_CTYPE_NOTIFY
+	 * (e.g. RegisterNotification) to signal that a future CHANGED/STABLE
+	 * response will follow.
+	 */
+	BT_AVRCP_STATUS_INTERIM = 0xf9,
+
+	/** Changed response.
+	 * Indicates that the notified value has changed and a final result is provided.
+	 * @note AV/C Response code (not an Error Status).
+	 */
+	BT_AVRCP_STATUS_CHANGED = 0xfa,
+
+	/** Implemented response
+	 * - As IMPLEMENTED: used for SPECIFIC_INQUIRY and GENERAL_INQUIRY commands.
+	 * - As STABLE: used for STATUS commands to indicate a stable current state.
+	 */
+	BT_AVRCP_STATUS_IMPLEMENTED = 0xfb,
+
+	/** Stable response.
+	 * Alias of @ref BT_AVRCP_RSP_IMPLEMENTED for STATUS commands.
+	 */
+	BT_AVRCP_STATUS_STABLE = 0xfc,
+
+	/** In transition response.
+	 * The target is currently changing state (e.g., between play/pause).
+	 */
+	BT_AVRCP_STATUS_IN_TRANSITION = 0xfd,
+
+	/** Accepted response.
+	 * The command has been accepted and will be processed; a later response
+	 * (e.g., CHANGED/STABLE) may follow depending on the PDU.
+	 */
+	BT_AVRCP_STATUS_ACCEPTED = 0xfe,
+
+	/** Not implemented response.
+	 * The command/PDU is not supported by the target device.
+	 */
+	BT_AVRCP_STATUS_NOT_IMPLEMENTED = 0xff,


I think there are four types of status,

success -> For control, it means accepted. For status, it means stable.

in transition -> only for status, it means TG is busy.

not implemented -> It means the function is not implemented, such as the callback is not implemented, or the feature is not enabled.

Any other value of bt_avrcp_status_t. It means it is a rejected response. The value of bt_avrcp_status_t is the reason.

So, I think it can be optimised with these four types of response.

lylezhu2012 · 2025-09-29T10:57:32Z

subsys/bluetooth/host/classic/avrcp.c

+	uint8_t status;

-	switch (rsp->cap_id) {
-	case BT_AVRCP_CAP_COMPANY_ID:
-		expected_len = rsp->cap_cnt * BT_AVRCP_COMPANY_ID_SIZE;
-		break;
-	case BT_AVRCP_CAP_EVENTS_SUPPORTED:
-		expected_len = rsp->cap_cnt;
-		break;
-	default:
-		LOG_ERR("Unrecognized capability = 0x%x", rsp->cap_id);
-		return;
-	}
+	status = bt_avrcp_rsp_to_status(rsp_code, BT_AVRCP_STATUS_REJECTED);

-	if (buf->len < sizeof(*rsp) + expected_len) {
-		LOG_ERR("Invalid capability payload length: %d", buf->len);
+	if ((avrcp_ct_cb == NULL) || (avrcp_ct_cb->get_caps_rsp == NULL)) {
 		return;
 	}

-	avrcp_ct_cb->get_caps_rsp(get_avrcp_ct(avrcp), tid, rsp);
+	avrcp_ct_cb->get_caps_rsp(get_avrcp_ct(avrcp), tid, status, buf);


Why the checking of received data length is removed?

The previous code response parsing and net_buf length checks were handled in the stack. the patch changed moves to using net_buf the length checking and net buffer parsing logic to the application layer.

If it is a malformed packet, I think it should not be notified.

because the common min size checking can not cover the buffer length checking here. I can move moves to using net_buf the length back to stack before notify.

lylezhu2012 · 2025-09-29T11:09:04Z

subsys/bluetooth/host/classic/avrcp.c

+	if ((avrcp_tg_cb == NULL) || (avrcp_tg_cb->set_absolute_volume_req == NULL)) {
+		error_code = BT_AVRCP_STATUS_INTERNAL_ERROR;


If the error code should be not implemented?

Same comments for other commands.

"not implemented" more reasonable, updated.

lylezhu2012 · 2025-09-29T11:13:49Z

subsys/bluetooth/host/classic/avrcp.c

+	return err;
+}
+
+int bt_avrcp_ct_get_cap(struct bt_avrcp_ct *ct, uint8_t tid, uint8_t cap_id)


If the rsp callback should be checked here? Since the request is not always successful, if the get cap rsp callback is NULL, what is the purpose of the request?

Same comments for other commands.

lylezhu2012 · 2025-09-29T11:16:42Z

subsys/bluetooth/host/classic/avrcp.c

+	buf = bt_avrcp_create_pdu(&avrcp_pool);
+	if (buf == NULL) {
+		LOG_ERR("Failed to allocate buffer");
+		return -ENOMEM;


Suggested change

return -ENOMEM;

return -ENOBUFS;

Please check the same case in other functions.

Updated the check to use buf->len to ensure the buffer contains enough data for the requested chunk size. Signed-off-by: Make Shi <[email protected]>

This patch introduces support for AVRCP vendor-dependent commands and responses, including full handling of fragmented messages. - Adds fragmentation and reassembly logic for AVRCP vendor-dependent - Introduces TX queue management using delayed work for TG - Adds support for GetCapabilities PDUs - Add new Kconfig for vendor-dependent with fragmentation support Signed-off-by: Make Shi <[email protected]>

This patch adds AVRCP notification event handling for both CT and TG roles. Also add Shell command support for testing notification registration and responses. Signed-off-by: Make Shi <[email protected]>

- Added new callback APIs in bt_avrcp_ct_cb and bt_avrcp_tg_cb - Implemented command/response handlers for CT and TG roles - Extended shell commands for testing vendor dependent commands Signed-off-by: Make Shi <[email protected]>

sonarqubecloud · 2025-10-13T10:35:16Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
4.1% Duplication on New Code

See analysis details on SonarQube Cloud

MarkWangChinese · 2025-10-16T08:09:40Z

subsys/bluetooth/host/classic/avctp.c

-	if (net_buf_tailroom(buf) < chunk_size) {
-		LOG_WRN("Not enough tailroom for AVCTP payload (len: %d)", chunk_size);
+	if (buf->len < chunk_size) {
+		LOG_WRN("AVCTP payload too short: have %d, need %d", buf->len, chunk_size);


IMO, this part should be before the send_fragmented_avctp.

MarkWangChinese · 2025-10-16T08:30:33Z

subsys/bluetooth/host/classic/avrcp.c

+			return -ENOBUFS;
+		}
+
+		if (AVRCP_STATUS_IS_REJECTED(status) == true) {


This if is not needed from my viewpoint, otherwise the status_buf may not be used and released.

MarkWangChinese · 2025-10-16T09:08:28Z

subsys/bluetooth/host/classic/Kconfig

+config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE
+	int "AVRCP Vendor Dependent RX data buffer Size"
+	default 1024
+	range 1024 $(INT16_MAX)


Suggest to change as 512 $(INT16_MAX) because the requestcontinue is used when data length is bigger than 512.

MarkWangChinese · 2025-10-16T09:14:50Z

subsys/bluetooth/host/classic/avrcp.c

+	hdr->opcode = BT_AVRCP_OPC_VENDOR_DEPENDENT;
+
+	err = avrcp_send(ct->avrcp, buf, BT_AVCTP_CMD, tid);
+	if (err < 0) {


Is net_buf_unref(buf); needed here?

MarkWangChinese · 2025-10-17T03:18:56Z

subsys/bluetooth/host/classic/avrcp.c

+		goto err_rsp;
+	}
+
+	return avrcp_tg_cb->get_caps_req(get_avrcp_tg(avrcp), tid, cap_id);


Just curious: The avrcp_tg_cb->get_caps_req doesn't have return value and process_get_caps_cmd doesn't have return value too. Is there any building warning?

MarkWangChinese · 2025-10-17T06:19:51Z

subsys/bluetooth/host/classic/avrcp.c

+	}
+
+	memset(&ct->ct_notify[event_id], 0, sizeof(ct->ct_notify[0]));
+	ct->ct_notify[event_id].cb = cb;


Is it needed: clear the .cb when the event changed response is received. When registering one event that is already registered return error here.

MarkWangChinese · 2025-10-17T06:23:22Z

subsys/bluetooth/host/classic/avrcp.c

+		return -ENOTSUP;
+	}
+
+	switch (status) {


Is it needed: save the flag whether the event is registered, then return error if not registered by CT?

The registered event_id should be flagged. Only registered event can be responded.

I think it is not a dedicated issue for bt_avrcp_tg_send_notification_rsp(). For AVRCP, all responses should have the similar issue.

MarkWangChinese · 2025-10-17T06:32:34Z

include/zephyr/bluetooth/classic/avrcp.h

+	 *       first be registered using @ref bt_avrcp_notification_cb_t.
+	 */
+	void (*notification_rsp)(struct bt_avrcp_ct *ct, uint8_t tid, uint8_t status,
+				 uint8_t event_id, struct bt_avrcp_event_data *data);


IMO, suggest to callback struct bt_avrcp_event_data *data only through bt_avrcp_notification_cb_t, then this callback (changed as register_notification_rsp) only indicates the result of the registering.

MarkWangChinese · 2025-10-17T06:33:27Z

include/zephyr/bluetooth/classic/avrcp.h

+ *  @note The callback implementation should not block or perform heavy operations.
+ *       If needed, defer processing to another thread or task.
+ */
+typedef void(*bt_avrcp_notification_cb_t)(uint8_t event_id, struct bt_avrcp_event_data *data);


IMO, suggest: typedef void(*bt_avrcp_notification_cb_t)(uint8_t event_id, bool changed, struct bt_avrcp_event_data *data); then all the event data related callback is here.

lylezhu2012

The comments only for first 3 commits.

lylezhu2012 · 2025-10-17T03:41:49Z

subsys/bluetooth/host/classic/avctp.c

+	if (buf->len < chunk_size) {
+		LOG_WRN("AVCTP payload too short: have %d, need %d", buf->len, chunk_size);
 		goto failed;
 	}


Whether is the condition always false?
If user_data->sent_len is 0 and buf->len < chunk_size, it should be single packet. This condition is not be checked.

Or, if user_data->sent_len != 0U, buf->len < chunk_size should be always false since chunk_size = MIN(buf->len, max_payload_size - BT_AVCTP_HDR_SIZE_CONTINUE_END);.

lylezhu2012 · 2025-10-17T04:01:44Z

subsys/bluetooth/host/classic/Kconfig

 	help
 	  This option enables the AVRCP profile controller function

+config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE


Suggested change

config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE

config BT_AVRCP_VD_RX_SIZE

or

Suggested change

config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE

config BT_AVRCP_VENDOR_DEPENDENT_RX_SIZE

lylezhu2012 · 2025-10-17T04:06:26Z

subsys/bluetooth/host/classic/Kconfig

+config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE
+	int "AVRCP Vendor Dependent RX data buffer Size"
+	default 1024
+	range 1024 $(INT16_MAX)


Why is the minimum value 1024? I think it could be MTU of L2CAP.

lylezhu2012 · 2025-10-17T05:59:28Z

subsys/bluetooth/host/classic/avrcp.c

+	struct bt_avrcp_tg_vd_rsp_tx *tx = net_buf_user_data(buf);
+
+	__ASSERT(tx != NULL, "user_data is NULL in destroy callback");


If the buf allocated from avrcp_vd_tx_pool has been used to send data (the owner of the buffer is changed from AVRCP to L2CAP), the user data should not be used by AVRCP. Because the user data is dirty.

lylezhu2012 · 2025-10-17T06:05:23Z

subsys/bluetooth/host/classic/avrcp.c

+		break;
+	}
+
+	*out_status = status;


I think the status can be removed and pass value to *out_status directly in switch...case

lylezhu2012 · 2025-10-17T10:12:03Z

subsys/bluetooth/host/classic/avrcp.c

+		return -ENOTSUP;
+	}
+
+	switch (status) {


The registered event_id should be flagged. Only registered event can be responded.

I think it is not a dedicated issue for bt_avrcp_tg_send_notification_rsp(). For AVRCP, all responses should have the similar issue.

lylezhu2012 · 2025-10-17T10:16:10Z

subsys/bluetooth/host/classic/avrcp.c

+	case BT_AVRCP_STATUS_NOT_IMPLEMENTED:
+		rsp_code = BT_AVRCP_RSP_NOT_IMPLEMENTED;
+		break;


I think if the INTERIM has been responded, the NOT_IMPLEMENTED should not be replied.

lylezhu2012 · 2025-10-17T10:16:59Z

subsys/bluetooth/host/classic/avrcp.c

+
+	default:
+		rsp_code = BT_AVRCP_RSP_REJECTED;
+		break;


The tg->interim_sent[event_id] should also be cleared if the rsp_code is rejected.

lylezhu2012 · 2025-10-17T10:20:53Z

subsys/bluetooth/host/classic/avrcp.c

+	buf = bt_avrcp_create_vendor_pdu(NULL);
+	if (buf == NULL) {
+		LOG_ERR("Failed to allocate buffer");
+		return -ENOBUFS;
+	}
+
+	if (rsp_code == BT_AVRCP_RSP_REJECTED) {
+		if (net_buf_tailroom(buf) < sizeof(status)) {
+			LOG_ERR("Not enough space in net_buf");
+			net_buf_unref(buf);
+			return -ENOMEM;
+		}
+		net_buf_add_u8(buf, status);
+	} else {
+		err = build_notification_rsp_data(event_id, data, buf);
+		if (err < 0) {
+			net_buf_unref(buf);
+			return err;
+		}
+	}
+
+	err =  bt_avrcp_tg_send_vendor_rsp(tg, tid, BT_AVRCP_PDU_ID_REGISTER_NOTIFICATION,
+					   rsp_code, buf);
+	if (err < 0) {
+		LOG_ERR("Failed to send notification response (err: %d)", err);
+		net_buf_unref(buf);
+	}
+	return err;


If the INTERIM has been sent and if there is any error when sending the 2th response, how to avoid the consistency state between CT and TG? Recover tg->interim_sent[event_id]?

lylezhu2012 · 2025-10-17T10:23:04Z

subsys/bluetooth/host/classic/avrcp.c

+		err = build_notification_rsp_data(event_id, data, buf);
+		if (err < 0) {
+			net_buf_unref(buf);
+			return err;
+		}


Is it possible the buf is provided by the application? And AVRCP provides a series of auxiliary functions for packing the event data.

zephyrbot added area: Bluetooth Classic Bluetooth Classic (BR/EDR) area: Bluetooth labels Sep 19, 2025

zephyrbot requested review from MarkWangChinese, gzh-terry, jhedberg and lylezhu2012 September 19, 2025 09:40

zephyrbot assigned lylezhu2012 Sep 19, 2025

makeshi force-pushed the br_avrcp_vendor_dependent_cmd branch 10 times, most recently from 7251c67 to 54ff174 Compare September 23, 2025 12:34

MarkWangChinese reviewed Sep 23, 2025

View reviewed changes

gzh-terry reviewed Sep 24, 2025

View reviewed changes

makeshi force-pushed the br_avrcp_vendor_dependent_cmd branch 3 times, most recently from a665fac to dcdbd31 Compare September 26, 2025 05:40

makeshi requested review from MarkWangChinese and gzh-terry September 26, 2025 07:35

lylezhu2012 changed the title ~~Br avrcp vendor dependent cmd~~ Bluetooth: Classic: AVRCP: Br avrcp vendor dependent cmd Sep 29, 2025

lylezhu2012 changed the title ~~Bluetooth: Classic: AVRCP: Br avrcp vendor dependent cmd~~ Bluetooth: Classic: AVRCP: Support vendor dependent cmd Sep 29, 2025

makeshi force-pushed the br_avrcp_vendor_dependent_cmd branch 3 times, most recently from a54dac2 to 08e5c63 Compare September 29, 2025 06:01

lylezhu2012 reviewed Sep 29, 2025

View reviewed changes

lylezhu2012 mentioned this pull request Oct 9, 2025

Planned Bluetooth Classic improvement #76622

Open

26 tasks

Bluetooth: AVCTP: Fix payload length check in avctp_tx_processor

d50ad68

Updated the check to use buf->len to ensure the buffer contains enough data for the requested chunk size. Signed-off-by: Make Shi <[email protected]>

makeshi force-pushed the br_avrcp_vendor_dependent_cmd branch from 08e5c63 to 7f30b30 Compare October 13, 2025 03:50

zephyrbot requested a review from chengkai15 October 13, 2025 03:51

makeshi force-pushed the br_avrcp_vendor_dependent_cmd branch 3 times, most recently from 6f5302a to 3a07ada Compare October 13, 2025 06:59

makeshi added 3 commits October 13, 2025 17:56

Bluetooth: AVRCP: Add support for notification events

52d91a8

This patch adds AVRCP notification event handling for both CT and TG roles. Also add Shell command support for testing notification registration and responses. Signed-off-by: Make Shi <[email protected]>

makeshi force-pushed the br_avrcp_vendor_dependent_cmd branch from 3a07ada to 660cae6 Compare October 13, 2025 10:09

makeshi requested a review from lylezhu2012 October 13, 2025 12:07

MarkWangChinese reviewed Oct 17, 2025

View reviewed changes

lylezhu2012 reviewed Oct 17, 2025

View reviewed changes

	uint8_t play_status; /*< Play status: @ref bt_avrcp_playback_status_t/
	uint8_t play_status; /*< Play status: @ref bt_avrcp_playback_status_t /

		if ((avrcp_tg_cb == NULL) \|\| (avrcp_tg_cb->set_absolute_volume_req == NULL)) {
		error_code = BT_AVRCP_STATUS_INTERNAL_ERROR;

	config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE
	config BT_AVRCP_VD_RX_SIZE

	config BT_AVRCP_VENDOR_RX_DATA_BUF_SIZE
	config BT_AVRCP_VENDOR_DEPENDENT_RX_SIZE

		struct bt_avrcp_tg_vd_rsp_tx *tx = net_buf_user_data(buf);

		__ASSERT(tx != NULL, "user_data is NULL in destroy callback");

Bluetooth: Classic: AVRCP: Support vendor dependent cmd #96273

Are you sure you want to change the base?

Bluetooth: Classic: AVRCP: Support vendor dependent cmd #96273

Conversation

makeshi commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkWangChinese left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

makeshi commented Sep 19, 2025 •

edited

Loading