Skip to content

fix: negotiate MTU and survive truncated FTMS packets #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cahenesy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: negotiate MTU and survive truncated FTMS packets #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
39 changes: 37 additions & 2 deletions app/src/main/java/net/duhowpi/ftmsbridge/ble/BleConnectionManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ class BleConnectionManager(

private var controlPointChar: BluetoothGattCharacteristic? = null

/** ATT MTU in force on the current link; the notification payload is this minus 3. */
var negotiatedMtu = DEFAULT_ATT_MTU
private set

var isConnected = false
private set

Expand Down Expand Up @@ -231,10 +235,17 @@ class BleConnectionManager(
when (newState) {
BluetoothProfile.STATE_CONNECTED -> {
Log.i(tag, "Connected to ${gatt.device.name}")
debugLogger.logMessage("GATT connected, discovering services...")
isConnected = true
listener?.onConnected(gatt.device.name ?: "Unknown")
gatt.discoverServices()
// The default ATT MTU of 23 leaves only 20 bytes for a notification,
// which is not enough for a Treadmill Data packet that includes heart
// rate. Ask for more before discovering services; discovery starts from
// onMtuChanged either way, so a refusal is not fatal.
debugLogger.logMessage("GATT connected, requesting MTU $REQUESTED_MTU...")
if (!gatt.requestMtu(REQUESTED_MTU)) {
debugLogger.logMessage("MTU request rejected outright, discovering services...")
gatt.discoverServices()
}
}
BluetoothProfile.STATE_DISCONNECTED -> {
Log.i(tag, "Disconnected")
Expand All @@ -255,6 +266,17 @@ class BleConnectionManager(
}
}

override fun onMtuChanged(gatt: BluetoothGatt, mtu: Int, status: Int) {
if (ActivityCompat.checkSelfPermission(context, Manifest.permission.BLUETOOTH_CONNECT) != PackageManager.PERMISSION_GRANTED) return
// mtu includes the 3-byte ATT header, so the usable notification payload is mtu - 3.
negotiatedMtu = if (status == BluetoothGatt.GATT_SUCCESS) mtu else DEFAULT_ATT_MTU
debugLogger.logMessage(
"MTU negotiated: $negotiatedMtu (status=$status, payload=${negotiatedMtu - ATT_HEADER_SIZE} bytes)"
)
Log.i(tag, "MTU negotiated: $negotiatedMtu (status=$status)")
gatt.discoverServices()
}

override fun onServicesDiscovered(gatt: BluetoothGatt, status: Int) {
if (status != BluetoothGatt.GATT_SUCCESS) {
debugLogger.logMessage("Service discovery failed: $status")
Expand Down Expand Up @@ -486,5 +508,18 @@ class BleConnectionManager(
companion object {
private const val RESISTANCE_LOG_MIN_LEVEL = 1
private const val RESISTANCE_LOG_MAX_LEVEL = 22

/** ATT MTU every LE link starts at, leaving a 20-byte notification payload. */
private const val DEFAULT_ATT_MTU = 23

/** Bytes of ATT overhead in every notification. */
private const val ATT_HEADER_SIZE = 3

/**
* MTU requested on connect. A Treadmill Data packet carrying heart rate needs 21
* bytes, one more than the default link allows, so the last field is dropped.
* 517 is the maximum an LE link can negotiate; devices are free to answer lower.
*/
private const val REQUESTED_MTU = 517
}
}
47 changes: 43 additions & 4 deletions app/src/main/java/net/duhowpi/ftmsbridge/ftms/FtmsDataParser.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,43 @@ object FtmsDataParser {

private val tag = "FtmsDataParser"

/**
* Bytes of zero padding added behind every packet before parsing.
*
* A machine can describe more fields in its flags than the link can carry: with the
* default ATT MTU of 23 a notification holds 20 bytes, while a Treadmill Data packet
* that includes heart rate needs 21. Parsing such a packet straight out of a
* [ByteBuffer] throws [java.nio.BufferUnderflowException] part way through, and
* because notifications are delivered on a binder callback thread the exception is
* swallowed and the entire sample is lost — including the speed and distance that
* were present at the front of the packet.
*
* Truncation only ever removes trailing fields, so padding lets the fields that did
* arrive parse normally while the missing ones read back as zero. 16 bytes covers the
* longest tail any of these characteristics can declare.
*/
private const val TRUNCATION_PADDING = 16

/** Wraps [data] for parsing so that a short packet yields partial data, not none. */
private fun paddedBuffer(data: ByteArray): ByteBuffer =
ByteBuffer.wrap(data.copyOf(data.size + TRUNCATION_PADDING)).order(ByteOrder.LITTLE_ENDIAN)

/**
* Logs when a packet described more data than it carried, so a machine whose
* notifications do not fit the current MTU is visible rather than silently degraded.
*/
private fun warnIfTruncated(buf: ByteBuffer, data: ByteArray, flags: Int, what: String) {
if (buf.position() > data.size) {
Log.w(
tag,
"$what packet truncated: flags=0x%04X declared %d bytes, link delivered %d"
.format(flags, buf.position(), data.size)
)
}
}

fun parseTreadmillData(data: ByteArray): FitnessSample {
val buf = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN)
val buf = paddedBuffer(data)
val flags = buf.short.toInt() and 0xFFFF

// Bit 0: More Data (1 = no instantaneous speed in this packet)
Expand Down Expand Up @@ -100,6 +135,7 @@ object FtmsDataParser {
buf.short // skip percentage (sint16)
}

warnIfTruncated(buf, data, flags, "Treadmill")
Log.d(tag, "treadmill raw: speed=%.2f km/h incl=%.1f%% dist=${totalDistanceM}m kcal=${totalEnergyKcal} hr=${heartRateBpm} t=${elapsedTimeSec}s"
.format(speedKmh, inclinationPercent))
return FitnessSample(
Expand All @@ -122,7 +158,7 @@ object FtmsDataParser {
}

fun parseIndoorBikeData(data: ByteArray): FitnessSample {
val buf = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN)
val buf = paddedBuffer(data)
val flags = buf.short.toInt() and 0xFFFF

// Bit 0: More Data (0 = Instantaneous Speed present)
Expand Down Expand Up @@ -198,6 +234,7 @@ object FtmsDataParser {
remainingTimeSec = buf.short.toInt() and 0xFFFF
}

warnIfTruncated(buf, data, flags, "Indoor bike")
return FitnessSample(
speedKmh = speedKmh,
averageSpeedKmh = averageSpeedKmh,
Expand Down Expand Up @@ -238,7 +275,7 @@ object FtmsDataParser {
* bit 14: Remaining Time present (uint16 s)
*/
fun parseCrossTrainerData(data: ByteArray): FitnessSample {
val buf = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN)
val buf = paddedBuffer(data)
val flags = buf.short.toInt() and 0xFFFF

var speedKmh = 0.0
Expand Down Expand Up @@ -327,6 +364,7 @@ object FtmsDataParser {
remainingTimeSec = buf.short.toInt() and 0xFFFF
}

warnIfTruncated(buf, data, flags, "Cross trainer")
return FitnessSample(
speedKmh = speedKmh,
averageSpeedKmh = averageSpeedKmh,
Expand Down Expand Up @@ -366,7 +404,7 @@ object FtmsDataParser {
* bit 9: Remaining Time present (uint16 s)
*/
fun parseStairClimberData(data: ByteArray): FitnessSample {
val buf = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN)
val buf = paddedBuffer(data)
val flags = buf.short.toInt() and 0xFFFF

var cadenceRpm = 0.0
Expand Down Expand Up @@ -424,6 +462,7 @@ object FtmsDataParser {
remainingTimeSec = buf.short.toInt() and 0xFFFF
}

warnIfTruncated(buf, data, flags, "Stair climber")
return FitnessSample(
cadenceRpm = cadenceRpm,
averageCadenceRpm = averageCadenceRpm,
Expand Down