diff options
| author | 2024-10-16 10:13:19 -0700 | |
|---|---|---|
| committer | 2024-10-23 17:53:15 +0000 | |
| commit | 558a9cc698fb39016af944455ff3812af363d2cf (patch) | |
| tree | 218ee8b2b41c8d8ebc4206bbaec5e35d49884ce6 | |
| parent | c197313a287b5686af32bd86c2f56e0ac34f7749 (diff) | |
stack::avct: Various cleanup
1. Expand avct initiator/acceptor enum syntax
2. Address cpplint errors
3. Update conventional logging
a. Use 0x4 hex for l2cap cids
b. Use hex for bitmasks
4. Remove dead code and unused definitions
5. Enumified key data types and added string representations
6. Statically scoped l2cap callbacks to translation unit
7. Highlighted issue with data narrowing on RC features
Bug: 373471940
Test: m .
Flag: EXEMPT, Refactor
Change-Id: Ie3719d2b318fe068c8185fddd8d190ca1c5eeef9
26 files changed, 379 insertions, 381 deletions
diff --git a/system/bta/av/bta_av_act.cc b/system/bta/av/bta_av_act.cc index fdc2cfcb98..a01920504d 100644 --- a/system/bta/av/bta_av_act.cc +++ b/system/bta/av/bta_av_act.cc @@ -321,7 +321,7 @@ static void bta_av_rc_msg_cback(uint8_t handle, uint8_t label, uint8_t opcode, t * Returns the created rc handle * ******************************************************************************/ -uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, uint8_t role, uint8_t shdl, uint8_t lidx) { +uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, tAVCT_ROLE role, uint8_t shdl, uint8_t lidx) { if ((!btif_av_src_sink_coexist_enabled() || (btif_av_src_sink_coexist_enabled() && !btif_av_is_sink_enabled() && btif_av_is_source_enabled())) && @@ -330,14 +330,13 @@ uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, uint8_t role, uint8_t shdl, uint8_t l return BTA_AV_RC_HANDLE_NONE; } - tAVRC_CONN_CB ccb; RawAddress bda = RawAddress::kAny; uint8_t status = BTA_AV_RC_ROLE_ACP; int i; uint8_t rc_handle; - tBTA_AV_RCB* p_rcb; + tBTA_AV_RCB* p_rcb{nullptr}; - if (role == AVCT_INT) { + if (role == AVCT_ROLE_INITIATOR) { // Can't grab a stream control block that doesn't have a valid handle if (!shdl) { log::error("Can't grab stream control block for shdl = {} -> index = {}", shdl, shdl - 1); @@ -357,14 +356,16 @@ uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, uint8_t role, uint8_t shdl, uint8_t l } } - ccb.ctrl_cback = base::Bind(bta_av_rc_ctrl_cback); - ccb.msg_cback = base::Bind(bta_av_rc_msg_cback); - ccb.company_id = p_bta_av_cfg->company_id; - ccb.conn = role; - /* note: BTA_AV_FEAT_RCTG = AVRC_CT_TARGET, BTA_AV_FEAT_RCCT = AVRC_CT_CONTROL - */ - ccb.control = p_cb->features & - (BTA_AV_FEAT_RCTG | BTA_AV_FEAT_RCCT | BTA_AV_FEAT_METADATA | AVRC_CT_PASSIVE); + tAVRC_CONN_CB ccb = { + .ctrl_cback = base::Bind(bta_av_rc_ctrl_cback), + .msg_cback = base::Bind(bta_av_rc_msg_cback), + .company_id = p_bta_av_cfg->company_id, + .conn = role, + // note: BTA_AV_FEAT_RCTG = AVRC_CT_TARGET, BTA_AV_FEAT_RCCT = AVRC_CT_CONTROL + .control = + static_cast<uint8_t>(p_cb->features & (BTA_AV_FEAT_RCTG | BTA_AV_FEAT_RCCT | + BTA_AV_FEAT_METADATA | AVRC_CT_PASSIVE)), + }; if (AVRC_Open(&rc_handle, &ccb, bda) != AVRC_SUCCESS) { DEVICE_IOT_CONFIG_ADDR_INT_ADD_ONE(bda, IOT_CONF_KEY_AVRCP_CONN_FAIL_COUNT); @@ -392,8 +393,8 @@ uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, uint8_t role, uint8_t shdl, uint8_t l p_cb->rc_acp_idx = (i + 1); log::verbose("rc_acp_handle:{} idx:{}", p_cb->rc_acp_handle, p_cb->rc_acp_idx); } - log::verbose("create {}, role: {}, shdl:{}, rc_handle:{}, lidx:{}, status:0x{:x}", i, role, shdl, - p_rcb->handle, lidx, p_rcb->status); + log::verbose("create {}, role: {}, shdl:{}, rc_handle:{}, lidx:{}, status:0x{:x}", i, + avct_role_text(role), shdl, p_rcb->handle, lidx, p_rcb->status); return rc_handle; } @@ -539,7 +540,7 @@ void bta_av_rc_opened(tBTA_AV_CB* p_cb, tBTA_AV_DATA* p_data) { /* listen to browsing channel when the connection is open, * if peer initiated AVRCP connection and local device supports browsing * channel */ - AVRC_OpenBrowse(p_data->rc_conn_chg.handle, AVCT_ACP); + AVRC_OpenBrowse(p_data->rc_conn_chg.handle, AVCT_ROLE_ACCEPTOR); if (p_cb->rcb[i].lidx == (BTA_AV_NUM_LINKS + 1) && shdl != 0) { /* rc is opened on the RC only ACP channel, but is for a specific @@ -618,7 +619,7 @@ void bta_av_rc_opened(tBTA_AV_CB* p_cb, tBTA_AV_DATA* p_data) { (rc_open.peer_tg_features & BTA_AV_FEAT_BROWSE))) { if ((p_cb->rcb[i].status & BTA_AV_RC_ROLE_MASK) == BTA_AV_RC_ROLE_INT) { log::verbose("opening AVRC Browse channel"); - AVRC_OpenBrowse(p_data->rc_conn_chg.handle, AVCT_INT); + AVRC_OpenBrowse(p_data->rc_conn_chg.handle, AVCT_ROLE_INITIATOR); } } return; @@ -649,7 +650,7 @@ void bta_av_rc_opened(tBTA_AV_CB* p_cb, tBTA_AV_DATA* p_data) { if ((p_cb->features & BTA_AV_FEAT_BROWSE) && (rc_open.peer_features & BTA_AV_FEAT_BROWSE) && ((p_cb->rcb[i].status & BTA_AV_RC_ROLE_MASK) == BTA_AV_RC_ROLE_INT)) { log::verbose("opening AVRC Browse channel"); - AVRC_OpenBrowse(p_data->rc_conn_chg.handle, AVCT_INT); + AVRC_OpenBrowse(p_data->rc_conn_chg.handle, AVCT_ROLE_INITIATOR); } } @@ -1335,7 +1336,7 @@ void bta_av_conn_chg(tBTA_AV_DATA* p_data) { /* if the AVRCP is no longer listening, create the listening channel */ if (bta_av_cb.rc_acp_handle == BTA_AV_RC_HANDLE_NONE && bta_av_cb.features & BTA_AV_FEAT_RCTG) { - bta_av_rc_create(&bta_av_cb, AVCT_ACP, 0, BTA_AV_NUM_LINKS + 1); + bta_av_rc_create(&bta_av_cb, AVCT_ROLE_ACCEPTOR, 0, BTA_AV_NUM_LINKS + 1); } } @@ -1554,7 +1555,7 @@ void bta_av_sig_chg(tBTA_AV_DATA* p_data) { p_lcb->conn_msk = 0; /* clear the connect mask */ /* start listening when the signal channel is open */ if (p_cb->features & BTA_AV_FEAT_RCTG) { - bta_av_rc_create(p_cb, AVCT_ACP, 0, p_lcb->lidx); + bta_av_rc_create(p_cb, AVCT_ROLE_ACCEPTOR, 0, p_lcb->lidx); } /* this entry is not used yet. */ p_cb->conn_lcb |= mask; /* mark it as used */ @@ -2118,7 +2119,8 @@ static void bta_av_rc_disc_done_all(tBTA_AV_DATA* /* p_data */) { ((p_cb->features & BTA_AV_FEAT_RCTG) && (peer_ct_features & BTA_AV_FEAT_RCCT))) { p_lcb = bta_av_find_lcb(p_scb->PeerAddress(), BTA_AV_LCB_FIND); if (p_lcb) { - rc_handle = bta_av_rc_create(p_cb, AVCT_INT, (uint8_t)(p_scb->hdi + 1), p_lcb->lidx); + rc_handle = bta_av_rc_create(p_cb, AVCT_ROLE_INITIATOR, (uint8_t)(p_scb->hdi + 1), + p_lcb->lidx); if (rc_handle != BTA_AV_RC_HANDLE_NONE) { p_cb->rcb[rc_handle].peer_ct_features = peer_ct_features; p_cb->rcb[rc_handle].peer_tg_features = peer_tg_features; @@ -2302,7 +2304,8 @@ void bta_av_rc_disc_done(tBTA_AV_DATA* p_data) { ((p_cb->features & BTA_AV_FEAT_RCTG) && (peer_features & BTA_AV_FEAT_RCCT))) { p_lcb = bta_av_find_lcb(p_scb->PeerAddress(), BTA_AV_LCB_FIND); if (p_lcb) { - rc_handle = bta_av_rc_create(p_cb, AVCT_INT, (uint8_t)(p_scb->hdi + 1), p_lcb->lidx); + rc_handle = bta_av_rc_create(p_cb, AVCT_ROLE_INITIATOR, (uint8_t)(p_scb->hdi + 1), + p_lcb->lidx); if (rc_handle < BTA_AV_NUM_RCB) { p_cb->rcb[rc_handle].peer_features = peer_features; p_cb->rcb[rc_handle].cover_art_psm = cover_art_psm; @@ -2484,7 +2487,7 @@ void bta_av_rc_closed(tBTA_AV_DATA* p_data) { bta_av_data.rc_close = rc_close; (*p_cb->p_cback)(BTA_AV_RC_CLOSE_EVT, &bta_av_data); if (bta_av_cb.rc_acp_handle == BTA_AV_RC_HANDLE_NONE && bta_av_cb.features & BTA_AV_FEAT_RCTG) { - bta_av_rc_create(&bta_av_cb, AVCT_ACP, 0, BTA_AV_NUM_LINKS + 1); + bta_av_rc_create(&bta_av_cb, AVCT_ROLE_ACCEPTOR, 0, BTA_AV_NUM_LINKS + 1); } } diff --git a/system/bta/av/bta_av_int.h b/system/bta/av/bta_av_int.h index 94c63226e0..4f9773344e 100644 --- a/system/bta/av/bta_av_int.h +++ b/system/bta/av/bta_av_int.h @@ -37,6 +37,7 @@ #include "macros.h" #include "osi/include/list.h" #include "stack/include/a2dp_constants.h" +#include "stack/include/avct_api.h" #include "stack/include/avdt_api.h" #include "stack/include/bt_hdr.h" #include "stack/include/hci_error_code.h" @@ -526,7 +527,7 @@ public: bool use_rc; /* true if AVRCP is allowed */ bool started; /* true if stream started */ bool use_rtp_header_marker_bit; /* true if the encoded data packets have RTP - * headers, and the Marker bit in the header + * headers, with the Marker bit in the header * is set according to RFC 6416 */ uint8_t co_started; /* non-zero, if stream started from call-out perspective */ bool recfg_sup; /* true if the first attempt to reconfigure the stream was @@ -708,7 +709,7 @@ bool bta_av_chk_start(tBTA_AV_SCB* p_scb); void bta_av_restore_switch(void); void bta_av_conn_cback(uint8_t handle, const RawAddress& bd_addr, uint8_t event, tAVDT_CTRL* p_data, uint8_t scb_index); -uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, uint8_t role, uint8_t shdl, uint8_t lidx); +uint8_t bta_av_rc_create(tBTA_AV_CB* p_cb, tAVCT_ROLE role, uint8_t shdl, uint8_t lidx); void bta_av_stream_chg(tBTA_AV_SCB* p_scb, bool started); bool bta_av_is_scb_opening(tBTA_AV_SCB* p_scb); bool bta_av_is_scb_incoming(tBTA_AV_SCB* p_scb); diff --git a/system/bta/av/bta_av_main.cc b/system/bta/av/bta_av_main.cc index d8573d7f02..a992882a08 100644 --- a/system/bta/av/bta_av_main.cc +++ b/system/bta/av/bta_av_main.cc @@ -649,7 +649,7 @@ static void bta_av_api_register(tBTA_AV_DATA* p_data) { } /* start listening when A2DP is registered */ if (bta_av_cb.features & BTA_AV_FEAT_RCTG) { - bta_av_rc_create(&bta_av_cb, AVCT_ACP, 0, BTA_AV_NUM_LINKS + 1); + bta_av_rc_create(&bta_av_cb, AVCT_ROLE_ACCEPTOR, 0, BTA_AV_NUM_LINKS + 1); } /* if the AV and AVK are both supported, it cannot support the CT role @@ -658,7 +658,7 @@ static void bta_av_api_register(tBTA_AV_DATA* p_data) { /* if TG is not supported, we need to register to AVCT now */ if ((bta_av_cb.features & (BTA_AV_FEAT_RCTG)) == 0) { bta_ar_reg_avct(); - bta_av_rc_create(&bta_av_cb, AVCT_ACP, 0, BTA_AV_NUM_LINKS + 1); + bta_av_rc_create(&bta_av_cb, AVCT_ROLE_ACCEPTOR, 0, BTA_AV_NUM_LINKS + 1); } if (com::android::bluetooth::flags::avrcp_sdp_records()) { // Add control record for sink profile. diff --git a/system/bta/jv/bta_jv_act.cc b/system/bta/jv/bta_jv_act.cc index 852204ed61..cb00b7b6f8 100644 --- a/system/bta/jv/bta_jv_act.cc +++ b/system/bta/jv/bta_jv_act.cc @@ -38,8 +38,6 @@ #include "osi/include/allocator.h" #include "osi/include/properties.h" #include "stack/btm/btm_sec.h" -#include "stack/include/avct_api.h" // AVCT_PSM -#include "stack/include/avdt_api.h" // AVDT_PSM #include "stack/include/bt_hdr.h" #include "stack/include/bt_psm_types.h" #include "stack/include/bt_types.h" @@ -621,8 +619,8 @@ bool bta_jv_check_psm(uint16_t psm) { } break; - case AVCT_PSM: /* 0x17 */ - case AVDT_PSM: /* 0x19 */ + case BT_PSM_AVCTP: /* 0x17 */ + case BT_PSM_AVDTP: /* 0x19 */ if (!bta_sys_is_register(BTA_ID_AV)) { ret = true; } diff --git a/system/btif/avrcp/avrcp_service.cc b/system/btif/avrcp/avrcp_service.cc index 577628eb2b..ce1b1ce577 100644 --- a/system/btif/avrcp/avrcp_service.cc +++ b/system/btif/avrcp/avrcp_service.cc @@ -32,6 +32,7 @@ #include "btif/include/btif_av.h" #include "btif/include/btif_common.h" #include "osi/include/osi.h" +#include "profile/avrcp/avrcp_config.h" #include "profile/avrcp/device.h" #include "stack/include/a2dp_api.h" #include "stack/include/bt_hdr.h" @@ -100,7 +101,7 @@ public: return AVRC_Open(p_handle, p_ccb, bd_addr); } - uint16_t OpenBrowse(uint8_t handle, uint8_t conn_role) override { + uint16_t OpenBrowse(uint8_t handle, tAVCT_ROLE conn_role) override { return AVRC_OpenBrowse(handle, conn_role); } diff --git a/system/profile/avrcp/avrcp_internal.h b/system/profile/avrcp/avrcp_internal.h index 665382ce61..a863cb7cf5 100644 --- a/system/profile/avrcp/avrcp_internal.h +++ b/system/profile/avrcp/avrcp_internal.h @@ -16,11 +16,9 @@ #pragma once -#include "avrcp_config.h" #include "stack/include/a2dp_api.h" #include "stack/include/avrc_api.h" #include "stack/include/bt_hdr.h" -#include "stack/include/sdp_api.h" #include "types/bluetooth/uuid.h" #include "types/raw_address.h" @@ -48,7 +46,7 @@ public: virtual uint16_t Open(uint8_t* p_handle, tAVRC_CONN_CB* p_ccb, const RawAddress& bd_addr) = 0; - virtual uint16_t OpenBrowse(uint8_t handle, uint8_t conn_role) = 0; + virtual uint16_t OpenBrowse(uint8_t handle, tAVCT_ROLE conn_role) = 0; virtual uint16_t GetPeerMtu(uint8_t handle) = 0; diff --git a/system/profile/avrcp/connection_handler.cc b/system/profile/avrcp/connection_handler.cc index 6faf5cf0e7..9608294161 100644 --- a/system/profile/avrcp/connection_handler.cc +++ b/system/profile/avrcp/connection_handler.cc @@ -233,14 +233,14 @@ bool ConnectionHandler::AvrcpConnect(bool initiator, const RawAddress& bdaddr) { } open_cb.msg_cback = base::Bind(&ConnectionHandler::MessageCb, weak_ptr_factory_.GetWeakPtr()); open_cb.company_id = AVRC_CO_GOOGLE; - open_cb.conn = initiator ? AVRC_CONN_INT : AVRC_CONN_ACP; // 0 if initiator, 1 if acceptor + open_cb.conn = initiator ? AVCT_ROLE_INITIATOR : AVCT_ROLE_ACCEPTOR; // TODO (apanicke): We shouldn't need RCCT to do absolute volume. The current // AVRC_API requires it though. open_cb.control = BTA_AV_FEAT_RCTG | BTA_AV_FEAT_RCCT | BTA_AV_FEAT_METADATA | AVRC_CT_PASSIVE; uint8_t handle = 0; uint16_t status = avrc_->Open(&handle, &open_cb, bdaddr); - log::info("handle=0x{:x} status= 0x{:x}", handle, status); + log::info("handle=0x{:x} status=0x{:x}", handle, status); return status == AVRC_SUCCESS; } @@ -266,7 +266,7 @@ void ConnectionHandler::InitiatorControlCb(uint8_t handle, uint8_t event, uint16 bool supports_browsing = feature_iter->second & BTA_AV_FEAT_BROWSE; if (supports_browsing) { - avrc_->OpenBrowse(handle, AVCT_INT); + avrc_->OpenBrowse(handle, AVCT_ROLE_INITIATOR); } // TODO (apanicke): Implement a system to cache SDP entries. For most @@ -395,7 +395,7 @@ void ConnectionHandler::AcceptorControlCb(uint8_t handle, uint8_t event, uint16_ }; if (SdpLookup(*peer_addr, base::Bind(sdp_lambda, this, handle), false)) { - avrc_->OpenBrowse(handle, AVCT_ACP); + avrc_->OpenBrowse(handle, AVCT_ROLE_ACCEPTOR); } else { // SDP search failed, this could be due to a collision between outgoing // and incoming connection. In any case, we need to reject the current diff --git a/system/profile/avrcp/tests/avrcp_connection_handler_test.cc b/system/profile/avrcp/tests/avrcp_connection_handler_test.cc index 73334576f1..080bda09f3 100644 --- a/system/profile/avrcp/tests/avrcp_connection_handler_test.cc +++ b/system/profile/avrcp/tests/avrcp_connection_handler_test.cc @@ -206,7 +206,7 @@ TEST_F(AvrcpConnectionHandlerTest, remoteDeviceConnectionTest) { // Set an Expectation that OpenBrowse will be called in acceptor mode when the // device connects. - EXPECT_CALL(mock_avrcp_, OpenBrowse(1, AVCT_ACP)).Times(1); + EXPECT_CALL(mock_avrcp_, OpenBrowse(1, AVCT_ROLE_ACCEPTOR)).Times(1); if (com::android::bluetooth::flags::avrcp_connect_a2dp_with_delay()) { // Set an expectation that SDP for audio will be performed @@ -374,7 +374,7 @@ TEST_F(AvrcpConnectionHandlerTest, multipleRemoteDeviceConnectionTest) { // Set an Expectation that OpenBrowse will be called in acceptor mode when the // device connects on handle 1 - EXPECT_CALL(mock_avrcp_, OpenBrowse(1, AVCT_ACP)).Times(1); + EXPECT_CALL(mock_avrcp_, OpenBrowse(1, AVCT_ROLE_ACCEPTOR)).Times(1); // Call the callback with a message saying that a remote device has connected conn_cb.ctrl_cback.Run(1, AVRC_OPEN_IND_EVT, 0, &RawAddress::kAny); @@ -391,7 +391,7 @@ TEST_F(AvrcpConnectionHandlerTest, multipleRemoteDeviceConnectionTest) { // Set an Expectation that OpenBrowse will be called in acceptor mode when the // device connects on handle 2 - EXPECT_CALL(mock_avrcp_, OpenBrowse(2, AVCT_ACP)).Times(1); + EXPECT_CALL(mock_avrcp_, OpenBrowse(2, AVCT_ROLE_ACCEPTOR)).Times(1); // Call the callback with a message saying that a remote device has connected // with a different address @@ -504,7 +504,7 @@ TEST_F(AvrcpConnectionHandlerTest, connectToBrowsableRemoteDeviceTest) { // Set an Expectation that OpenBrowse will be called since browsing is listed // as supported in SDP - EXPECT_CALL(mock_avrcp_, OpenBrowse(1, AVCT_INT)).Times(1); + EXPECT_CALL(mock_avrcp_, OpenBrowse(1, AVCT_ROLE_INITIATOR)).Times(1); // Call the callback with a message saying that a remote device has connected // with a different address diff --git a/system/profile/avrcp/tests/avrcp_test_helper.h b/system/profile/avrcp/tests/avrcp_test_helper.h index 7a2f63ab2a..ea6ed4d828 100644 --- a/system/profile/avrcp/tests/avrcp_test_helper.h +++ b/system/profile/avrcp/tests/avrcp_test_helper.h @@ -80,7 +80,7 @@ public: MOCK_METHOD4(FindService, uint16_t(uint16_t, const RawAddress&, tAVRC_SDP_DB_PARAMS*, tAVRC_FIND_CBACK)); MOCK_METHOD3(Open, uint16_t(uint8_t*, tAVRC_CONN_CB*, const RawAddress&)); - MOCK_METHOD2(OpenBrowse, uint16_t(uint8_t, uint8_t)); + MOCK_METHOD2(OpenBrowse, uint16_t(uint8_t, tAVCT_ROLE)); MOCK_METHOD1(GetPeerMtu, uint16_t(uint8_t)); MOCK_METHOD1(GetBrowseMtu, uint16_t(uint8_t)); MOCK_METHOD1(Close, uint16_t(uint8_t)); diff --git a/system/stack/avct/avct_api.cc b/system/stack/avct/avct_api.cc index 10c441d876..2c4426f1a5 100644 --- a/system/stack/avct/avct_api.cc +++ b/system/stack/avct/avct_api.cc @@ -33,6 +33,7 @@ #include "internal_include/bt_target.h" #include "osi/include/allocator.h" #include "stack/include/bt_hdr.h" +#include "stack/include/bt_psm_types.h" #include "stack/include/l2cap_interface.h" #include "types/raw_address.h" @@ -68,20 +69,20 @@ void AVCT_Register() { /* register PSM with L2CAP */ if (!stack::l2cap::get_interface().L2CA_RegisterWithSecurity( - AVCT_PSM, avct_l2c_appl, true /* enable_snoop */, nullptr, kAvrcMtu, 0, sec)) { - log::error("Unable to register with L2CAP AVCT profile psm:AVCT_PSM[0x0017]"); + BT_PSM_AVCTP, avct_l2c_appl, true /* enable_snoop */, nullptr, kAvrcMtu, 0, sec)) { + log::error("Unable to register with L2CAP AVCT profile psm:{}", bt_psm_text(BT_PSM_AVCTP)); } /* Include the browsing channel which uses eFCR */ - tL2CAP_ERTM_INFO ertm_info; - ertm_info.preferred_mode = L2CAP_FCR_ERTM_MODE; + tL2CAP_ERTM_INFO ertm_info = { + .preferred_mode = L2CAP_FCR_ERTM_MODE, + }; if (!stack::l2cap::get_interface().L2CA_RegisterWithSecurity( - AVCT_BR_PSM, avct_l2c_br_appl, true /*enable_snoop*/, &ertm_info, kAvrcBrMtu, + BT_PSM_AVCTP_BROWSE, avct_l2c_br_appl, true /*enable_snoop*/, &ertm_info, kAvrcBrMtu, AVCT_MIN_BROWSE_MTU, sec)) { - log::error( - "Unable to register with L2CAP AVCT_BR profile " - "psm:AVCT_BR_PSM[0x001b]"); + log::error("Unable to register with L2CAP AVCT_BROWSE profile psm:{}", + bt_psm_text(BT_PSM_AVCTP_BROWSE)); } } @@ -103,10 +104,10 @@ void AVCT_Deregister(void) { log::verbose("AVCT_Deregister"); /* deregister PSM with L2CAP */ - stack::l2cap::get_interface().L2CA_Deregister(AVCT_PSM); + stack::l2cap::get_interface().L2CA_Deregister(BT_PSM_AVCTP); /* deregister AVCT_BR_PSM with L2CAP */ - stack::l2cap::get_interface().L2CA_Deregister(AVCT_BR_PSM); + stack::l2cap::get_interface().L2CA_Deregister(BT_PSM_AVCTP_BROWSE); } /******************************************************************************* @@ -131,7 +132,7 @@ uint16_t AVCT_CreateConn(uint8_t* p_handle, tAVCT_CC* p_cc, const RawAddress& pe tAVCT_CCB* p_ccb; tAVCT_LCB* p_lcb; - log::verbose("AVCT_CreateConn: {}, control:{}", p_cc->role, p_cc->control); + log::verbose("AVCT_CreateConn:{}, control:0x{:x}", avct_role_text(p_cc->role), p_cc->control); /* Allocate ccb; if no ccbs, return failure */ p_ccb = avct_ccb_alloc(p_cc); @@ -142,7 +143,7 @@ uint16_t AVCT_CreateConn(uint8_t* p_handle, tAVCT_CC* p_cc, const RawAddress& pe *p_handle = avct_ccb_to_idx(p_ccb); /* if initiator connection */ - if (p_cc->role == AVCT_INT) { + if (p_cc->role == AVCT_ROLE_INITIATOR) { /* find link; if none allocate a new one */ p_lcb = avct_lcb_by_bd(peer_addr); if (p_lcb == NULL) { @@ -162,9 +163,10 @@ uint16_t AVCT_CreateConn(uint8_t* p_handle, tAVCT_CC* p_cc, const RawAddress& pe if (result == AVCT_SUCCESS) { /* bind lcb to ccb */ p_ccb->p_lcb = p_lcb; - log::verbose("ch_state: {}", p_lcb->ch_state); - tAVCT_LCB_EVT avct_lcb_evt; - avct_lcb_evt.p_ccb = p_ccb; + log::verbose("ch_state:{}", avct_ch_state_text(p_lcb->ch_state)); + tAVCT_LCB_EVT avct_lcb_evt = { + .p_ccb = p_ccb, + }; avct_lcb_event(p_lcb, AVCT_LCB_UL_BIND_EVT, &avct_lcb_evt); } } @@ -226,13 +228,13 @@ uint16_t AVCT_RemoveConn(uint8_t handle) { * Returns AVCT_SUCCESS if successful, otherwise error. * ******************************************************************************/ -uint16_t AVCT_CreateBrowse(uint8_t handle, uint8_t role) { +uint16_t AVCT_CreateBrowse(uint8_t handle, tAVCT_ROLE role) { uint16_t result = AVCT_SUCCESS; tAVCT_CCB* p_ccb; tAVCT_BCB* p_bcb; int index; - log::verbose("AVCT_CreateBrowse: {}", role); + log::verbose("AVCT_CreateBrowse: role:{}", avct_role_text(role)); /* map handle to ccb */ p_ccb = avct_ccb_by_idx(handle); @@ -246,7 +248,7 @@ uint16_t AVCT_CreateBrowse(uint8_t handle, uint8_t role) { } /* if initiator connection */ - if (role == AVCT_INT) { + if (role == AVCT_ROLE_INITIATOR) { /* the link control block must exist before this function is called as INT. */ if ((p_ccb->p_lcb == NULL) || (p_ccb->p_lcb->allocated == 0)) { @@ -266,7 +268,7 @@ uint16_t AVCT_CreateBrowse(uint8_t handle, uint8_t role) { /* bind bcb to ccb */ p_ccb->p_bcb = p_bcb; p_bcb->peer_addr = p_ccb->p_lcb->peer_addr; - log::verbose("ch_state: {}", p_bcb->ch_state); + log::verbose("Created BCB ch_state:{}", avct_ch_state_text(p_bcb->ch_state)); tAVCT_LCB_EVT avct_lcb_evt; avct_lcb_evt.p_ccb = p_ccb; avct_bcb_event(p_bcb, AVCT_LCB_UL_BIND_EVT, &avct_lcb_evt); @@ -390,7 +392,7 @@ uint16_t AVCT_MsgReq(uint8_t handle, uint8_t label, uint8_t cr, BT_HDR* p_msg) { if (p_msg == NULL) { return AVCT_NO_RESOURCES; } - log::verbose("len: {} layer_specific: {}", p_msg->len, p_msg->layer_specific); + log::verbose("msg_len:{} msg_layer_specific:{}", p_msg->len, p_msg->layer_specific); /* map handle to ccb */ p_ccb = avct_ccb_by_idx(handle); diff --git a/system/stack/avct/avct_bcb_act.cc b/system/stack/avct/avct_bcb_act.cc index 9693888370..37a5d20a97 100644 --- a/system/stack/avct/avct_bcb_act.cc +++ b/system/stack/avct/avct_bcb_act.cc @@ -25,8 +25,6 @@ * *****************************************************************************/ -#define LOG_TAG "bluetooth" - #include <bluetooth/log.h> #include <com_android_bluetooth_flags.h> #include <string.h> @@ -39,11 +37,29 @@ #include "stack/avct/avct_int.h" #include "stack/include/avct_api.h" #include "stack/include/bt_hdr.h" +#include "stack/include/bt_psm_types.h" #include "stack/include/bt_types.h" #include "stack/include/l2cap_interface.h" using namespace bluetooth; +static void avct_bcb_chnl_open(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_chnl_disc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_send_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_open_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_open_fail(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_close_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_close_cfm(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_cong_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_bind_conn(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_bind_fail(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_unbind_disc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_chk_disc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_discard_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_dealloc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); +static void avct_bcb_free_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); + /* action function list */ const tAVCT_BCB_ACTION avct_bcb_action[] = { avct_bcb_chnl_open, /* AVCT_LCB_CHNL_OPEN */ @@ -91,7 +107,7 @@ static BT_HDR* avct_bcb_msg_asmbl(tAVCT_BCB* /* p_bcb */, BT_HDR* p_buf) { /* must be single packet - can not fragment */ if (pkt_type != AVCT_PKT_TYPE_SINGLE) { osi_free_and_reset((void**)&p_buf); - log::warn("Pkt type={} - fragmentation not allowed. drop it", pkt_type); + log::warn("Pkt type:{} - fragmentation not allowed. drop it", pkt_type); } return p_buf; } @@ -109,19 +125,15 @@ static BT_HDR* avct_bcb_msg_asmbl(tAVCT_BCB* /* p_bcb */, BT_HDR* p_buf) { void avct_bcb_chnl_open(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* /* p_data */) { uint16_t result = AVCT_RESULT_FAIL; tAVCT_LCB* p_lcb = avct_lcb_by_bcb(p_bcb); - tL2CAP_ERTM_INFO ertm_info; - - /* Set the FCR options: Browsing channel mandates ERTM */ - ertm_info.preferred_mode = L2CAP_FCR_ERTM_MODE; /* call l2cap connect req */ p_bcb->ch_state = AVCT_CH_CONN; if (com::android::bluetooth::flags::use_encrypt_req_for_av()) { p_bcb->ch_lcid = stack::l2cap::get_interface().L2CA_ConnectReqWithSecurity( - AVCT_BR_PSM, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE | BTA_SEC_ENCRYPT); + BT_PSM_AVCTP_BROWSE, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE | BTA_SEC_ENCRYPT); } else { p_bcb->ch_lcid = stack::l2cap::get_interface().L2CA_ConnectReqWithSecurity( - AVCT_BR_PSM, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE); + BT_PSM_AVCTP_BROWSE, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE); } if (p_bcb->ch_lcid == 0) { /* if connect req failed, send ourselves close event */ @@ -166,7 +178,7 @@ bool is_valid_role_check(const tAVCT_CCB* p_ccb) { return com::android::bluetooth::flags:: associate_browse_l2cap_request_with_active_control_channel() ? true - : p_ccb->cc.role == AVCT_ACP; + : p_ccb->cc.role == AVCT_ROLE_ACCEPTOR; } } // namespace @@ -264,7 +276,7 @@ void avct_bcb_close_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* /* p_data */) { for (int idx = 0; idx < AVCT_NUM_CONN; idx++, p_ccb++) { if (p_ccb->allocated && (p_ccb->p_bcb == p_bcb)) { - if (p_ccb->cc.role == AVCT_INT) { + if (p_ccb->cc.role == AVCT_ROLE_INITIATOR) { (*p_ccb->cc.p_ctrl_cback)(avct_ccb_to_idx(p_ccb), AVCT_BROWSE_DISCONN_CFM_EVT, 0, &p_lcb->peer_addr); } else { @@ -288,26 +300,19 @@ void avct_bcb_close_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* /* p_data */) { * ******************************************************************************/ void avct_bcb_close_cfm(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { - tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; - uint8_t event = 0; /* Whether BCB initiated channel close */ - bool ch_close = p_bcb->ch_close; - tAVCT_CTRL_CBACK* p_cback; - + const bool ch_close = p_bcb->ch_close; p_bcb->ch_close = false; p_bcb->allocated = 0; + + tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; for (int idx = 0; idx < AVCT_NUM_CONN; idx++, p_ccb++) { if (p_ccb->allocated && (p_ccb->p_bcb == p_bcb)) { /* if this ccb initiated close send disconnect cfm otherwise ind */ - if (ch_close) { - event = AVCT_BROWSE_DISCONN_CFM_EVT; - } else { - event = AVCT_BROWSE_DISCONN_IND_EVT; - } - - p_cback = p_ccb->cc.p_ctrl_cback; - p_ccb->p_bcb = NULL; - if (p_ccb->p_lcb == NULL) { + uint8_t event = ch_close ? AVCT_BROWSE_DISCONN_CFM_EVT : AVCT_BROWSE_DISCONN_IND_EVT; + tAVCT_CTRL_CBACK* p_cback = p_ccb->cc.p_ctrl_cback; + p_ccb->p_bcb = nullptr; + if (p_ccb->p_lcb == nullptr) { avct_ccb_dealloc(p_ccb, AVCT_NO_EVT, 0, NULL); } (*p_cback)(avct_ccb_to_idx(p_ccb), event, p_data->result, &p_bcb->peer_addr); @@ -397,11 +402,10 @@ void avct_bcb_bind_fail(tAVCT_BCB* /* p_bcb */, tAVCT_LCB_EVT* p_data) { ******************************************************************************/ void avct_bcb_cong_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; - uint8_t event; tAVCT_LCB* p_lcb = avct_lcb_by_bcb(p_bcb); /* set event */ - event = (p_data->cong) ? AVCT_BROWSE_CONG_IND_EVT : AVCT_BROWSE_UNCONG_IND_EVT; + uint8_t event = (p_data->cong) ? AVCT_BROWSE_CONG_IND_EVT : AVCT_BROWSE_UNCONG_IND_EVT; /* send event to all ccbs on this lcb */ for (int idx = 0; idx < AVCT_NUM_CONN; idx++, p_ccb++) { @@ -435,7 +439,8 @@ void avct_bcb_discard_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { p_bcb->p_tx_msg->layer_specific = (p_data->ul_msg.cr << 8) + p_data->ul_msg.label; /* the channel is closed, opening or closing - open it again */ - log::verbose("ch_state: {}, allocated:{}->{}", p_bcb->ch_state, p_bcb->allocated, + log::verbose("ch_state:{} bcb_allocated:{} ccb_lcb_allocated:{}", + avct_ch_state_text(p_bcb->ch_state), p_bcb->allocated, p_data->ul_msg.p_ccb->p_lcb->allocated); p_bcb->allocated = p_data->ul_msg.p_ccb->p_lcb->allocated; avct_bcb_event(p_bcb, AVCT_LCB_UL_BIND_EVT, (tAVCT_LCB_EVT*)p_data->ul_msg.p_ccb); @@ -464,7 +469,7 @@ void avct_bcb_send_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { /* initialize packet type and other stuff */ if (curr_msg_len > (p_bcb->peer_mtu - AVCT_HDR_LEN_SINGLE)) { - log::error("msg len ({}) exceeds peer mtu({}-{})!!", curr_msg_len, p_bcb->peer_mtu, + log::error("msg_len:{} exceeds peer mtu:{} header-{})!!", curr_msg_len, p_bcb->peer_mtu, AVCT_HDR_LEN_SINGLE); osi_free_and_reset((void**)&p_data->ul_msg.p_buf); return; @@ -488,7 +493,7 @@ void avct_bcb_send_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { /* send message to L2CAP */ if (stack::l2cap::get_interface().L2CA_DataWrite(p_bcb->ch_lcid, p_buf) != tL2CAP_DW_RESULT::SUCCESS) { - log::warn("Unable to write L2CAP data peer:{} cid:{}", p_bcb->peer_addr, p_bcb->ch_lcid); + log::warn("Unable to write L2CAP data peer:{} cid:0x{:04x}", p_bcb->peer_addr, p_bcb->ch_lcid); } } @@ -543,7 +548,7 @@ void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { } if (p_data->p_buf->len < AVCT_HDR_LEN_SINGLE) { - log::warn("Invalid AVCTP packet length {}: must be at least {}", p_data->p_buf->len, + log::warn("Invalid AVCTP packet length:{} must be at least:{}", p_data->p_buf->len, AVCT_HDR_LEN_SINGLE); osi_free_and_reset((void**)&p_data->p_buf); return; @@ -558,7 +563,7 @@ void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { /* check for invalid cr_ipid */ if (cr_ipid == AVCT_CR_IPID_INVALID) { - log::warn("Invalid cr_ipid {}", cr_ipid); + log::warn("Invalid cr_ipid:{}", cr_ipid); osi_free_and_reset((void**)&p_data->p_buf); return; } @@ -583,7 +588,7 @@ void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { } /* PID not found; drop message */ - log::warn("No ccb for PID={:x}", pid); + log::warn("No ccb for PID=0x{:x}", pid); osi_free_and_reset((void**)&p_data->p_buf); /* if command send reject */ @@ -597,7 +602,8 @@ void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { p_buf->layer_specific = AVCT_DATA_BROWSE; if (stack::l2cap::get_interface().L2CA_DataWrite(p_bcb->ch_lcid, p_buf) != tL2CAP_DW_RESULT::SUCCESS) { - log::warn("Unable to write L2CAP data peer:{} cid:{}", p_bcb->peer_addr, p_bcb->ch_lcid); + log::warn("Unable to write L2CAP data peer:{} cid:0x{:04x}", p_bcb->peer_addr, + p_bcb->ch_lcid); } } } @@ -615,13 +621,13 @@ void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data) { void avct_bcb_dealloc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* /* p_data */) { tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; - log::verbose("{}", p_bcb->allocated); + log::verbose("BCB allocated:{}", p_bcb->allocated); for (int idx = 0; idx < AVCT_NUM_CONN; idx++, p_ccb++) { /* if ccb allocated and */ if ((p_ccb->allocated) && (p_ccb->p_bcb == p_bcb)) { p_ccb->p_bcb = NULL; - log::verbose("used by ccb: {}", idx); + log::verbose("used by ccb idx:{}", idx); break; } } @@ -716,6 +722,6 @@ tAVCT_BCB* avct_bcb_by_lcid(uint16_t lcid) { } /* out of lcbs */ - log::warn("No bcb for lcid {:x}", lcid); - return NULL; + log::warn("No bcb for lcid 0x{:04x}", lcid); + return nullptr; } diff --git a/system/stack/avct/avct_ccb.cc b/system/stack/avct/avct_ccb.cc index 6210fd2fa7..6ffa34c3fb 100644 --- a/system/stack/avct/avct_ccb.cc +++ b/system/stack/avct/avct_ccb.cc @@ -23,8 +23,6 @@ * ******************************************************************************/ -#define LOG_TAG "avctp" - #include <bluetooth/log.h> #include <string.h> @@ -53,7 +51,7 @@ tAVCT_CCB* avct_ccb_alloc(tAVCT_CC* p_cc) { if (!p_ccb->allocated) { p_ccb->allocated = AVCT_ALOC_LCB; memcpy(&p_ccb->cc, p_cc, sizeof(tAVCT_CC)); - log::verbose("avct_ccb_alloc {}", i); + log::verbose("Allocated ccb idx:{}", i); break; } } @@ -80,7 +78,7 @@ tAVCT_CCB* avct_ccb_alloc(tAVCT_CC* p_cc) { void avct_ccb_dealloc(tAVCT_CCB* p_ccb, uint8_t event, uint16_t result, const RawAddress* bd_addr) { tAVCT_CTRL_CBACK* p_cback = p_ccb->cc.p_ctrl_cback; - log::verbose("avct_ccb_dealloc {}", avct_ccb_to_idx(p_ccb)); + log::verbose("Deallocating idx:{}", avct_ccb_to_idx(p_ccb)); if (p_ccb->p_bcb == NULL) { memset(p_ccb, 0, sizeof(tAVCT_CCB)); @@ -131,11 +129,11 @@ tAVCT_CCB* avct_ccb_by_idx(uint8_t idx) { /* verify ccb is allocated */ if (!p_ccb->allocated) { p_ccb = NULL; - log::warn("ccb {} not allocated", idx); + log::warn("ccb idx:{} not allocated", idx); } } else { p_ccb = NULL; - log::warn("No ccb for idx {}", idx); + log::warn("No ccb for idx:{}", idx); } return p_ccb; } diff --git a/system/stack/avct/avct_int.h b/system/stack/avct/avct_int.h index 0800523062..9efe7ad25e 100644 --- a/system/stack/avct/avct_int.h +++ b/system/stack/avct/avct_int.h @@ -24,7 +24,10 @@ #ifndef AVCT_INT_H #define AVCT_INT_H +#include <string> + #include "avct_api.h" +#include "include/macros.h" #include "internal_include/bt_target.h" #include "osi/include/fixed_queue.h" #include "stack/include/bt_hdr.h" @@ -48,10 +51,22 @@ enum { }; /* "states" used for L2CAP channel */ -#define AVCT_CH_IDLE 0 /* No connection */ -#define AVCT_CH_CONN 1 /* Waiting for connection confirm */ -#define AVCT_CH_CFG 2 /* Waiting for configuration complete */ -#define AVCT_CH_OPEN 3 /* Channel opened */ +enum tAVCT_CH { + AVCT_CH_IDLE = 0, /* No connection */ + AVCT_CH_CONN = 1, /* Waiting for connection confirm */ + AVCT_CH_CFG = 2, /* Waiting for configuration complete */ + AVCT_CH_OPEN = 3, /* Channel opened */ +}; + +inline std::string avct_ch_state_text(const int& state) { + switch (state) { + CASE_RETURN_STRING(AVCT_CH_IDLE); + CASE_RETURN_STRING(AVCT_CH_CONN); + CASE_RETURN_STRING(AVCT_CH_CFG); + CASE_RETURN_STRING(AVCT_CH_OPEN); + } + RETURN_UNKNOWN_TYPE_STRING(int, state); +} /* "no event" indicator used by ccb dealloc */ #define AVCT_NO_EVT 0xFF @@ -59,16 +74,6 @@ enum { /***************************************************************************** * data types ****************************************************************************/ -/* sub control block type - common data members for tAVCT_LCB and tAVCT_BCB */ -typedef struct { - uint16_t peer_mtu; /* peer l2c mtu */ - uint16_t ch_result; /* L2CAP connection result value */ - uint16_t ch_lcid; /* L2CAP channel LCID */ - uint8_t allocated; /* 0, not allocated. index+1, otherwise. */ - uint8_t state; /* The state machine state */ - uint8_t ch_state; /* L2CAP channel state */ -} tAVCT_SCB; - /* link control block type */ typedef struct { uint16_t peer_mtu; /* peer l2c mtu */ @@ -89,7 +94,7 @@ typedef struct { uint16_t peer_mtu; /* peer l2c mtu */ uint16_t ch_result; /* L2CAP connection result value */ uint16_t ch_lcid; /* L2CAP channel LCID */ - uint8_t allocated; /* 0, not allocated. index+1, otherwise. */ + uint8_t allocated; // 0: no link allocated. otherwise link index+1 uint8_t state; /* The state machine state */ uint8_t ch_state; /* L2CAP channel state */ uint16_t conflict_lcid; /* L2CAP channel LCID */ @@ -173,23 +178,6 @@ void avct_lcb_free_msg_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data); /* BCB action functions */ typedef void (*tAVCT_BCB_ACTION)(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_chnl_open(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_unbind_disc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_open_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_open_fail(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_close_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_close_cfm(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_bind_conn(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_chk_disc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_chnl_disc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_bind_fail(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_cong_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_discard_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_send_msg(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); -void avct_bcb_free_msg_ind(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); - -void avct_bcb_dealloc(tAVCT_BCB* p_bcb, tAVCT_LCB_EVT* p_data); extern const tAVCT_BCB_ACTION avct_bcb_action[]; extern const uint8_t avct_lcb_pkt_type_len[]; @@ -203,6 +191,8 @@ tAVCT_CCB* avct_ccb_by_idx(uint8_t idx); extern bool avct_msg_ind_for_src_sink_coexist(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data, uint8_t label, uint8_t cr_ipid, uint16_t pid); +std::string avct_sm_state_text(const int& state); + /***************************************************************************** * global data ****************************************************************************/ diff --git a/system/stack/avct/avct_l2c.cc b/system/stack/avct/avct_l2c.cc index adca67c851..ffb3a64d3a 100644 --- a/system/stack/avct/avct_l2c.cc +++ b/system/stack/avct/avct_l2c.cc @@ -21,8 +21,6 @@ * This AVCTP module interfaces to L2CAP * ******************************************************************************/ -#define LOG_TAG "avctp" - #include <bluetooth/log.h> #include "avct_api.h" @@ -32,18 +30,20 @@ #include "osi/include/allocator.h" #include "stack/include/bt_hdr.h" #include "stack/include/l2cap_interface.h" +#include "stack/include/l2cdefs.h" #include "types/raw_address.h" using namespace bluetooth; /* callback function declarations */ -void avct_l2c_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16_t psm, uint8_t id); -void avct_l2c_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result); -void avct_l2c_config_cfm_cback(uint16_t lcid, uint16_t result, tL2CAP_CFG_INFO* p_cfg); -void avct_l2c_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg); -void avct_l2c_disconnect_ind_cback(uint16_t lcid, bool ack_needed); -void avct_l2c_congestion_ind_cback(uint16_t lcid, bool is_congested); -void avct_l2c_data_ind_cback(uint16_t lcid, BT_HDR* p_buf); +static void avct_l2c_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16_t psm, + uint8_t id); +static void avct_l2c_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result); +static void avct_l2c_config_cfm_cback(uint16_t lcid, uint16_t result, tL2CAP_CFG_INFO* p_cfg); +static void avct_l2c_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg); +static void avct_l2c_disconnect_ind_cback(uint16_t lcid, bool ack_needed); +static void avct_l2c_congestion_ind_cback(uint16_t lcid, bool is_congested); +static void avct_l2c_data_ind_cback(uint16_t lcid, BT_HDR* p_buf); static void avct_on_l2cap_error(uint16_t lcid, uint16_t result); /* L2CAP callback function structure */ @@ -103,21 +103,19 @@ static bool avct_l2c_is_passive(tAVCT_LCB* p_lcb) { ******************************************************************************/ void avct_l2c_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16_t /* psm */, uint8_t /* id */) { - tAVCT_LCB* p_lcb; tL2CAP_CONN result = tL2CAP_CONN::L2CAP_CONN_OK; /* do we already have a channel for this peer? */ - p_lcb = avct_lcb_by_bd(bd_addr); - if (p_lcb == NULL) { + tAVCT_LCB* p_lcb = avct_lcb_by_bd(bd_addr); + if (p_lcb == nullptr) { /* no, allocate lcb */ p_lcb = avct_lcb_alloc(bd_addr); - if (p_lcb == NULL) { + if (p_lcb == nullptr) { /* no ccb available, reject L2CAP connection */ result = tL2CAP_CONN::L2CAP_CONN_NO_RESOURCES; } - } - /* else we already have a channel for this peer */ - else { + } else { + /* else we already have a channel for this peer */ if (!avct_l2c_is_passive(p_lcb) || (p_lcb->ch_state == AVCT_CH_OPEN)) { /* this LCB included CT role - reject */ result = tL2CAP_CONN::L2CAP_CONN_NO_RESOURCES; @@ -125,19 +123,8 @@ void avct_l2c_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16 /* TG role only - accept the connection from CT. move the channel ID to * the conflict list */ p_lcb->conflict_lcid = p_lcb->ch_lcid; - log::verbose("avct_l2c_connect_ind_cback conflict_lcid:0x{:x}", p_lcb->conflict_lcid); - } - } - - if (p_lcb) { - log::verbose("avct_l2c_connect_ind_cback: 0x{:x}, res: {}, ch_state: {}", lcid, result, - p_lcb->ch_state); - } - - /* If we reject the connection, send DisconnectReq */ - if (result != tL2CAP_CONN::L2CAP_CONN_OK) { - if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { - log::warn("Unable to send L2CAP disconnect request peer:{} cid:{}", bd_addr, lcid); + log::verbose("Accept connection from controller lcid:0x{:04x} conflict_lcid:0x{:04x}", lcid, + p_lcb->conflict_lcid); } } @@ -146,10 +133,13 @@ void avct_l2c_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16 if (btif_av_src_sink_coexist_enabled()) { tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; for (int i = 0; i < AVCT_NUM_CONN; i++, p_ccb++) { - if (p_ccb && p_ccb->allocated && (p_ccb->p_lcb == NULL) && (p_ccb->cc.role == AVCT_ACP)) { + if (p_ccb && p_ccb->allocated && (p_ccb->p_lcb == NULL) && + (p_ccb->cc.role == AVCT_ROLE_ACCEPTOR)) { p_ccb->p_lcb = p_lcb; - log::verbose("ACP bind {} ccb to lcb, alloc {}, lcb {}, role {}, pid 0x{:x}", i, - p_ccb->allocated, fmt::ptr(p_ccb->p_lcb), p_ccb->cc.role, p_ccb->cc.pid); + log::verbose( + "Source and sink coexistance enabled acceptor bind ccb to lcb idx:{} " + "allocated:{} role {} pid 0x{:x}", + i, p_ccb->allocated, avct_role_text(p_ccb->cc.role), p_ccb->cc.pid); } } } @@ -158,10 +148,19 @@ void avct_l2c_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16 /* transition to configuration state */ p_lcb->ch_state = AVCT_CH_CFG; + } else { + /* If we reject the connection, send DisconnectReq */ + if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { + log::warn("Unable to send L2CAP disconnect request peer:{} lcid:0x{:04x}", bd_addr, lcid); + } } if (p_lcb) { - log::verbose("ch_state cni: {}", p_lcb->ch_state); + log::debug("Received remote connection request peer:{} lcid:0x{:04x} res:{} ch_state:{}", + bd_addr, lcid, l2cap_result_code_text(result), avct_ch_state_text(p_lcb->ch_state)); + } else { + log::info("Ignoring remote connection request peer:{} lcid:0x{:04x} res:{} ch_state:{}", + bd_addr, lcid, l2cap_result_code_text(result), avct_ch_state_text(p_lcb->ch_state)); } } @@ -186,7 +185,8 @@ static void avct_on_l2cap_error(uint16_t lcid, uint16_t result) { /* Send L2CAP disconnect req */ if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { - log::warn("Unable to send L2CAP disconnect request peer:{} cid:{}", p_lcb->peer_addr, lcid); + log::warn("Unable to send L2CAP disconnect request peer:{} lcid:0x{:04x}", p_lcb->peer_addr, + lcid); } } } @@ -207,36 +207,34 @@ void avct_l2c_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result) { /* look up lcb for this channel */ p_lcb = avct_lcb_by_lcid(lcid); if (p_lcb != NULL) { - log::verbose( - "avct_l2c_connect_cfm_cback lcid:0x{:x} result: {} ch_state: {}, " - "conflict_lcid:0x{:x}", - lcid, result, p_lcb->ch_state, p_lcb->conflict_lcid); + log::verbose("lcid:0x{:04x} result:{} ch_state:{} conflict_lcid:0x{:04x}", lcid, result, + avct_ch_state_text(p_lcb->ch_state), p_lcb->conflict_lcid); /* if in correct state */ if (p_lcb->ch_state == AVCT_CH_CONN) { /* if result successful */ if (result == tL2CAP_CONN::L2CAP_CONN_OK) { /* set channel state */ p_lcb->ch_state = AVCT_CH_CFG; - } - /* else failure */ - else { - log::error("invoked with non OK status"); + } else { + /* else failure */ + log::error("invoked with non OK status lcid:0x{:04x} result:{}", lcid, + l2cap_result_code_text(result)); } } else if (p_lcb->conflict_lcid == lcid) { /* we must be in AVCT_CH_CFG state for the ch_lcid channel */ - log::verbose("avct_l2c_connect_cfm_cback ch_state: {}, conflict_lcid:0x{:x}", p_lcb->ch_state, + log::verbose("ch_state:{} conflict_lcid:0x{:04x}", avct_ch_state_text(p_lcb->ch_state), p_lcb->conflict_lcid); if (result == tL2CAP_CONN::L2CAP_CONN_OK) { /* just in case the peer also accepts our connection - Send L2CAP * disconnect req */ if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { - log::warn("Unable to send L2CAP disconnect request peer:{} cid:{}", p_lcb->peer_addr, - lcid); + log::warn("Unable to send L2CAP disconnect request peer:{} cid:0x{:04x}", + p_lcb->peer_addr, lcid); } } p_lcb->conflict_lcid = 0; } - log::verbose("ch_state cnc: {}", p_lcb->ch_state); + log::verbose("ch_state:{}", avct_ch_state_text(p_lcb->ch_state)); } } @@ -250,22 +248,29 @@ void avct_l2c_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result) { * Returns void * ******************************************************************************/ -void avct_l2c_config_cfm_cback(uint16_t lcid, uint16_t /* initiator */, tL2CAP_CFG_INFO* p_cfg) { +void avct_l2c_config_cfm_cback(uint16_t lcid, uint16_t is_initiator_local, tL2CAP_CFG_INFO* p_cfg) { avct_l2c_config_ind_cback(lcid, p_cfg); - tAVCT_LCB* p_lcb; - /* look up lcb for this channel */ - p_lcb = avct_lcb_by_lcid(lcid); - if (p_lcb != NULL) { - log::verbose("avct_l2c_config_cfm_cback: 0x{:x}, ch_state: {},", lcid, p_lcb->ch_state); - /* if in correct state */ - if (p_lcb->ch_state == AVCT_CH_CFG) { - p_lcb->ch_state = AVCT_CH_OPEN; - avct_lcb_event(p_lcb, AVCT_LCB_LL_OPEN_EVT, NULL); - } - log::verbose("ch_state cfc: {}", p_lcb->ch_state); + tAVCT_LCB* p_lcb = avct_lcb_by_lcid(lcid); + if (p_lcb == nullptr) { + log::warn("Received config confirm for unknown peer lcid::0x{:04x} is_initiator_local:{}", lcid, + is_initiator_local); + return; + } + + /* if in correct state */ + if (p_lcb->ch_state == AVCT_CH_CFG) { + p_lcb->ch_state = AVCT_CH_OPEN; + avct_lcb_event(p_lcb, AVCT_LCB_LL_OPEN_EVT, NULL); + } else { + log::warn( + "Received config confirm in wrong state lcid:0x{:04x} ch_state:{} " + "is_initiator_local:{}", + lcid, avct_ch_state_text(p_lcb->ch_state), is_initiator_local); } + log::verbose("ch_state lcid:0x{:04x} ch_state:{} is_initiator_local:{}", lcid, + avct_ch_state_text(p_lcb->ch_state), is_initiator_local); } /******************************************************************************* @@ -284,7 +289,8 @@ void avct_l2c_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg) { /* look up lcb for this channel */ p_lcb = avct_lcb_by_lcid(lcid); if (p_lcb != NULL) { - log::verbose("avct_l2c_config_ind_cback: 0x{:x}, ch_state: {}", lcid, p_lcb->ch_state); + log::verbose("avct_l2c_config_ind_cback: 0x{:04x}, ch_state:{}", lcid, + avct_ch_state_text(p_lcb->ch_state)); /* store the mtu in tbl */ if (p_cfg->mtu_present) { p_lcb->peer_mtu = p_cfg->mtu; @@ -339,7 +345,7 @@ void avct_l2c_disconnect(uint16_t lcid, uint16_t result) { tAVCT_LCB_EVT avct_lcb_evt; avct_lcb_evt.result = res; avct_lcb_event(p_lcb, AVCT_LCB_LL_CLOSE_EVT, &avct_lcb_evt); - log::verbose("ch_state dc: {}", p_lcb->ch_state); + log::verbose("ch_state:{}", p_lcb->ch_state); } } @@ -377,15 +383,13 @@ void avct_l2c_congestion_ind_cback(uint16_t lcid, bool is_congested) { * ******************************************************************************/ void avct_l2c_data_ind_cback(uint16_t lcid, BT_HDR* p_buf) { - tAVCT_LCB* p_lcb; - - log::verbose("avct_l2c_data_ind_cback: 0x{:x}", lcid); + log::verbose("lcid: 0x{:02x}", lcid); /* look up lcb for this channel */ - p_lcb = avct_lcb_by_lcid(lcid); + tAVCT_LCB* p_lcb = avct_lcb_by_lcid(lcid); if (p_lcb != NULL) { avct_lcb_event(p_lcb, AVCT_LCB_LL_MSG_EVT, (tAVCT_LCB_EVT*)&p_buf); - } else /* prevent buffer leak */ - { + } else { + /* prevent buffer leak */ log::warn("ERROR -> avct_l2c_data_ind_cback drop buffer"); osi_free(p_buf); } diff --git a/system/stack/avct/avct_l2c_br.cc b/system/stack/avct/avct_l2c_br.cc index 4c5ccc503b..3f76e0b76f 100644 --- a/system/stack/avct/avct_l2c_br.cc +++ b/system/stack/avct/avct_l2c_br.cc @@ -24,30 +24,29 @@ * *****************************************************************************/ -#define LOG_TAG "avctp" - #include <bluetooth/log.h> -#include "avct_api.h" -#include "avct_int.h" #include "internal_include/bt_target.h" #include "osi/include/allocator.h" +#include "stack/avct/avct_int.h" +#include "stack/include/avct_api.h" #include "stack/include/bt_hdr.h" #include "stack/include/l2cap_interface.h" +#include "stack/include/l2cdefs.h" #include "types/raw_address.h" using namespace bluetooth; /* callback function declarations */ -void avct_l2c_br_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16_t psm, - uint8_t id); -void avct_l2c_br_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result); -void avct_l2c_br_config_cfm_cback(uint16_t lcid, uint16_t result, tL2CAP_CFG_INFO* p_cfg); -void avct_l2c_br_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg); -void avct_l2c_br_disconnect_ind_cback(uint16_t lcid, bool ack_needed); -void avct_l2c_br_congestion_ind_cback(uint16_t lcid, bool is_congested); -void avct_l2c_br_data_ind_cback(uint16_t lcid, BT_HDR* p_buf); -void avct_br_on_l2cap_error(uint16_t lcid, uint16_t result); +static void avct_l2c_br_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16_t psm, + uint8_t id); +static void avct_l2c_br_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result); +static void avct_l2c_br_config_cfm_cback(uint16_t lcid, uint16_t result, tL2CAP_CFG_INFO* p_cfg); +static void avct_l2c_br_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg); +static void avct_l2c_br_disconnect_ind_cback(uint16_t lcid, bool ack_needed); +static void avct_l2c_br_congestion_ind_cback(uint16_t lcid, bool is_congested); +static void avct_l2c_br_data_ind_cback(uint16_t lcid, BT_HDR* p_buf); +static void avct_br_on_l2cap_error(uint16_t lcid, uint16_t result); /* L2CAP callback function structure */ const tL2CAP_APPL_INFO avct_l2c_br_appl = {avct_l2c_br_connect_ind_cback, @@ -77,9 +76,8 @@ const tL2CAP_APPL_INFO avct_l2c_br_appl = {avct_l2c_br_connect_ind_cback, ******************************************************************************/ static bool avct_l2c_br_is_passive(tAVCT_BCB* p_bcb) { bool is_passive = false; - tAVCT_LCB* p_lcb; tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; - p_lcb = avct_lcb_by_bcb(p_bcb); + tAVCT_LCB* p_lcb = avct_lcb_by_bcb(p_bcb); int i; for (i = 0; i < AVCT_NUM_CONN; i++, p_ccb++) { @@ -106,12 +104,10 @@ static bool avct_l2c_br_is_passive(tAVCT_BCB* p_bcb) { ******************************************************************************/ void avct_l2c_br_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uint16_t /* psm */, uint8_t /* id */) { - tAVCT_LCB* p_lcb; tL2CAP_CONN result = tL2CAP_CONN::L2CAP_CONN_NO_RESOURCES; - tAVCT_BCB* p_bcb; - tL2CAP_ERTM_INFO ertm_info; + tAVCT_BCB* p_bcb{nullptr}; - p_lcb = avct_lcb_by_bd(bd_addr); + tAVCT_LCB* p_lcb = avct_lcb_by_bd(bd_addr); if (p_lcb != NULL) { /* control channel exists */ p_bcb = avct_bcb_by_lcb(p_lcb); @@ -136,26 +132,20 @@ void avct_l2c_br_connect_ind_cback(const RawAddress& bd_addr, uint16_t lcid, uin } } } - /* else no control channel yet, reject */ - - /* Set the FCR options: Browsing channel mandates ERTM */ - ertm_info.preferred_mode = L2CAP_FCR_ERTM_MODE; - - /* If we reject the connection, send DisconnectReq */ - if (result != tL2CAP_CONN::L2CAP_CONN_OK) { - log::verbose("Connection rejected to lcid:0x{:x}", lcid); - if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { - log::warn("Unable to send L2CAP disconnect request cid:{}", lcid); - } - } /* if result ok, proceed with connection */ if (result == tL2CAP_CONN::L2CAP_CONN_OK) { /* store LCID */ p_bcb->ch_lcid = lcid; - /* transition to configuration state */ p_bcb->ch_state = AVCT_CH_CFG; + } else { + /* else no control channel yet, reject */ + /* If we reject the connection, send DisconnectReq */ + log::verbose("Connection rejected to lcid:0x{:x}", lcid); + if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { + log::warn("Unable to send L2CAP disconnect request cid:0x{:04x}", lcid); + } } } @@ -174,7 +164,7 @@ void avct_br_on_l2cap_error(uint16_t lcid, uint16_t result) { p_bcb->ch_result = result; /* Send L2CAP disconnect req */ - avct_l2c_br_disconnect(lcid, 0); + avct_l2c_br_disconnect(lcid, 0 /* is_ack_needed */); } /******************************************************************************* @@ -188,24 +178,23 @@ void avct_br_on_l2cap_error(uint16_t lcid, uint16_t result) { * ******************************************************************************/ void avct_l2c_br_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result) { - tAVCT_BCB* p_bcb; - /* look up bcb for this channel */ - p_bcb = avct_bcb_by_lcid(lcid); - - if (p_bcb == NULL) { + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if (p_bcb == nullptr) { + log::warn( + "Received browse connect confirm callback without a browse control channel " + "lcid:0x{:02x} status:{}", + lcid, l2cap_result_code_text(result)); return; } - /* if in correct state */ if (p_bcb->ch_state == AVCT_CH_CONN) { /* if result successful */ if (result == tL2CAP_CONN::L2CAP_CONN_OK) { /* set channel state */ p_bcb->ch_state = AVCT_CH_CFG; - } - /* else failure */ - else { - log::error("Invoked with non OK status"); + } else { + log::error("Invoked with non OK lcid:0x{:04x} state:{} status:{}", lcid, + avct_ch_state_text(p_bcb->ch_state), l2cap_result_code_text(result)); } } else if (p_bcb->conflict_lcid == lcid) { /* we must be in AVCT_CH_CFG state for the ch_lcid channel */ @@ -214,8 +203,12 @@ void avct_l2c_br_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result) { * disconnect req */ log::verbose("Disconnect conflict_lcid:0x{:x}", p_bcb->conflict_lcid); if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { - log::warn("Unable to send L2CAP disconnect request peer:{} cid:{}", p_bcb->peer_addr, lcid); + log::warn("Unable to send L2CAP disconnect request peer:{} lcid:0x{:04x}", p_bcb->peer_addr, + lcid); } + } else { + log::error("Invoked with failure peer:{} lcid:0x{:04x} conflict_lcid:0x{:04x} status:{}", + p_bcb->peer_addr, lcid, p_bcb->conflict_lcid, l2cap_result_code_text(result)); } p_bcb->conflict_lcid = 0; } @@ -231,19 +224,21 @@ void avct_l2c_br_connect_cfm_cback(uint16_t lcid, tL2CAP_CONN result) { * Returns void * ******************************************************************************/ -void avct_l2c_br_config_cfm_cback(uint16_t lcid, uint16_t /* initiator */, tL2CAP_CFG_INFO* p_cfg) { +void avct_l2c_br_config_cfm_cback(uint16_t lcid, uint16_t initiator, tL2CAP_CFG_INFO* p_cfg) { + log::verbose("lcid:0x{:04x} initiator:{}", lcid, initiator); avct_l2c_br_config_ind_cback(lcid, p_cfg); - tAVCT_BCB* p_lcb; - - /* look up lcb for this channel */ - p_lcb = avct_bcb_by_lcid(lcid); - if ((p_lcb == NULL) || (p_lcb->ch_state != AVCT_CH_CFG)) { + /* look up bcb for this channel */ + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if ((p_bcb == NULL) || (p_bcb->ch_state != AVCT_CH_CFG)) { + log::warn( + "Got config confirm callback with no browse channel or browse channel not in " + "configuration state"); return; } - p_lcb->ch_state = AVCT_CH_OPEN; - avct_bcb_event(p_lcb, AVCT_LCB_LL_OPEN_EVT, NULL); + p_bcb->ch_state = AVCT_CH_OPEN; + avct_bcb_event(p_bcb, AVCT_LCB_LL_OPEN_EVT, NULL); } /******************************************************************************* @@ -257,26 +252,27 @@ void avct_l2c_br_config_cfm_cback(uint16_t lcid, uint16_t /* initiator */, tL2CA * ******************************************************************************/ void avct_l2c_br_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg) { - tAVCT_BCB* p_lcb; - uint16_t max_mtu = BT_DEFAULT_BUFFER_SIZE - L2CAP_MIN_OFFSET - BT_HDR_SIZE; + log::verbose("lcid:0x{:04x}", lcid); + const uint16_t max_mtu = BT_DEFAULT_BUFFER_SIZE - L2CAP_MIN_OFFSET - BT_HDR_SIZE; - /* look up lcb for this channel */ - p_lcb = avct_bcb_by_lcid(lcid); - if (p_lcb == NULL) { + /* look up bcb for this channel */ + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if (p_bcb == NULL) { + log::warn("Unable to find browse control block lcid:0x{:02x}", lcid); return; } /* store the mtu in tbl */ - p_lcb->peer_mtu = L2CAP_DEFAULT_MTU; + p_bcb->peer_mtu = L2CAP_DEFAULT_MTU; if (p_cfg->mtu_present) { - p_lcb->peer_mtu = p_cfg->mtu; + p_bcb->peer_mtu = p_cfg->mtu; } - if (p_lcb->peer_mtu > max_mtu) { - p_lcb->peer_mtu = max_mtu; + if (p_bcb->peer_mtu > max_mtu) { + p_bcb->peer_mtu = max_mtu; } - log::verbose("peer_mtu:{} use:{}", p_lcb->peer_mtu, max_mtu); + log::verbose("peer_mtu:{} max_mtu:{}", p_bcb->peer_mtu, max_mtu); } /******************************************************************************* @@ -290,41 +286,37 @@ void avct_l2c_br_config_ind_cback(uint16_t lcid, tL2CAP_CFG_INFO* p_cfg) { * ******************************************************************************/ void avct_l2c_br_disconnect_ind_cback(uint16_t lcid, bool /* ack_needed */) { - tAVCT_BCB* p_lcb; uint16_t result = AVCT_RESULT_FAIL; - /* look up lcb for this channel */ - p_lcb = avct_bcb_by_lcid(lcid); - if (p_lcb == NULL) { + /* look up bcb for this channel */ + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if (p_bcb == NULL) { return; } tAVCT_LCB_EVT avct_lcb_evt; avct_lcb_evt.result = result; - avct_bcb_event(p_lcb, AVCT_LCB_LL_CLOSE_EVT, &avct_lcb_evt); + avct_bcb_event(p_bcb, AVCT_LCB_LL_CLOSE_EVT, &avct_lcb_evt); } void avct_l2c_br_disconnect(uint16_t lcid, uint16_t result) { if (!stack::l2cap::get_interface().L2CA_DisconnectReq(lcid)) { - log::warn("Unable to send L2CAP disconnect request cid:{}", lcid); + log::warn("Unable to send L2CAP disconnect request cid:0x{:04x}", lcid); } - tAVCT_BCB* p_lcb; - uint16_t res; - - /* look up lcb for this channel */ - p_lcb = avct_bcb_by_lcid(lcid); - if (p_lcb == NULL) { + /* look up bcb for this channel */ + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if (p_bcb == NULL) { return; } /* result value may be previously stored */ - res = (p_lcb->ch_result != 0) ? p_lcb->ch_result : result; - p_lcb->ch_result = 0; + uint16_t res = (p_bcb->ch_result != 0) ? p_bcb->ch_result : result; + p_bcb->ch_result = 0; tAVCT_LCB_EVT avct_lcb_evt; avct_lcb_evt.result = res; - avct_bcb_event(p_lcb, AVCT_LCB_LL_CLOSE_EVT, &avct_lcb_evt); + avct_bcb_event(p_bcb, AVCT_LCB_LL_CLOSE_EVT, &avct_lcb_evt); } /******************************************************************************* @@ -338,17 +330,15 @@ void avct_l2c_br_disconnect(uint16_t lcid, uint16_t result) { * ******************************************************************************/ void avct_l2c_br_congestion_ind_cback(uint16_t lcid, bool is_congested) { - tAVCT_BCB* p_lcb; - - /* look up lcb for this channel */ - p_lcb = avct_bcb_by_lcid(lcid); - if (p_lcb == NULL) { + /* look up bcb for this channel */ + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if (p_bcb == NULL) { return; } tAVCT_LCB_EVT avct_lcb_evt; avct_lcb_evt.cong = is_congested; - avct_bcb_event(p_lcb, AVCT_LCB_LL_CONG_EVT, &avct_lcb_evt); + avct_bcb_event(p_bcb, AVCT_LCB_LL_CONG_EVT, &avct_lcb_evt); } /******************************************************************************* @@ -362,17 +352,15 @@ void avct_l2c_br_congestion_ind_cback(uint16_t lcid, bool is_congested) { * ******************************************************************************/ void avct_l2c_br_data_ind_cback(uint16_t lcid, BT_HDR* p_buf) { - tAVCT_BCB* p_lcb; - tAVCT_LCB_EVT evt_data; - - /* look up lcb for this channel */ - p_lcb = avct_bcb_by_lcid(lcid); - if (p_lcb == NULL) { + /* look up bcb for this channel */ + tAVCT_BCB* p_bcb = avct_bcb_by_lcid(lcid); + if (p_bcb == NULL) { /* prevent buffer leak */ osi_free(p_buf); return; } + tAVCT_LCB_EVT evt_data{}; evt_data.p_buf = p_buf; - avct_bcb_event(p_lcb, AVCT_LCB_LL_MSG_EVT, &evt_data); + avct_bcb_event(p_bcb, AVCT_LCB_LL_MSG_EVT, &evt_data); } diff --git a/system/stack/avct/avct_lcb.cc b/system/stack/avct/avct_lcb.cc index 01673e0854..26d4a73160 100644 --- a/system/stack/avct/avct_lcb.cc +++ b/system/stack/avct/avct_lcb.cc @@ -23,17 +23,15 @@ * ******************************************************************************/ -#define LOG_TAG "avctp" - #include <bluetooth/log.h> #include <string.h> #include "avct_api.h" #include "avct_int.h" #include "device/include/device_iot_config.h" +#include "include/macros.h" #include "internal_include/bt_target.h" #include "osi/include/allocator.h" -#include "osi/include/osi.h" #include "types/raw_address.h" using namespace bluetooth; @@ -54,6 +52,16 @@ const char* const avct_lcb_evt_str[] = {"UL_BIND_EVT", "UL_UNBIND_EVT", "UL_MS /* lcb state machine states */ enum { AVCT_LCB_IDLE_ST, AVCT_LCB_OPENING_ST, AVCT_LCB_OPEN_ST, AVCT_LCB_CLOSING_ST }; +std::string avct_sm_state_text(const int& state) { + switch (state) { + CASE_RETURN_STRING(AVCT_LCB_IDLE_ST); + CASE_RETURN_STRING(AVCT_LCB_OPENING_ST); + CASE_RETURN_STRING(AVCT_LCB_OPEN_ST); + CASE_RETURN_STRING(AVCT_LCB_CLOSING_ST); + } + RETURN_UNKNOWN_TYPE_STRING(int, state); +} + /* state machine action enumeration list */ enum { AVCT_LCB_CHNL_OPEN, @@ -162,7 +170,7 @@ void avct_lcb_event(tAVCT_LCB* p_lcb, uint8_t event, tAVCT_LCB_EVT* p_data) { uint8_t action; int i; - log::verbose("LCB lcb={} event={} state={}", p_lcb->allocated, avct_lcb_evt_str[event], + log::verbose("LCB lcb_allocated={} event={} state={}", p_lcb->allocated, avct_lcb_evt_str[event], avct_lcb_st_str[p_lcb->state]); /* look up the state table for the current state */ @@ -197,22 +205,18 @@ void avct_lcb_event(tAVCT_LCB* p_lcb, uint8_t event, tAVCT_LCB_EVT* p_data) { * ******************************************************************************/ void avct_bcb_event(tAVCT_BCB* p_bcb, uint8_t event, tAVCT_LCB_EVT* p_data) { - tAVCT_LCB_ST_TBL state_table; - uint8_t action; - int i; - - log::verbose("BCB lcb={} event={} state={}", p_bcb->allocated, avct_lcb_evt_str[event], - avct_lcb_st_str[p_bcb->state]); + log::info("BCB bcb_allocated={} event={} state={}", p_bcb->allocated, avct_lcb_evt_str[event], + avct_lcb_st_str[p_bcb->state]); /* look up the state table for the current state */ - state_table = avct_lcb_st_tbl[p_bcb->state]; + tAVCT_LCB_ST_TBL state_table = avct_lcb_st_tbl[p_bcb->state]; /* set next state */ p_bcb->state = state_table[event][AVCT_LCB_NEXT_STATE]; /* execute action functions */ - for (i = 0; i < AVCT_LCB_ACTIONS; i++) { - action = state_table[event][i]; + for (int i = 0; i < AVCT_LCB_ACTIONS; i++) { + uint8_t action = state_table[event][i]; if (action != AVCT_LCB_IGNORE) { (*avct_bcb_action[action])(p_bcb, p_data); } else { @@ -246,7 +250,7 @@ tAVCT_LCB* avct_lcb_by_bd(const RawAddress& bd_addr) { /* if no lcb found */ p_lcb = NULL; - log::verbose("No lcb for addr {}", bd_addr); + log::verbose("No lcb for addr:{}", bd_addr); } return p_lcb; } @@ -269,7 +273,7 @@ tAVCT_LCB* avct_lcb_alloc(const RawAddress& bd_addr) { if (!p_lcb->allocated) { p_lcb->allocated = (uint8_t)(i + 1); p_lcb->peer_addr = bd_addr; - log::verbose("avct_lcb_alloc {}", p_lcb->allocated); + log::verbose("lcb_allocated:{}", p_lcb->allocated); p_lcb->tx_q = fixed_queue_new(SIZE_MAX); p_lcb->peer_mtu = L2CAP_LE_MIN_MTU; break; @@ -295,14 +299,14 @@ tAVCT_LCB* avct_lcb_alloc(const RawAddress& bd_addr) { * ******************************************************************************/ void avct_lcb_dealloc(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* /* p_data */) { - log::verbose("allocated: {}", p_lcb->allocated); + log::verbose("lcb_allocated:{}", p_lcb->allocated); // Check if the LCB is still referenced tAVCT_CCB* p_ccb = &avct_cb.ccb[0]; for (size_t i = 0; i < AVCT_NUM_CONN; i++, p_ccb++) { if (p_ccb->allocated && p_ccb->p_lcb == p_lcb) { - log::verbose("LCB in use; lcb index: {}", i); + log::verbose("LCB in use; lcb index:{}", i); return; } } @@ -382,8 +386,7 @@ bool avct_lcb_last_ccb(tAVCT_LCB* p_lcb, tAVCT_CCB* p_ccb_last) { log::warn("avct_lcb_last_ccb"); for (i = 0; i < AVCT_NUM_CONN; i++, p_ccb++) { - log::warn("{:x}: aloc:{}, lcb:0x{}/0x{}, ccb:0x{}/0x{}", i, p_ccb->allocated, - fmt::ptr(p_ccb->p_lcb), fmt::ptr(p_lcb), fmt::ptr(p_ccb), fmt::ptr(p_ccb_last)); + log::warn("index:{} allocated:{}, ", i, p_ccb->allocated); if (p_ccb->allocated && (p_ccb->p_lcb == p_lcb) && (p_ccb != p_ccb_last)) { return false; } diff --git a/system/stack/avct/avct_lcb_act.cc b/system/stack/avct/avct_lcb_act.cc index e06bf57634..986bce7d34 100644 --- a/system/stack/avct/avct_lcb_act.cc +++ b/system/stack/avct/avct_lcb_act.cc @@ -33,7 +33,9 @@ #include "internal_include/bt_target.h" #include "osi/include/allocator.h" #include "stack/avct/avct_defs.h" +#include "stack/include/avct_api.h" #include "stack/include/bt_hdr.h" +#include "stack/include/bt_psm_types.h" #include "stack/include/bt_types.h" #include "stack/include/l2cap_interface.h" @@ -190,10 +192,10 @@ void avct_lcb_chnl_open(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* /* p_data */) { p_lcb->ch_state = AVCT_CH_CONN; if (com::android::bluetooth::flags::use_encrypt_req_for_av()) { p_lcb->ch_lcid = stack::l2cap::get_interface().L2CA_ConnectReqWithSecurity( - AVCT_PSM, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE | BTA_SEC_ENCRYPT); + BT_PSM_AVCTP, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE | BTA_SEC_ENCRYPT); } else { p_lcb->ch_lcid = stack::l2cap::get_interface().L2CA_ConnectReqWithSecurity( - AVCT_PSM, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE); + BT_PSM_AVCTP, p_lcb->peer_addr, BTA_SEC_AUTHENTICATE); } if (p_lcb->ch_lcid == 0) { /* if connect req failed, send ourselves close event */ @@ -239,7 +241,7 @@ void avct_lcb_open_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { bool is_originater = false; for (i = 0; i < AVCT_NUM_CONN; i++, p_ccb++) { - if (p_ccb->allocated && (p_ccb->p_lcb == p_lcb) && p_ccb->cc.role == AVCT_INT) { + if (p_ccb->allocated && (p_ccb->p_lcb == p_lcb) && p_ccb->cc.role == AVCT_ROLE_INITIATOR) { log::verbose("find int handle {}", i); is_originater = true; } @@ -250,16 +252,16 @@ void avct_lcb_open_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { /* if ccb allocated and */ /** M: to avoid avctp collision, make sure the collision can be checked @{ */ - log::verbose("{} ccb to lcb, alloc {}, lcb {}, role {}, pid 0x{:x}", i, p_ccb->allocated, - fmt::ptr(p_ccb->p_lcb), p_ccb->cc.role, p_ccb->cc.pid); + log::verbose("{} ccb to lcb, alloc {}, role {}, pid 0x{:04x}", i, p_ccb->allocated, + avct_role_text(p_ccb->cc.role), p_ccb->cc.pid); if (p_ccb->allocated && (p_ccb->p_lcb == p_lcb)) { /* if bound to this lcb send connect confirm event */ - if (p_ccb->cc.role == AVCT_INT) { + if (p_ccb->cc.role == AVCT_ROLE_INITIATOR) { /** @} */ bind = true; if (!stack::l2cap::get_interface().L2CA_SetTxPriority(p_lcb->ch_lcid, L2CAP_CHNL_PRIORITY_HIGH)) { - log::warn("Unable to set L2CAP transmit high priority peer:{} cid:{}", + log::warn("Unable to set L2CAP transmit high priority peer:{} lcid:0x{:04x}", p_ccb->p_lcb->peer_addr, p_lcb->ch_lcid); } p_ccb->cc.p_ctrl_cback(avct_ccb_to_idx(p_ccb), AVCT_CONNECT_CFM_EVT, 0, @@ -269,7 +271,7 @@ void avct_lcb_open_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { */ /** M: to avoid avctp collision, make sure the collision can be checked @{ */ - else if ((p_ccb->cc.role == AVCT_ACP) && avct_lcb_has_pid(p_lcb, p_ccb->cc.pid)) { + else if ((p_ccb->cc.role == AVCT_ROLE_ACCEPTOR) && avct_lcb_has_pid(p_lcb, p_ccb->cc.pid)) { /* bind ccb to lcb and send connect ind event */ if (is_originater) { log::error("int exist, unbind acp handle:{}", i); @@ -279,7 +281,7 @@ void avct_lcb_open_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { p_ccb->p_lcb = p_lcb; if (!stack::l2cap::get_interface().L2CA_SetTxPriority(p_lcb->ch_lcid, L2CAP_CHNL_PRIORITY_HIGH)) { - log::warn("Unable to set L2CAP transmit high priority peer:{} cid:{}", + log::warn("Unable to set L2CAP transmit high priority peer:{} lcid:0x{:04x}", p_ccb->p_lcb->peer_addr, p_lcb->ch_lcid); } p_ccb->cc.p_ctrl_cback(avct_ccb_to_idx(p_ccb), AVCT_CONNECT_IND_EVT, 0, @@ -297,7 +299,7 @@ void avct_lcb_open_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { bind = true; if (!stack::l2cap::get_interface().L2CA_SetTxPriority(p_lcb->ch_lcid, L2CAP_CHNL_PRIORITY_HIGH)) { - log::warn("Unable to set L2CAP transmit high priority peer:{} cid:{}", + log::warn("Unable to set L2CAP transmit high priority peer:{} lcid:0x{:04x}", p_ccb->p_lcb->peer_addr, p_lcb->ch_lcid); } p_ccb->cc.p_ctrl_cback(avct_ccb_to_idx(p_ccb), AVCT_CONNECT_CFM_EVT, 0, @@ -305,14 +307,14 @@ void avct_lcb_open_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { } /* if unbound acceptor and lcb doesn't already have a ccb for this PID */ - else if ((p_ccb->p_lcb == NULL) && (p_ccb->cc.role == AVCT_ACP) && + else if ((p_ccb->p_lcb == NULL) && (p_ccb->cc.role == AVCT_ROLE_ACCEPTOR) && (avct_lcb_has_pid(p_lcb, p_ccb->cc.pid) == NULL)) { /* bind ccb to lcb and send connect ind event */ bind = true; p_ccb->p_lcb = p_lcb; if (!stack::l2cap::get_interface().L2CA_SetTxPriority(p_lcb->ch_lcid, L2CAP_CHNL_PRIORITY_HIGH)) { - log::warn("Unable to set L2CAP transmit high priority peer:{} cid:{}", + log::warn("Unable to set L2CAP transmit high priority peer:{} lcid:0x{:04x}", p_ccb->p_lcb->peer_addr, p_lcb->ch_lcid); } p_ccb->cc.p_ctrl_cback(avct_ccb_to_idx(p_ccb), AVCT_CONNECT_IND_EVT, 0, @@ -369,7 +371,7 @@ void avct_lcb_close_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* /* p_data */) { for (i = 0; i < AVCT_NUM_CONN; i++, p_ccb++) { if (p_ccb->allocated && (p_ccb->p_lcb == p_lcb)) { - if (p_ccb->cc.role == AVCT_INT) { + if (p_ccb->cc.role == AVCT_ROLE_INITIATOR) { avct_ccb_dealloc(p_ccb, AVCT_DISCONNECT_IND_EVT, 0, &p_lcb->peer_addr); } else { p_ccb->p_lcb = NULL; @@ -406,7 +408,7 @@ void avct_lcb_close_cfm(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { event = AVCT_DISCONNECT_IND_EVT; } - if (p_ccb->cc.role == AVCT_INT) { + if (p_ccb->cc.role == AVCT_ROLE_INITIATOR) { avct_ccb_dealloc(p_ccb, event, p_data->result, &p_lcb->peer_addr); } else { p_ccb->p_lcb = NULL; @@ -714,7 +716,7 @@ void avct_lcb_msg_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { } /* PID not found; drop message */ - log::warn("No ccb for PID={:x}", pid); + log::warn("No ccb for PID=0x{:04x}", pid); osi_free_and_reset((void**)&p_data->p_buf); /* if command send reject */ @@ -727,7 +729,7 @@ void avct_lcb_msg_ind(tAVCT_LCB* p_lcb, tAVCT_LCB_EVT* p_data) { UINT16_TO_BE_STREAM(p, pid); if (stack::l2cap::get_interface().L2CA_DataWrite(p_lcb->ch_lcid, p_buf) != tL2CAP_DW_RESULT::SUCCESS) { - log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", p_lcb->peer_addr, + log::warn("Unable to write L2CAP data peer:{} lcid:0x{:04x} len:{}", p_lcb->peer_addr, p_lcb->ch_lcid, p_buf->len); } } diff --git a/system/stack/avrc/avrc_api.cc b/system/stack/avrc/avrc_api.cc index 2acddeb5e0..a2d75c218a 100644 --- a/system/stack/avrc/avrc_api.cc +++ b/system/stack/avrc/avrc_api.cc @@ -1062,16 +1062,15 @@ uint16_t AVRC_GetProfileVersion() { * *****************************************************************************/ uint16_t AVRC_Open(uint8_t* p_handle, tAVRC_CONN_CB* p_ccb, const RawAddress& peer_addr) { - uint16_t status; - tAVCT_CC cc; - - cc.p_ctrl_cback = avrc_ctrl_cback; /* Control callback */ - cc.p_msg_cback = avrc_msg_cback; /* Message callback */ - cc.pid = UUID_SERVCLASS_AV_REMOTE_CONTROL; /* Profile ID */ - cc.role = p_ccb->conn; /* Initiator/acceptor role */ - cc.control = p_ccb->control; /* Control role (Control/Target) */ - - status = AVCT_CreateConn(p_handle, &cc, peer_addr); + tAVCT_CC cc = { + .p_ctrl_cback = avrc_ctrl_cback, /* Control callback */ + .p_msg_cback = avrc_msg_cback, /* Message callback */ + .pid = UUID_SERVCLASS_AV_REMOTE_CONTROL, /* Profile ID */ + .role = p_ccb->conn, /* Initiator/acceptor role */ + .control = p_ccb->control, /* Control role (Control/Target) */ + }; + + uint16_t status = AVCT_CreateConn(p_handle, &cc, peer_addr); if (status == AVCT_SUCCESS) { avrc_cb.ccb[*p_handle] = *p_ccb; memset(&avrc_cb.ccb_int[*p_handle], 0, sizeof(tAVRC_CONN_INT_CB)); @@ -1080,7 +1079,8 @@ uint16_t AVRC_Open(uint8_t* p_handle, tAVRC_CONN_CB* p_ccb, const RawAddress& pe avrc_cb.ccb_int[*p_handle].tle = alarm_new("avrcp.commandTimer"); avrc_cb.ccb_int[*p_handle].cmd_q = fixed_queue_new(SIZE_MAX); } - log::verbose("role: {}, control:{} status:{}, handle:{}", cc.role, cc.control, status, *p_handle); + log::verbose("role: {}, control:0x{:x} status:{}, handle:{}", avct_role_text(cc.role), cc.control, + status, *p_handle); return status; } @@ -1123,7 +1123,7 @@ uint16_t AVRC_Close(uint8_t handle) { * the connection. * *****************************************************************************/ -uint16_t AVRC_OpenBrowse(uint8_t handle, uint8_t conn_role) { +uint16_t AVRC_OpenBrowse(uint8_t handle, tAVCT_ROLE conn_role) { return AVCT_CreateBrowse(handle, conn_role); } diff --git a/system/stack/avrc/avrc_sdp.cc b/system/stack/avrc/avrc_sdp.cc index d4ed5bb421..61ba721b5f 100644 --- a/system/stack/avrc/avrc_sdp.cc +++ b/system/stack/avrc/avrc_sdp.cc @@ -28,6 +28,7 @@ #include "avrc_api.h" #include "avrc_int.h" +#include "stack/include/bt_psm_types.h" #include "stack/include/bt_types.h" #include "stack/include/bt_uuid16.h" #include "stack/include/sdp_api.h" @@ -64,16 +65,19 @@ static uint16_t a2dp_attr_list_sdp[] = { * Returns Nothing. * *****************************************************************************/ -static void avrc_sdp_cback(const RawAddress& /* bd_addr */, tSDP_STATUS status) { - log::verbose("status: {}", status); +static void avrc_sdp_cback(const RawAddress& bd_addr, tSDP_STATUS status) { + log::verbose("peer:{} status: {}", bd_addr, status); /* reset service_uuid, so can start another find service */ avrc_cb.service_uuid = 0; /* return info from sdp record in app callback function */ - avrc_cb.find_cback.Run(status); - - return; + if (!avrc_cb.find_cback.is_null()) { + avrc_cb.find_cback.Run(status); + } else { + log::warn("Received SDP callback with NULL callback peer:{} status:{}", bd_addr, + sdp_status_text(status)); + } } /****************************************************************************** @@ -246,7 +250,7 @@ uint16_t AVRC_AddRecord(uint16_t service_uuid, const char* p_service_name, tSDP_PROTOCOL_ELEM avrc_proto_desc_list[AVRC_NUM_PROTO_ELEMS]; avrc_proto_desc_list[0].num_params = 1; avrc_proto_desc_list[0].protocol_uuid = UUID_PROTOCOL_L2CAP; - avrc_proto_desc_list[0].params[0] = AVCT_PSM; + avrc_proto_desc_list[0].params[0] = BT_PSM_AVCTP; avrc_proto_desc_list[0].params[1] = 0; for (index = 1; index < AVRC_NUM_PROTO_ELEMS; index++) { avrc_proto_desc_list[index].num_params = 1; @@ -270,7 +274,7 @@ uint16_t AVRC_AddRecord(uint16_t service_uuid, const char* p_service_name, avrc_add_proto_desc_lists[i].num_elems = 2; avrc_add_proto_desc_lists[i].list_elem[0].num_params = 1; avrc_add_proto_desc_lists[i].list_elem[0].protocol_uuid = UUID_PROTOCOL_L2CAP; - avrc_add_proto_desc_lists[i].list_elem[0].params[0] = AVCT_BR_PSM; + avrc_add_proto_desc_lists[i].list_elem[0].params[0] = BT_PSM_AVCTP_BROWSE; avrc_add_proto_desc_lists[i].list_elem[0].params[1] = 0; avrc_add_proto_desc_lists[i].list_elem[1].num_params = 1; avrc_add_proto_desc_lists[i].list_elem[1].protocol_uuid = UUID_PROTOCOL_AVCTP; diff --git a/system/stack/fuzzers/avrc_fuzzer.cc b/system/stack/fuzzers/avrc_fuzzer.cc index 1592b707fd..be0c80f32c 100644 --- a/system/stack/fuzzers/avrc_fuzzer.cc +++ b/system/stack/fuzzers/avrc_fuzzer.cc @@ -20,18 +20,17 @@ #include <cstdint> #include <functional> -#include <optional> #include <vector> #include "osi/include/allocator.h" #include "stack/include/avct_api.h" #include "stack/include/avrc_api.h" +#include "stack/include/bt_psm_types.h" #include "test/fake/fake_osi.h" #include "test/mock/mock_btif_config.h" #include "test/mock/mock_stack_acl.h" #include "test/mock/mock_stack_btm_dev.h" #include "test/mock/mock_stack_l2cap_api.h" -#include "test/mock/mock_stack_l2cap_ble.h" #include "types/bluetooth/uuid.h" // TODO(b/369381361) Enfore -Wmissing-prototypes @@ -81,11 +80,11 @@ public: [](uint16_t psm, const tL2CAP_APPL_INFO& p_cb_info, bool enable_snoop, tL2CAP_ERTM_INFO* p_ertm_info, uint16_t my_mtu, uint16_t required_remote_mtu, uint16_t sec_level) { - log::assert_that(psm == AVCT_PSM || psm == AVCT_BR_PSM, - "assert failed: psm == AVCT_PSM || psm == AVCT_BR_PSM"); - if (psm == AVCT_PSM) { + log::assert_that(psm == BT_PSM_AVCTP || psm == BT_PSM_AVCTP_BROWSE, + "assert failed: psm == BT_PSM_AVCTP || psm == BT_PSM_AVCTP_BROWSE"); + if (psm == BT_PSM_AVDTP) { avct_appl = p_cb_info; - } else if (psm == AVCT_BR_PSM) { + } else if (psm == BT_PSM_AVCTP_BROWSE) { avct_br_appl = p_cb_info; } return psm; @@ -186,7 +185,7 @@ static void Fuzz(const uint8_t* data, size_t size) { tAVRC_CONN_CB ccb = { .ctrl_cback = base::Bind(ctrl_cb), .msg_cback = base::Bind(msg_cb), - .conn = (uint8_t)(is_initiator ? AVCT_INT : AVCT_ACP), + .conn = (is_initiator ? AVCT_ROLE_INITIATOR : AVCT_ROLE_ACCEPTOR), .control = (uint8_t)(is_controller ? AVCT_CONTROL : AVCT_TARGET), }; diff --git a/system/stack/include/avct_api.h b/system/stack/include/avct_api.h index 0ac2399b93..0cf58171ba 100644 --- a/system/stack/include/avct_api.h +++ b/system/stack/include/avct_api.h @@ -26,8 +26,9 @@ #define AVCT_API_H #include <cstdint> +#include <string> -#include "internal_include/bt_target.h" +#include "include/macros.h" #include "stack/include/bt_hdr.h" #include "types/raw_address.h" @@ -42,10 +43,6 @@ #define AVCT_PID_IN_USE 3 /* PID already in use */ #define AVCT_NOT_OPEN 4 /* Connection not open */ -/* PSM for AVCT. */ -#define AVCT_PSM 0x0017 -#define AVCT_BR_PSM 0x001B - /* Protocol revision numbers */ #define AVCT_REV_1_0 0x0100 #define AVCT_REV_1_2 0x0102 @@ -69,8 +66,18 @@ #define AVCT_BROWSE_OFFSET 17 /* the default offset for browsing channel */ /* Connection role. */ -#define AVCT_INT 0 /* Initiator connection */ -#define AVCT_ACP 1 /* Acceptor connection */ +typedef enum { + AVCT_ROLE_INITIATOR = 0, /* Initiator connection */ + AVCT_ROLE_ACCEPTOR = 1, /* Acceptor connection */ +} tAVCT_ROLE; + +inline std::string avct_role_text(const tAVCT_ROLE& role) { + switch (role) { + CASE_RETURN_TEXT(AVCT_ROLE_INITIATOR); + CASE_RETURN_TEXT(AVCT_ROLE_ACCEPTOR); + } + RETURN_UNKNOWN_TYPE_STRING(tAVCT_ROLE, role); +} /* Control role. */ #define AVCT_TARGET 1 /* target */ @@ -116,7 +123,7 @@ typedef struct { tAVCT_CTRL_CBACK* p_ctrl_cback; /* Control callback */ tAVCT_MSG_CBACK* p_msg_cback; /* Message callback */ uint16_t pid; /* Profile ID */ - uint8_t role; /* Initiator/acceptor role */ + tAVCT_ROLE role; /* Initiator/acceptor role */ uint8_t control; /* Control role (Control/Target) */ } tAVCT_CC; @@ -207,7 +214,7 @@ uint16_t AVCT_RemoveConn(uint8_t handle); * Returns AVCT_SUCCESS if successful, otherwise error. * ******************************************************************************/ -uint16_t AVCT_CreateBrowse(uint8_t handle, uint8_t role); +uint16_t AVCT_CreateBrowse(uint8_t handle, tAVCT_ROLE role); /******************************************************************************* * diff --git a/system/stack/include/avrc_api.h b/system/stack/include/avrc_api.h index 918bdfb436..589ddb5021 100644 --- a/system/stack/include/avrc_api.h +++ b/system/stack/include/avrc_api.h @@ -65,12 +65,6 @@ /* If conflict, allow the other side to succeed */ #define AVRC_CT_PASSIVE 4 -/* Connection role */ -/* initiator */ -#define AVRC_CONN_INT AVCT_INT -/* Acceptor */ -#define AVRC_CONN_ACP AVCT_ACP - /* AVRC CTRL events */ /* AVRC_OPEN_IND_EVT event is sent when the connection is successfully opened. * This eventis sent in response to an AVRC_Open(). */ @@ -201,7 +195,7 @@ typedef struct { * attribute filter * to be ATTR_ID_SERVICE_CLASS_ID_LIST, * ATTR_ID_BT_PROFILE_DESC_LIST, - * ATTR_ID_SUPPORTED_FEATURES, ATTR_ID_SERVICE_NAME and + * ATTR_ID_SUPPORTED_FEATURES, ATTR_ID_SERVICE_NAME, * ATTR_ID_PROVIDER_NAME. * If not NULL, the input is taken as the filter. */ } tAVRC_SDP_DB_PARAMS; @@ -229,7 +223,7 @@ typedef struct { tAVRC_CTRL_CBACK ctrl_cback; /* application control callback */ tAVRC_MSG_CBACK msg_cback; /* application message callback */ uint32_t company_id; /* the company ID */ - uint8_t conn; /* Connection role (Initiator/acceptor) */ + tAVCT_ROLE conn; /* Connection role (Initiator/acceptor) */ uint8_t control; /* Control role (Control/Target) */ } tAVRC_CONN_CB; @@ -460,7 +454,7 @@ uint16_t AVRC_Close(uint8_t handle); * the connection. * *****************************************************************************/ -uint16_t AVRC_OpenBrowse(uint8_t handle, uint8_t conn_role); +uint16_t AVRC_OpenBrowse(uint8_t handle, tAVCT_ROLE conn_role); /****************************************************************************** * diff --git a/system/stack/include/bt_psm_types.h b/system/stack/include/bt_psm_types.h index a5bb3e47e4..3f0716110b 100644 --- a/system/stack/include/bt_psm_types.h +++ b/system/stack/include/bt_psm_types.h @@ -36,9 +36,9 @@ enum tBT_PSM : uint16_t { BT_PSM_UPNP = 0x0015, BT_PSM_AVCTP = 0x0017, BT_PSM_AVDTP = 0x0019, - BT_PSM_AVCTP_13 = 0x001B, /* Advanced Control - Browsing */ - BT_PSM_UDI_CP = 0x001D, /* Unrestricted Digital Information Profile C-Plane */ - BT_PSM_ATT = 0x001F, /* Attribute Protocol */ + BT_PSM_AVCTP_BROWSE = 0x001B, /* Advanced Control - Browsing */ + BT_PSM_UDI_CP = 0x001D, /* Unrestricted Digital Information Profile C-Plane */ + BT_PSM_ATT = 0x001F, /* Attribute Protocol */ BT_PSM_EATT = 0x0027, /* We will not allocate a PSM in the reserved range to 3rd party apps */ @@ -58,7 +58,7 @@ inline std::string bt_psm_text(const tBT_PSM& psm) { CASE_RETURN_STRING_HEX04(BT_PSM_UPNP); CASE_RETURN_STRING_HEX04(BT_PSM_AVCTP); CASE_RETURN_STRING_HEX04(BT_PSM_AVDTP); - CASE_RETURN_STRING_HEX04(BT_PSM_AVCTP_13); + CASE_RETURN_STRING_HEX04(BT_PSM_AVCTP_BROWSE); CASE_RETURN_STRING_HEX04(BT_PSM_UDI_CP); CASE_RETURN_STRING_HEX04(BT_PSM_ATT); CASE_RETURN_STRING_HEX04(BT_PSM_EATT); diff --git a/system/stack/test/stack_l2cap_test.cc b/system/stack/test/stack_l2cap_test.cc index d2d19bf39e..ffe78055bc 100644 --- a/system/stack/test/stack_l2cap_test.cc +++ b/system/stack/test/stack_l2cap_test.cc @@ -287,7 +287,7 @@ TEST_F(StackL2capTest, bt_psm_text) { {BT_PSM_UPNP, "BT_PSM_UPNP"}, {BT_PSM_AVCTP, "BT_PSM_AVCTP"}, {BT_PSM_AVDTP, "BT_PSM_AVDTP"}, - {BT_PSM_AVCTP_13, "BT_PSM_AVCTP_13"}, + {BT_PSM_AVCTP_BROWSE, "BT_PSM_AVCTP_BROWSE"}, {BT_PSM_UDI_CP, "BT_PSM_UDI_CP"}, {BT_PSM_ATT, "BT_PSM_ATT"}, {BT_PSM_EATT, "BT_PSM_EATT"}, diff --git a/system/test/mock/mock_stack_avct_api.cc b/system/test/mock/mock_stack_avct_api.cc index 42dae21043..a63348f30c 100644 --- a/system/test/mock/mock_stack_avct_api.cc +++ b/system/test/mock/mock_stack_avct_api.cc @@ -24,7 +24,7 @@ #include "test/common/mock_functions.h" #include "types/raw_address.h" -uint16_t AVCT_CreateBrowse(uint8_t /* handle */, uint8_t /* role */) { +uint16_t AVCT_CreateBrowse(uint8_t /* handle */, tAVCT_ROLE /* role */) { inc_func_call_count(__func__); return 0; } diff --git a/system/test/mock/mock_stack_avrc_api.cc b/system/test/mock/mock_stack_avrc_api.cc index 5409f75345..22b3f15f19 100644 --- a/system/test/mock/mock_stack_avrc_api.cc +++ b/system/test/mock/mock_stack_avrc_api.cc @@ -58,7 +58,7 @@ uint16_t AVRC_Open(uint8_t* /* p_handle */, tAVRC_CONN_CB* /* p_ccb */, inc_func_call_count(__func__); return 0; } -uint16_t AVRC_OpenBrowse(uint8_t /* handle */, uint8_t /* conn_role */) { +uint16_t AVRC_OpenBrowse(uint8_t /* handle */, tAVCT_ROLE /* conn_role */) { inc_func_call_count(__func__); return 0; } |