Skip to content
This repository was archived by the owner on Jul 4, 2024. It is now read-only.

Remove enum ambiguity from TransactionResult.responseStatus() #36

Open
droolingsheep wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Remove enum ambiguity from TransactionResult.responseStatus() #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions src/main/java/com/fitbit/bluetooth/fbgatt/GattClientCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
import com.fitbit.bluetooth.fbgatt.tx.GattClientDiscoverServicesTransaction;
import com.fitbit.bluetooth.fbgatt.tx.RequestGattClientPhyChangeTransaction;
import com.fitbit.bluetooth.fbgatt.tx.RequestMtuGattTransaction;
import com.fitbit.bluetooth.fbgatt.util.GattDisconnectReason;
import com.fitbit.bluetooth.fbgatt.util.GattStatus;
import com.fitbit.bluetooth.fbgatt.util.GattUtils;

import android.bluetooth.BluetoothDevice;
import android.bluetooth.BluetoothGatt;
import android.bluetooth.BluetoothGattCallback;
Expand All @@ -25,13 +25,11 @@
import android.bluetooth.BluetoothProfile;
import android.os.Handler;
import android.os.Looper;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;

import timber.log.Timber;

/**
Expand Down Expand Up @@ -123,7 +121,7 @@ public void onPhyUpdate(BluetoothGatt gatt, int txPhy, int rxPhy, int status) {
.txPhy(txPhy)
.rxPhy(rxPhy)
.gattState(conn.getGattState())
.responseStatus(GattDisconnectReason.getReasonForCode(status).ordinal()).build(), conn);
.responseStatus(GattStatus.getStatusForCode(status)).build(), conn);
}
});
}
Expand Down Expand Up @@ -224,7 +222,7 @@ public void onConnectionStateChange(BluetoothGatt gatt, int status, int newState
asyncConnListener.onClientConnectionStateChanged(new TransactionResult.Builder()
.resultStatus(TransactionResult.TransactionResultStatus.FAILURE)
.gattState(conn.getGattState())
.responseStatus(GattDisconnectReason.getReasonForCode(status).ordinal()).build(), conn);
.disconnectReason(GattDisconnectReason.getReasonForCode(status)).build(), conn);
}
}, HOT_QUEUE_EMPTYING_TIME);
} else {
Expand Down Expand Up @@ -256,8 +254,7 @@ public void onConnectionStateChange(BluetoothGatt gatt, int status, int newState
for(ConnectionEventListener asyncConnListener : conn.getConnectionEventListeners()) {
handler.post(() -> asyncConnListener.onClientConnectionStateChanged(new TransactionResult.Builder()
.resultStatus(TransactionResult.TransactionResultStatus.SUCCESS)
.gattState(conn.getGattState())
.responseStatus(GattDisconnectReason.getReasonForCode(status).ordinal()).build(), conn));
.gattState(conn.getGattState()).build(), conn));
}
});
break;
Expand Down Expand Up @@ -298,7 +295,7 @@ public void onServicesDiscovered(BluetoothGatt gatt, int status) {
.transactionName(GattClientDiscoverServicesTransaction.NAME)
.serverServices(discoveredServices)
.gattState(conn.getGattState())
.responseStatus(GattDisconnectReason.getReasonForCode(status).ordinal()).build(), conn);
.responseStatus(GattStatus.getStatusForCode(status)).build(), conn);
}
});
}
Expand Down Expand Up @@ -473,7 +470,7 @@ public void onMtuChanged(BluetoothGatt gatt, int mtu, int status) {
.transactionName(RequestMtuGattTransaction.NAME)
.mtu(mtu)
.gattState(conn.getGattState())
.responseStatus(GattDisconnectReason.getReasonForCode(status).ordinal()).build();
.responseStatus(GattStatus.getStatusForCode(status)).build();
// since this is one of the events that could happen asynchronously, we will
// need to iterate through our connection listeners
handler.post(() -> {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/fitbit/bluetooth/fbgatt/GattConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ public void simulateDisconnect() {
asyncConnListener.onClientConnectionStateChanged(new TransactionResult.Builder()
.resultStatus(TransactionResult.TransactionResultStatus.FAILURE)
.gattState(getGattState())
.responseStatus(GattStatus.GATT_UNKNOWN.ordinal()).build(), this);
.responseStatus(GattStatus.GATT_UNKNOWN).build(), this);
}
} else {
throw new IllegalStateException(String.format(Locale.ENGLISH, "[%s] You can't simulate a disconnection if you are not in mock mode", getDevice()));
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/fitbit/bluetooth/fbgatt/GattDisconnectReason.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* Created by iowens on 8/10/17.
*/

@Deprecated

@droolingsheep droolingsheep Sep 26, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also just delete this enum entirely in favor of the other one.

@ionutlepi ionutlepi Sep 29, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say yes with renaming the other one to GattFailureReason since they are not all ending in a gatt disconnect.

enum GattDisconnectReason {
GATT_CONN_UNKNOWN(0),
GATT_CONN_NO_RESOURCES(4), /* connection fail for l2cap resource failure */
Expand Down
9 changes: 3 additions & 6 deletions src/main/java/com/fitbit/bluetooth/fbgatt/GattServerCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@

import com.fitbit.bluetooth.fbgatt.btcopies.BluetoothGattCharacteristicCopy;
import com.fitbit.bluetooth.fbgatt.btcopies.BluetoothGattDescriptorCopy;
import com.fitbit.bluetooth.fbgatt.util.GattDisconnectReason;
import com.fitbit.bluetooth.fbgatt.util.GattStatus;
import com.fitbit.bluetooth.fbgatt.util.GattUtils;

import android.bluetooth.BluetoothDevice;
import android.bluetooth.BluetoothGattCharacteristic;
import android.bluetooth.BluetoothGattDescriptor;
Expand All @@ -23,13 +23,11 @@
import android.os.Build;
import android.os.Handler;
import android.os.Looper;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.UUID;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
Expand Down Expand Up @@ -112,7 +110,7 @@ public void onConnectionStateChange(BluetoothDevice device, int status, int newS
// success because we received this data
TransactionResult result = new TransactionResult.Builder()
.gattState(conn.getGattState())
.responseStatus(status)
.disconnectReason(GattDisconnectReason.getReasonForCode(status))
.resultStatus(TransactionResult.TransactionResultStatus.FAILURE).build();
handler.post(() -> asyncListener.onServerConnectionStateChanged(device, result, conn));
}
Expand All @@ -127,7 +125,6 @@ public void onConnectionStateChange(BluetoothDevice device, int status, int newS
// success because we received this data
TransactionResult result = new TransactionResult.Builder()
.gattState(conn.getGattState())
.responseStatus(status)
.resultStatus(TransactionResult.TransactionResultStatus.SUCCESS).build();
handler.post(() -> asyncListener.onServerConnectionStateChanged(device, result, conn));
}
Expand Down Expand Up @@ -395,7 +392,7 @@ public void onNotificationSent(BluetoothDevice device, int status) {
ArrayList<GattServerListener> copy = new ArrayList<>(listeners.size());
copy.addAll(listeners);
for (GattServerListener listener : copy) {
handler.post(() -> listener.onServerNotificationSent(device, GattStatus.getStatusForCode(status).ordinal()));
handler.post(() -> listener.onServerNotificationSent(device, GattStatus.getStatusForCode(status)));
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/fitbit/bluetooth/fbgatt/GattServerListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import com.fitbit.bluetooth.fbgatt.btcopies.BluetoothGattCharacteristicCopy;
import com.fitbit.bluetooth.fbgatt.btcopies.BluetoothGattDescriptorCopy;
import com.fitbit.bluetooth.fbgatt.util.GattStatus;

import android.bluetooth.BluetoothDevice;
import android.bluetooth.BluetoothGattCharacteristic;
Expand Down Expand Up @@ -117,7 +118,7 @@ interface GattServerListener {
* @param device The bluetooth device
* @param status The status
*/
void onServerNotificationSent(BluetoothDevice device, int status);
void onServerNotificationSent(BluetoothDevice device, GattStatus status);

/**
* *DO NOT USE UNLESS YOU KNOW WHAT YOU ARE DOING!!!* library internal method for notifying the {@link FitbitGatt} that the server
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/fitbit/bluetooth/fbgatt/GattServerTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import com.fitbit.bluetooth.fbgatt.btcopies.BluetoothGattCharacteristicCopy;
import com.fitbit.bluetooth.fbgatt.btcopies.BluetoothGattDescriptorCopy;
import com.fitbit.bluetooth.fbgatt.util.GattStatus;
import com.fitbit.bluetooth.fbgatt.util.GattUtils;
import android.bluetooth.BluetoothDevice;
import android.bluetooth.BluetoothGattService;
Expand Down Expand Up @@ -162,7 +163,7 @@ public void onServerExecuteWrite(BluetoothDevice device, int requestId, boolean
}

@Override
public void onServerNotificationSent(BluetoothDevice device, int status) {
public void onServerNotificationSent(BluetoothDevice device, GattStatus status) {
Timber.v("[%s] onServerNotificationSent not handled in tx: %s", utils.debugSafeGetBtDeviceName(device), getName());
}

Expand Down
56 changes: 41 additions & 15 deletions src/main/java/com/fitbit/bluetooth/fbgatt/TransactionResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,17 @@
package com.fitbit.bluetooth.fbgatt;

import com.fitbit.bluetooth.fbgatt.util.Bytes;
import com.fitbit.bluetooth.fbgatt.util.GattDisconnectReason;
import com.fitbit.bluetooth.fbgatt.util.GattStatus;

import android.bluetooth.BluetoothDevice;
import android.bluetooth.BluetoothGattCharacteristic;
import android.bluetooth.BluetoothGattService;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.UUID;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

/**
* A class to encapsulate the range of possible data to be returned from the transactions and the
Expand Down Expand Up @@ -56,7 +55,13 @@ public enum TransactionResultStatus {
/**
* The gatt operation response status
*/
private final int responseStatus;
private final GattStatus responseStatus;

/**
* The gatt disconnect reason
*/
@Nullable
private final GattDisconnectReason disconnectReason;
/**
* If handling a gatt server read or write request, the requestId for ensuring proper response
*/
Expand Down Expand Up @@ -146,7 +151,7 @@ public enum TransactionResultStatus {
* @param newTransactionName The new transaction name
*/
TransactionResult(TransactionResult result, String newTransactionName) {
this(result.resultState, result.resultStatus, result.responseStatus, result.rssi,
this(result.resultState, result.resultStatus, result.responseStatus, result.disconnectReason, result.rssi,
result.mtu, result.requestId, result.characteristicUuid, result.serviceUuid,
result.descriptorUuid, result.data, result.offset, result.gattServerServices,
result.preparedWrite, result.responseRequired, result.transactionName, result.txPhy,
Expand Down Expand Up @@ -178,7 +183,7 @@ public enum TransactionResultStatus {
* @param responseRequired Whether a response is required for a given write request
* @param transactionName The transaction name to get around obfuscation in logging
*/
TransactionResult(GattState state, TransactionResultStatus status, int responseStatus, int rssi,
TransactionResult(GattState state, TransactionResultStatus status, GattStatus responseStatus, GattDisconnectReason disconnectReason, int rssi,
int mtu, int requestId, UUID characteristicUuid, UUID serviceUuid,
UUID descriptorUuid, byte[] data, int offset, List<BluetoothGattService> services, boolean preparedWrite, boolean responseRequired, String transactionName, int txPhy, int rxPhy, List<TransactionResult> transactionResults) {
this.resultState = state;
Expand All @@ -199,6 +204,7 @@ public enum TransactionResultStatus {
this.txPhy = txPhy;
this.rxPhy = rxPhy;
this.transactionResults = new ArrayList<>(transactionResults);
this.disconnectReason = disconnectReason;
}

/**
Expand Down Expand Up @@ -335,8 +341,17 @@ public boolean isResponseRequired() {
* transaction response
* @return The gatt status enum for the ordinal
*/
public String getResponseCodeString(){
return GattStatus.values()[this.responseStatus].name();
public String getResponseCodeString() {
return this.responseStatus != null ? this.responseStatus.name() : "null";
}

/**
* Get the disconnect reason if there is one.
* @return The disconnect reason if there is one
*/
@Nullable
public GattDisconnectReason getDisconnectReason() {
return this.disconnectReason;
}

/**
Expand Down Expand Up @@ -370,7 +385,7 @@ public List<TransactionResult> getTransactionResults(){
*/
public static class Builder {

private int responseStatus;
private GattStatus responseStatus;
private int requestId;
private UUID characteristicUuid;
private UUID serviceUuid;
Expand All @@ -388,6 +403,7 @@ public static class Builder {
private boolean responseRequired;
private String transactionName = "Unknown";
private ArrayList<TransactionResult> results = new ArrayList<>();
private GattDisconnectReason disconnectReason;

public Builder() {

Expand Down Expand Up @@ -435,11 +451,21 @@ public Builder data(byte[] data) {

/**
* Adds the response status to this builder
* @param code The repsonse status ordinal
* @param status The response status ordinal
* @return This builder
*/
public Builder responseStatus(GattStatus status) {
this.responseStatus = status;
return this;
}

/**
* Adds the disconnect reason to this builder
* @param reason The disconnect reason
* @return This builder
*/
public Builder responseStatus(int code) {
this.responseStatus = code;
public Builder disconnectReason(GattDisconnectReason reason) {
this.disconnectReason = reason;
return this;
}

Expand Down Expand Up @@ -581,7 +607,7 @@ public Builder rxPhy(int rxPhy) {
* @return the {@link TransactionResult} with the properties described in the builder
*/
public TransactionResult build() {
return new TransactionResult(resultState, resultStatus, responseStatus, rssi,
return new TransactionResult(resultState, resultStatus, responseStatus, disconnectReason, rssi,
mtu, requestId, characteristicUuid, serviceUuid,
descriptorUuid, data, offset, services, preparedWrite, responseRequired, transactionName, txPhy, rxPhy, results);
}
Expand All @@ -590,6 +616,6 @@ public TransactionResult build() {

@Override
public String toString() {
return String.format(Locale.ENGLISH, "Transaction Name: %s, Gatt State: %s, Transaction Result Status: %s, Response Status: %s, rssi: %d, mtu: %d, Characteristic UUID: %s, Service UUID: %s, Descriptor UUID: %s, Data: %s, Offset: %d, txPhy: %d, rxPhy: %d, transaction results: %s", this.transactionName, this.resultState, this.resultStatus, GattStatus.getStatusForCode(this.responseStatus), this.rssi, this.mtu, this.characteristicUuid, this.serviceUuid, this.descriptorUuid, Bytes.byteArrayToHexString(this.data), this.offset, this.txPhy, this.rxPhy, this.transactionResults);
return String.format(Locale.ENGLISH, "Transaction Name: %s, Gatt State: %s, Transaction Result Status: %s, Response Status: %s, rssi: %d, mtu: %d, Characteristic UUID: %s, Service UUID: %s, Descriptor UUID: %s, Data: %s, Offset: %d, txPhy: %d, rxPhy: %d, transaction results: %s", this.transactionName, this.resultState, this.resultStatus, this.responseStatus, this.rssi, this.mtu, this.characteristicUuid, this.serviceUuid, this.descriptorUuid, Bytes.byteArrayToHexString(this.data), this.offset, this.txPhy, this.rxPhy, this.transactionResults);
}
}
6 changes: 3 additions & 3 deletions ...m/fitbit/bluetooth/fbgatt/tx/AddGattServerServiceCharacteristicDescriptorTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected void transaction(GattTransactionCallback callback) {
if(null != serviceChar) {
if(doesDescriptorAlreadyExist(serviceChar, descriptor)) {
TransactionResult.Builder builder = new TransactionResult.Builder().transactionName(getName());
builder.responseStatus(GattDisconnectReason.getReasonForCode(GattDisconnectReason.GATT_CONN_NO_RESOURCES.getCode()).ordinal());
builder.disconnectReason(GattDisconnectReason.GATT_CONN_NO_RESOURCES);

@ionutlepi ionutlepi Sep 29, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel for these cases we should maybe have it as failureReason since this does not result in a disconnection

getGattServer().setState(GattState.ADD_SERVICE_CHARACTERISTIC_DESCRIPTOR_FAILURE);
Timber.w("The gatt service characteristic descriptor %s, is a duplicate, and could not be added to characteristic: %s, on service: %s",
serviceChar.getUuid(), descriptor.getUuid(), service.getUuid());
Expand All @@ -83,7 +83,7 @@ protected void transaction(GattTransactionCallback callback) {
builder.serviceUuid(serverService.getUuid())
.characteristicUuid(characteristic.getUuid())
.descriptorUuid(descriptor.getUuid());
builder.responseStatus(GattStatus.GATT_SUCCESS.getCode());
builder.responseStatus(GattStatus.GATT_SUCCESS);
getGattServer().setState(GattState.ADD_SERVICE_CHARACTERISTIC_DESCRIPTOR_SUCCESS);
Timber.e("The gatt service characteristic descriptor could not be added: %s", service.getUuid());
builder.gattState(getGattServer().getGattState())
Expand All @@ -105,7 +105,7 @@ protected void transaction(GattTransactionCallback callback) {
private void respondWithError(String message, GattTransactionCallback callback) {
TransactionResult.Builder builder = new TransactionResult.Builder().transactionName(getName());
Timber.w(message);
builder.responseStatus(GattDisconnectReason.getReasonForCode(GattDisconnectReason.GATT_CONN_NO_RESOURCES.getCode()).ordinal());
builder.disconnectReason(GattDisconnectReason.GATT_CONN_NO_RESOURCES);
getGattServer().setState(GattState.ADD_SERVICE_CHARACTERISTIC_DESCRIPTOR_FAILURE);
Timber.e("The gatt service characteristic descriptor could not be added: %s", service.getUuid());
builder.gattState(getGattServer().getGattState())
Expand Down
Loading