Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e4192ae2 authored by Kyunglyul Hyun's avatar Kyunglyul Hyun
Browse files

Invoke MTU Callbacks on Failure 

Previously, MTU configuration failures were not propagated
to the Java layer due to setting of `p_q_cmd` to NULL.

This change ensures that error callbacks are invoked
correctly when MTU configuration fails before a request is sent
by bypassing the NULL check.

It enables a test to confirm the behavior.

Fix: 307981748
Bug: 356550596
Bug: 350622873

Test: atest GattClientTest
Change-Id: Ib5083a579ca881be4cd0d02ec4e20d265e15cf46
parent 748fad9d

framework/tests/bumble/src/android/bluetooth/GattClientTest.java

+9 −20
Original line number Diff line number Diff line
@@ -49,10 +49,10 @@ import com.google.testing.junit.testparameterinjector.TestParameter; 
import com.google.testing.junit.testparameterinjector.TestParameterInjector;



import org.junit.Assume;

import org.junit.Ignore;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.AdditionalMatchers;

import org.mockito.InOrder;

import org.mockito.invocation.Invocation;
@@ -483,30 +483,19 @@ public class GattClientTest { 
    }



    @Test

    @Ignore("b/307981748: requestMTU should return a direct error")

    public void requestMtu_notConnected_isFalse() {

        advertiseWithBumble();



        BluetoothDevice device =

                mAdapter.getRemoteLeDevice(

                        Utils.BUMBLE_RANDOM_ADDRESS, BluetoothDevice.ADDRESS_TYPE_RANDOM);

        BluetoothGattCallback gattCallback = mock(BluetoothGattCallback.class);



        BluetoothGatt gatt = device.connectGatt(mContext, false, gattCallback);

        // Do not wait for connection state change callback and ask MTU directly

        assertThat(gatt.requestMtu(MTU_REQUESTED)).isFalse();

    }



    @Test

    @Ignore("b/307981748: requestMTU should return a direct error or a error on the callback")

    public void requestMtu_invalidParamer_isFalse() {

    @RequiresFlagsEnabled(Flags.FLAG_GATT_CALLBACK_ON_FAILURE)

    public void requestMtu_invalidParameter_isFalse() {

        BluetoothGattCallback gattCallback = mock(BluetoothGattCallback.class);

        BluetoothGatt gatt = connectGattAndWaitConnection(gattCallback);



        try {

            assertThat(gatt.requestMtu(1024)).isTrue();

            // verify(gattCallback, timeout(5000).atLeast(1)).onMtuChanged(eq(gatt),

            // eq(ANDROID_MTU), eq(BluetoothGatt.GATT_FAILURE));

            // status should be 0x87 (GATT_ILLEGAL_PARAMETER) but not defined.

            verify(gattCallback, timeout(5000).atLeast(1))

                    .onMtuChanged(

                            eq(gatt),

                            anyInt(),

                            AdditionalMatchers.not(eq(BluetoothGatt.GATT_SUCCESS)));

        } finally {

            disconnectAndWaitDisconnection(gatt, gattCallback);

        }

system/bta/gatt/bta_gattc_act.cc

+18 −12
Original line number Diff line number Diff line
@@ -1264,9 +1264,12 @@ static void bta_gattc_exec_cmpl(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_OP_CMP 


/** configure MTU operation complete */

static void bta_gattc_cfg_mtu_cmpl(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_OP_CMPL* p_data) {

  tBTA_GATTC cb_data;



  p_clcb->status = p_data->status;

  if (p_clcb->p_q_cmd) {

    GATT_CONFIGURE_MTU_OP_CB cb = p_clcb->p_q_cmd->api_mtu.mtu_cb;

    void* my_cb_data = p_clcb->p_q_cmd->api_mtu.mtu_cb_data;

  tBTA_GATTC cb_data;



    osi_free_and_reset((void**)&p_clcb->p_q_cmd);
@@ -1274,26 +1277,30 @@ static void bta_gattc_cfg_mtu_cmpl(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_OP_ 
      p_clcb->p_srcb->mtu = p_data->p_cmpl->mtu;

    }



    if (cb) {

      cb(p_clcb->bta_conn_id, p_data->status, my_cb_data);

    }

  }



  /* configure MTU complete, callback */

  p_clcb->status = p_data->status;

  cb_data.cfg_mtu.conn_id = p_clcb->bta_conn_id;

  cb_data.cfg_mtu.status = p_data->status;

  cb_data.cfg_mtu.mtu = p_clcb->p_srcb->mtu;



  if (cb) {

    cb(p_clcb->bta_conn_id, p_data->status, my_cb_data);

  }



  (*p_clcb->p_rcb->p_cback)(BTA_GATTC_CFG_MTU_EVT, &cb_data);

}



/** operation completed */

void bta_gattc_op_cmpl(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_DATA* p_data) {

  if (p_clcb->p_q_cmd == NULL) {

    if (com::android::bluetooth::flags::gatt_callback_on_failure() &&

        p_data->op_cmpl.op_code == GATTC_OPTYPE_CONFIG) {

      bta_gattc_cfg_mtu_cmpl(p_clcb, &p_data->op_cmpl);

      return;

    }

    log::error("No pending command gatt client command");

    return;

  }



  const tGATTC_OPTYPE op = p_data->op_cmpl.op_code;

  switch (op) {

    case GATTC_OPTYPE_READ:
@@ -1348,7 +1355,6 @@ void bta_gattc_op_cmpl(tBTA_GATTC_CLCB* p_clcb, const tBTA_GATTC_DATA* p_data) { 
    /* If there are more clients waiting for the MTU results on the same device,

     * lets trigger them now.

     */



    auto outstanding_conn_ids = GATTC_GetAndRemoveListOfConnIdsWaitingForMtuRequest(p_clcb->bda);

    for (auto conn_id : outstanding_conn_ids) {

      tBTA_GATTC_CLCB* p_clcb = bta_gattc_find_clcb_by_conn_id(conn_id);