diff --git a/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsScreen.kt b/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsScreen.kt index ffc06691..22034b16 100644 --- a/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsScreen.kt +++ b/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsScreen.kt @@ -44,6 +44,8 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.SegmentedButton import androidx.compose.material3.SegmentedButtonDefaults import androidx.compose.material3.SingleChoiceSegmentedButtonRow +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable @@ -95,12 +97,18 @@ fun DeviceSettingsScreenHost( LaunchedEffect(address) { vm.initialize(address) } val context = LocalContext.current + val snackbarHostState = remember { SnackbarHostState() } LaunchedEffect(Unit) { vm.events.collect { event -> when (event) { DeviceSettingsViewModel.Event.OpenBluetoothSettings -> { context.startActivity(Intent(Settings.ACTION_BLUETOOTH_SETTINGS)) } + is DeviceSettingsViewModel.Event.SendFailed -> { + snackbarHostState.showSnackbar( + context.getString(R.string.device_settings_send_failed, event.message ?: ""), + ) + } } } } @@ -110,6 +118,7 @@ fun DeviceSettingsScreenHost( DeviceSettingsScreen( state = currentState, + snackbarHostState = snackbarHostState, onNavigateUp = { vm.navUp() }, onAncModeChange = { vm.setAncMode(it) }, onConversationalAwarenessChange = { vm.setConversationalAwareness(it) }, @@ -137,6 +146,7 @@ fun DeviceSettingsScreenHost( @Composable fun DeviceSettingsScreen( state: DeviceSettingsViewModel.State, + snackbarHostState: SnackbarHostState = remember { SnackbarHostState() }, onNavigateUp: () -> Unit, onAncModeChange: (AapSetting.AncMode.Value) -> Unit = {}, onConversationalAwarenessChange: (Boolean) -> Unit = {}, @@ -195,6 +205,7 @@ fun DeviceSettingsScreen( }, ) }, + snackbarHost = { SnackbarHost(hostState = snackbarHostState) }, ) { paddingValues -> LazyColumn( modifier = Modifier.padding(paddingValues), @@ -216,7 +227,7 @@ fun DeviceSettingsScreen( connectionStateLabel = stateDetection?.state?.getLabel(context), lastSeen = device.lastSeenFormatted(state.now), firstSeen = firstSeen, - canRename = device.isAapConnected, + canRename = device.isAapReady, onRename = onDeviceNameChange, ) } @@ -1010,22 +1021,38 @@ private fun RenameDialog( ) { var textValue by remember { mutableStateOf(currentName) } + // The decoder in DefaultAapDeviceProfile only round-trips printable ASCII (0x20..0x7E), + // so even if the device accepts a UTF-8 name we can't display it back correctly. Accept any + // input up to the 32-byte UX cap, but flag non-ASCII with an inline error so the user + // understands why Rename is disabled. + val hasInvalidAscii = textValue.any { it.code !in 0x20..0x7E } + val isValid = textValue.isNotBlank() && !hasInvalidAscii + androidx.compose.material3.AlertDialog( onDismissRequest = onDismiss, title = { Text(stringResource(R.string.device_settings_rename_label)) }, text = { androidx.compose.material3.OutlinedTextField( value = textValue, - onValueChange = { if (it.length <= 32) textValue = it }, + onValueChange = { newValue -> + // US_ASCII encoding maps non-ASCII chars to '?' (1 byte each), giving a + // stable upper bound equal to the UTF-16 char count. Keeps the cap the user + // sees consistent regardless of character content. + if (newValue.toByteArray(Charsets.US_ASCII).size <= 32) textValue = newValue + }, singleLine = true, label = { Text(stringResource(R.string.device_settings_rename_hint)) }, + isError = hasInvalidAscii, + supportingText = if (hasInvalidAscii) { + { Text(stringResource(R.string.device_settings_rename_invalid_ascii)) } + } else null, modifier = Modifier.fillMaxWidth(), ) }, confirmButton = { androidx.compose.material3.TextButton( - onClick = { if (textValue.isNotBlank()) onConfirm(textValue) }, - enabled = textValue.isNotBlank() && textValue != currentName, + onClick = { if (isValid) onConfirm(textValue) }, + enabled = isValid && textValue != currentName, ) { Text(stringResource(R.string.device_settings_rename_confirm)) } diff --git a/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModel.kt b/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModel.kt index 8f8b011d..945081cd 100644 --- a/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModel.kt +++ b/app/src/main/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModel.kt @@ -57,6 +57,7 @@ class DeviceSettingsViewModel @Inject constructor( sealed interface Event { data object OpenBluetoothSettings : Event + data class SendFailed(val command: AapCommand, val message: String?) : Event } val events = SingleEventFlow() @@ -130,27 +131,26 @@ class DeviceSettingsViewModel @Inject constructor( } } - private fun send(command: AapCommand) { + private suspend fun sendInternal(command: AapCommand) { val address = targetAddress.value ?: return - launch { - try { - aapManager.sendCommand(address, command) - log(TAG) { "Sent $command to $address" } - } catch (e: Exception) { - log(TAG) { "Failed to send $command: ${e.message}" } - } + try { + aapManager.sendCommand(address, command) + log(TAG) { "Sent $command to $address" } + } catch (e: Exception) { + log(TAG, WARN) { "Failed to send $command: ${e.message}" } + // SingleEventFlow is backed by a BUFFERED Channel — use the suspending emit to avoid + // dropping the event under momentary backpressure. + events.emit(Event.SendFailed(command, e.message)) } } + private fun send(command: AapCommand) { + launch { sendInternal(command) } + } + private fun sendProGated(command: AapCommand) = launch { if (upgradeRepo.isPro()) { - val address = targetAddress.value ?: return@launch - try { - aapManager.sendCommand(address, command) - log(TAG) { "Sent $command to $address" } - } catch (e: Exception) { - log(TAG) { "Failed to send $command: ${e.message}" } - } + sendInternal(command) } else { navTo(Nav.Main.Upgrade) } diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/AapConnection.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/AapConnection.kt index 5c649695..ef2ac604 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/AapConnection.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/AapConnection.kt @@ -174,8 +174,19 @@ internal class AapConnection( } } + val preSendDeviceInfo = currentState.deviceInfo applyOptimisticUpdate(currentState, command) - sendRaw(command) + try { + sendRaw(command) + } catch (e: Exception) { + // Rollback is scoped to rename — the name is the one user-visible writable field + // that has no device echo to correct an incorrect optimistic update, so a failed + // send would otherwise leave the UI permanently lying about the device name. + if (command is AapCommand.SetDeviceName && preSendDeviceInfo != null) { + _state.value = _state.value.copy(deviceInfo = preSendDeviceInfo) + } + throw e + } } /** @@ -244,7 +255,12 @@ internal class AapConnection( is AapCommand.SetSleepDetection -> { AapSetting.SleepDetection::class to AapSetting.SleepDetection(enabled = command.enabled) } - is AapCommand.SetDeviceName -> return // No optimistic state — name comes from deviceInfo + is AapCommand.SetDeviceName -> { + val currentInfo = baseState.deviceInfo ?: return + _state.value = baseState + .copy(deviceInfo = currentInfo.copy(name = command.name), lastMessageAt = Instant.now()) + return + } } _state.value = baseState.withSetting(updated.first, updated.second).copy(lastMessageAt = Instant.now()) } diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/DefaultAapDeviceProfile.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/DefaultAapDeviceProfile.kt index 14e9de85..af2a25ce 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/DefaultAapDeviceProfile.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/protocol/DefaultAapDeviceProfile.kt @@ -449,11 +449,24 @@ class DefaultAapDeviceProfile( } private fun buildRenameMessage(name: String): ByteArray { + // Uses the opcode 0x1A format from the LibrePods AAP docs + Linux implementation. + // Verified end-to-end on AirPods Pro 2 USB-C (firmware 81.2675...): the device accepts + // the rename, persists it, and echoes the new name back via the next 0x001D device info + // message on reconnect. + // + // Note: the LibrePods Android code (AACPManager.createRenamePacket) uses a DIFFERENT + // format with opcode 0x1E and a trailing NUL, but on-device testing shows the device + // silently ignores that variant. The Linux / documented format is the one that works. + // + // Scope: this only changes the AirPods firmware's self-reported name (what 0x001D + // returns). It does NOT update the phone's Bluetooth alias — Android's system Bluetooth + // settings read from the bond database, which is separate and would require + // BluetoothDevice.setAlias(). That's intentionally out of scope here. val nameBytes = name.toByteArray(Charsets.UTF_8) require(nameBytes.size <= 127) { "Device name too long: ${nameBytes.size} bytes (max 127)" } return byteArrayOf( 0x04, 0x00, 0x04, 0x00, - 0x1E, 0x00, + 0x1A, 0x00, 0x01, nameBytes.size.toByte(), 0x00, ) + nameBytes } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index bb5f71b2..96558db7 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -495,6 +495,8 @@ Rename Device name Rename + Only ASCII characters are supported + Could not apply setting: %1$s Connected Devices Other devices currently connected to these AirPods Device %d diff --git a/app/src/test/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModelTest.kt b/app/src/test/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModelTest.kt index 5721aa45..00a13b89 100644 --- a/app/src/test/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModelTest.kt +++ b/app/src/test/java/eu/darken/capod/main/ui/devicesettings/DeviceSettingsViewModelTest.kt @@ -7,11 +7,15 @@ import eu.darken.capod.common.upgrade.UpgradeRepo import eu.darken.capod.monitor.core.DeviceMonitor import eu.darken.capod.monitor.core.PodDevice import eu.darken.capod.pods.core.apple.aap.AapConnectionManager +import eu.darken.capod.pods.core.apple.aap.protocol.AapCommand import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.shouldBeInstanceOf +import io.mockk.Called import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -215,4 +219,46 @@ class DeviceSettingsViewModelTest : BaseTest() { coVerify(exactly = 1) { bluetoothManager.nudgeConnection(bonded) } vm.state.first().isForceConnecting shouldBe false } + + @Test + fun `setDeviceName forwards SetDeviceName command to aapManager`() = runTest(testDispatcher) { + val vm = createViewModel() + vm.initialize(testAddress) + vm.state.first() + + vm.setDeviceName("NewName") + + coVerify { aapManager.sendCommand(testAddress, AapCommand.SetDeviceName("NewName")) } + } + + @Test + fun `setDeviceName when no target address is a no-op`() = runTest(testDispatcher) { + val vm = createViewModel() + // Intentionally skip initialize — targetAddress stays null. + + vm.setDeviceName("NewName") + + // `any()` can't be used here (AapCommand is sealed, mockk can't stub it), + // so verify the entire manager was not called instead. + verify { aapManager wasNot Called } + } + + @Test + fun `setDeviceName failure emits SendFailed event`() = runTest(testDispatcher) { + val failure = IllegalStateException("socket closed") + coEvery { + aapManager.sendCommand(testAddress, AapCommand.SetDeviceName("NewName")) + } throws failure + + val vm = createViewModel() + vm.initialize(testAddress) + vm.state.first() + + vm.setDeviceName("NewName") + + val event = vm.events.first() + val sendFailed = event.shouldBeInstanceOf() + sendFailed.command shouldBe AapCommand.SetDeviceName("NewName") + sendFailed.message shouldBe "socket closed" + } } diff --git a/app/src/test/java/eu/darken/capod/pods/core/apple/aap/devices/DefaultAapDeviceProfileNewSettingsTest.kt b/app/src/test/java/eu/darken/capod/pods/core/apple/aap/devices/DefaultAapDeviceProfileNewSettingsTest.kt index 76ef5270..98976a82 100644 --- a/app/src/test/java/eu/darken/capod/pods/core/apple/aap/devices/DefaultAapDeviceProfileNewSettingsTest.kt +++ b/app/src/test/java/eu/darken/capod/pods/core/apple/aap/devices/DefaultAapDeviceProfileNewSettingsTest.kt @@ -128,16 +128,23 @@ class DefaultAapDeviceProfileNewSettingsTest : BaseAapSessionTest() { @Test fun `decode unknown returns null`() { profile.decodeSetting(settingsMessage(0x31, 0x00)).shouldBeNull() } } - // ── Device Rename (0x1E) ──────────────────────────────── + // ── Device Rename (0x1A) ──────────────────────────────── + // The working opcode is 0x1A (not the 0x1E variant in LibrePods Android); see the rationale + // comment in DefaultAapDeviceProfile.buildRenameMessage for the on-device test details. @Nested inner class DeviceRenameTests { @Test - fun `encode simple ASCII name`() { + fun `encode simple ASCII name full frame`() { + // Locks in the full wire format: header + opcode prefix (1A 00 01) + length (LE u16) + name. val bytes = profile.encodeCommand(AapCommand.SetDeviceName("MyPods")) - bytes[4] shouldBe 0x1E.toByte() - bytes[6] shouldBe 6.toByte() // length - String(bytes, 8, 6, Charsets.UTF_8) shouldBe "MyPods" + val expected = byteArrayOf( + 0x04, 0x00, 0x04, 0x00, + 0x1A, 0x00, 0x01, + 0x06, 0x00, + 0x4D, 0x79, 0x50, 0x6F, 0x64, 0x73, + ) + bytes shouldBe expected } @Test @@ -145,8 +152,9 @@ class DefaultAapDeviceProfileNewSettingsTest : BaseAapSessionTest() { val name = "AirPods \uD83C\uDFA7" // headphone emoji val nameBytes = name.toByteArray(Charsets.UTF_8) val bytes = profile.encodeCommand(AapCommand.SetDeviceName(name)) - bytes[6] shouldBe nameBytes.size.toByte() - String(bytes, 8, nameBytes.size, Charsets.UTF_8) shouldBe name + bytes.size shouldBe 9 + nameBytes.size + bytes[7] shouldBe nameBytes.size.toByte() + String(bytes, 9, nameBytes.size, Charsets.UTF_8) shouldBe name } @Test @@ -159,7 +167,8 @@ class DefaultAapDeviceProfileNewSettingsTest : BaseAapSessionTest() { fun `encode accepts 127 byte name`() { val name = "A".repeat(127) val bytes = profile.encodeCommand(AapCommand.SetDeviceName(name)) - bytes[6] shouldBe 127.toByte() + bytes.size shouldBe 9 + 127 + bytes[7] shouldBe 127.toByte() } }