feat(chat): stabilize multi-image attachment picker (#986)
This commit is contained in:
1 parent
c7d7abf4a1
commit
0ae5ed4db2
11 files changed
+432
-55
No files matched your search
@@ -0,0 +1,104 @@
|
||||
package com.m57.hermescontrol
|
||||
|
||||
import android.os.Handler
|
||||
import android.os.Looper
|
||||
|
||||
/** Coordinates background reply handling and a bounded connection lease for external activities. */
|
||||
object ExternalActivityLifecycleGuard {
|
||||
private const val CONNECTION_LEASE_TIMEOUT_MS = 10 * 60 * 1_000L
|
||||
|
||||
private val lock = Any()
|
||||
private var generation = 0L
|
||||
private var externalActivityInFlight = false
|
||||
private var resultDelivered = false
|
||||
private var hostResumed = true
|
||||
private var releaseConnectionLease: (() -> Unit)? = null
|
||||
|
||||
fun launchExternalActivity(
|
||||
acquireConnectionLease: () -> Unit,
|
||||
releaseConnectionLease: () -> Unit,
|
||||
prepareForBackground: () -> Unit,
|
||||
cleanupAfterLaunchFailure: () -> Unit,
|
||||
scheduleTimeout: (Long, () -> Unit) -> Unit = ::scheduleTimeout,
|
||||
launch: () -> Unit,
|
||||
) {
|
||||
val launchGeneration =
|
||||
synchronized(lock) {
|
||||
generation += 1
|
||||
externalActivityInFlight = true
|
||||
resultDelivered = false
|
||||
this.releaseConnectionLease = releaseConnectionLease
|
||||
generation
|
||||
}
|
||||
|
||||
try {
|
||||
acquireConnectionLease()
|
||||
scheduleTimeout(CONNECTION_LEASE_TIMEOUT_MS) {
|
||||
finishExternalActivity(launchGeneration)
|
||||
}
|
||||
prepareForBackground()
|
||||
launch()
|
||||
} catch (error: Throwable) {
|
||||
cleanupAfterLaunchFailure()
|
||||
finishExternalActivity(launchGeneration)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
fun onHostPaused() {
|
||||
synchronized(lock) {
|
||||
hostResumed = false
|
||||
}
|
||||
}
|
||||
|
||||
fun onHostResumed() {
|
||||
val release =
|
||||
synchronized(lock) {
|
||||
hostResumed = true
|
||||
if (externalActivityInFlight && resultDelivered) finishExternalActivityLocked() else null
|
||||
}
|
||||
release?.invoke()
|
||||
}
|
||||
|
||||
fun externalActivityReturned() {
|
||||
val release =
|
||||
synchronized(lock) {
|
||||
if (!externalActivityInFlight) return
|
||||
resultDelivered = true
|
||||
if (hostResumed) finishExternalActivityLocked() else null
|
||||
}
|
||||
release?.invoke()
|
||||
}
|
||||
|
||||
internal fun resetForTest() {
|
||||
synchronized(lock) {
|
||||
generation += 1
|
||||
externalActivityInFlight = false
|
||||
resultDelivered = false
|
||||
hostResumed = true
|
||||
releaseConnectionLease = null
|
||||
}
|
||||
}
|
||||
|
||||
private fun finishExternalActivity(expectedGeneration: Long) {
|
||||
val release =
|
||||
synchronized(lock) {
|
||||
if (!externalActivityInFlight || generation != expectedGeneration) return
|
||||
finishExternalActivityLocked()
|
||||
}
|
||||
release?.invoke()
|
||||
}
|
||||
|
||||
private fun finishExternalActivityLocked(): (() -> Unit)? {
|
||||
externalActivityInFlight = false
|
||||
resultDelivered = false
|
||||
return releaseConnectionLease.also { releaseConnectionLease = null }
|
||||
}
|
||||
|
||||
private fun scheduleTimeout(
|
||||
delayMs: Long,
|
||||
action: () -> Unit,
|
||||
) {
|
||||
Handler(Looper.getMainLooper()).postDelayed(action, delayMs)
|
||||
}
|
||||
}
|
||||
@@ -75,19 +75,21 @@ class MainActivity : ComponentActivity() {
|
||||
consumeNotificationIntent(intent)
|
||||
}
|
||||
|
||||
override fun onStart() {
|
||||
super.onStart()
|
||||
override fun onResume() {
|
||||
super.onResume()
|
||||
NotificationHelper.setAppForeground(this, true)
|
||||
ExternalActivityLifecycleGuard.onHostResumed()
|
||||
NotificationHelper.stop(this)
|
||||
if (AuthManager.isGatedMode() || !AuthManager.getToken().isNullOrBlank()) {
|
||||
HermesWsClient.connect()
|
||||
}
|
||||
}
|
||||
|
||||
override fun onStop() {
|
||||
override fun onPause() {
|
||||
ExternalActivityLifecycleGuard.onHostPaused()
|
||||
NotificationHelper.setAppForeground(this, false)
|
||||
NotificationHelper.start(this)
|
||||
super.onStop()
|
||||
super.onPause()
|
||||
}
|
||||
|
||||
private fun consumeNotificationIntent(intent: Intent?) {
|
||||
|
||||
@@ -97,6 +97,7 @@ object HermesWsClient {
|
||||
private val intentionalClose = AtomicBoolean(false)
|
||||
private val acceptQueuedMessages = AtomicBoolean(true)
|
||||
private val appInForeground = AtomicBoolean(true)
|
||||
private val externalActivityConnectionLease = AtomicBoolean(false)
|
||||
private val messageQueue = ConcurrentLinkedQueue<String>()
|
||||
private val queuedMessagesById = ConcurrentHashMap<String, String>()
|
||||
private val outboundLock = Any()
|
||||
@@ -285,9 +286,19 @@ object HermesWsClient {
|
||||
if (!foreground) disconnectIfIdleInBackground()
|
||||
}
|
||||
|
||||
fun acquireExternalActivityConnectionLease() {
|
||||
externalActivityConnectionLease.set(true)
|
||||
}
|
||||
|
||||
fun releaseExternalActivityConnectionLease() {
|
||||
externalActivityConnectionLease.set(false)
|
||||
disconnectIfIdleInBackground()
|
||||
}
|
||||
|
||||
private fun disconnectIfIdleInBackground() {
|
||||
synchronized(outboundLock) {
|
||||
if (!appInForeground.get() &&
|
||||
!externalActivityConnectionLease.get() &&
|
||||
!pendingReply &&
|
||||
pendingCalls.isEmpty() &&
|
||||
messageQueue.isEmpty()
|
||||
|
||||
@@ -20,17 +20,22 @@ class ChatAttachmentsDelegate(
|
||||
name: String,
|
||||
mimeType: String,
|
||||
size: Long,
|
||||
) {
|
||||
) = addAttachments(
|
||||
listOf(
|
||||
Attachment(
|
||||
uri = uri,
|
||||
name = name,
|
||||
mimeType = mimeType,
|
||||
size = size,
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
fun addAttachments(attachments: List<Attachment>) {
|
||||
if (attachments.isEmpty()) return
|
||||
uiState.update { state ->
|
||||
val attachment =
|
||||
Attachment(
|
||||
uri = uri,
|
||||
name = name,
|
||||
mimeType = mimeType,
|
||||
size = size,
|
||||
)
|
||||
state.copy(
|
||||
pendingAttachments = state.pendingAttachments + attachment,
|
||||
pendingAttachments = state.pendingAttachments + attachments,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,6 +75,7 @@ import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import androidx.lifecycle.repeatOnLifecycle
|
||||
import androidx.lifecycle.viewmodel.compose.viewModel
|
||||
import com.m57.hermescontrol.ExternalActivityLifecycleGuard
|
||||
import com.m57.hermescontrol.HistoryScreen
|
||||
import com.m57.hermescontrol.NavigationController
|
||||
import com.m57.hermescontrol.R
|
||||
@@ -85,6 +86,7 @@ import com.m57.hermescontrol.data.update.AppUpdateState
|
||||
import com.m57.hermescontrol.data.update.UpdateNoticeManager
|
||||
import com.m57.hermescontrol.data.ws.ConnectionStatus
|
||||
import com.m57.hermescontrol.data.ws.HermesWsClient
|
||||
import com.m57.hermescontrol.notification.NotificationHelper
|
||||
import com.m57.hermescontrol.theme.LocalHermesStatusColors
|
||||
import com.m57.hermescontrol.ui.chat.components.ChatConnectionBanner
|
||||
import com.m57.hermescontrol.ui.chat.components.ChatInputBar
|
||||
@@ -170,6 +172,7 @@ fun ChatScreen(
|
||||
val saveAttachmentLauncher =
|
||||
rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
|
||||
try {
|
||||
ExternalActivityLifecycleGuard.externalActivityReturned()
|
||||
val path = pendingSavePath
|
||||
val destination = acceptedSaveDestination(result.resultCode, result.data?.data)
|
||||
if (destination != null && path != null) {
|
||||
@@ -293,6 +296,15 @@ fun ChatScreen(
|
||||
val snackbarHostState = remember { SnackbarHostState() }
|
||||
val isDark = isSystemInDarkTheme()
|
||||
val context = LocalContext.current
|
||||
val launchExternalActivity: (() -> Unit) -> Unit = { launch ->
|
||||
ExternalActivityLifecycleGuard.launchExternalActivity(
|
||||
acquireConnectionLease = HermesWsClient::acquireExternalActivityConnectionLease,
|
||||
releaseConnectionLease = HermesWsClient::releaseExternalActivityConnectionLease,
|
||||
prepareForBackground = { NotificationHelper.start(context) },
|
||||
cleanupAfterLaunchFailure = { NotificationHelper.stop(context) },
|
||||
launch = launch,
|
||||
)
|
||||
}
|
||||
|
||||
val micListeningPrompt = stringResource(R.string.chat_mic_listening)
|
||||
val sttNotAvailableMsg = stringResource(R.string.stt_not_available)
|
||||
@@ -303,6 +315,7 @@ fun ChatScreen(
|
||||
rememberLauncherForActivityResult(
|
||||
ActivityResultContracts.StartActivityForResult(),
|
||||
) { result ->
|
||||
ExternalActivityLifecycleGuard.externalActivityReturned()
|
||||
isListening = false
|
||||
if (result.resultCode == Activity.RESULT_OK) {
|
||||
val spokenText =
|
||||
@@ -327,6 +340,7 @@ fun ChatScreen(
|
||||
rememberLauncherForActivityResult(
|
||||
ActivityResultContracts.RequestPermission(),
|
||||
) { granted ->
|
||||
ExternalActivityLifecycleGuard.externalActivityReturned()
|
||||
if (granted) {
|
||||
if (SpeechRecognizer.isRecognitionAvailable(context)) {
|
||||
val intent =
|
||||
@@ -341,7 +355,9 @@ fun ChatScreen(
|
||||
)
|
||||
}
|
||||
isListening = true
|
||||
speechLauncher.launch(intent)
|
||||
launchExternalActivity {
|
||||
speechLauncher.launch(intent)
|
||||
}
|
||||
} else {
|
||||
scrollScope.launch {
|
||||
snackbarHostState.showSnackbar(sttNotAvailableMsg)
|
||||
@@ -354,24 +370,43 @@ fun ChatScreen(
|
||||
}
|
||||
}
|
||||
|
||||
// File picker launcher for attachments (issue #195)
|
||||
// Multi-file picker for attachments (issue #195).
|
||||
val filePickerLauncher =
|
||||
rememberLauncherForActivityResult(
|
||||
ActivityResultContracts.GetContent(),
|
||||
) { uri: Uri? ->
|
||||
if (uri != null) {
|
||||
val cursor = context.contentResolver.query(uri, null, null, null, null)
|
||||
cursor?.use { c ->
|
||||
if (c.moveToFirst()) {
|
||||
val nameIdx = c.getColumnIndex(OpenableColumns.DISPLAY_NAME)
|
||||
val sizeIdx = c.getColumnIndex(OpenableColumns.SIZE)
|
||||
val name = if (nameIdx >= 0) c.getString(nameIdx) else uri.lastPathSegment ?: "file"
|
||||
val size = if (sizeIdx >= 0) c.getLong(sizeIdx) else 0L
|
||||
val mimeType = context.contentResolver.getType(uri) ?: "application/octet-stream"
|
||||
viewModel.addAttachment(uri.toString(), name, mimeType, size)
|
||||
}
|
||||
ActivityResultContracts.GetMultipleContents(),
|
||||
) { uris: List<Uri> ->
|
||||
ExternalActivityLifecycleGuard.externalActivityReturned()
|
||||
val attachments =
|
||||
uris.mapNotNull { uri ->
|
||||
runCatching {
|
||||
var name = uri.lastPathSegment ?: "file"
|
||||
var size = 0L
|
||||
context.contentResolver
|
||||
.query(
|
||||
uri,
|
||||
arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE),
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
)?.use { cursor ->
|
||||
if (cursor.moveToFirst()) {
|
||||
val nameIdx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)
|
||||
val sizeIdx = cursor.getColumnIndex(OpenableColumns.SIZE)
|
||||
if (nameIdx >= 0 && !cursor.isNull(nameIdx)) name = cursor.getString(nameIdx)
|
||||
if (sizeIdx >= 0 && !cursor.isNull(sizeIdx)) size = cursor.getLong(sizeIdx)
|
||||
}
|
||||
}
|
||||
Attachment(
|
||||
uri = uri.toString(),
|
||||
name = name,
|
||||
mimeType = context.contentResolver.getType(uri) ?: "application/octet-stream",
|
||||
size = size,
|
||||
)
|
||||
}.onFailure { error ->
|
||||
Log.w("ChatScreen", "Skipping unreadable picked attachment", error)
|
||||
}.getOrNull()
|
||||
}
|
||||
}
|
||||
viewModel.addAttachments(attachments)
|
||||
}
|
||||
|
||||
// Camera photo launcher (issue #195)
|
||||
@@ -381,6 +416,7 @@ fun ChatScreen(
|
||||
rememberLauncherForActivityResult(
|
||||
ActivityResultContracts.TakePicture(),
|
||||
) { success ->
|
||||
ExternalActivityLifecycleGuard.externalActivityReturned()
|
||||
val uri = pendingCameraUri
|
||||
pendingCameraUri = null
|
||||
if (success && uri != null) {
|
||||
@@ -635,23 +671,25 @@ fun ChatScreen(
|
||||
pendingSavePath = viewModel.gatewayPathFor(attachment)
|
||||
pendingSaveName = attachment.name
|
||||
pendingSaveMimeType = attachment.mimeType
|
||||
saveAttachmentLauncher.launch(
|
||||
Intent(Intent.ACTION_CREATE_DOCUMENT).apply {
|
||||
addCategory(Intent.CATEGORY_OPENABLE)
|
||||
type =
|
||||
attachment.mimeType
|
||||
.substringBefore(';')
|
||||
.trim()
|
||||
.takeIf { it.isNotBlank() } ?: "application/octet-stream"
|
||||
putExtra(
|
||||
Intent.EXTRA_TITLE,
|
||||
attachment.name
|
||||
.substringAfterLast('/')
|
||||
.substringAfterLast('\\')
|
||||
.ifBlank { "download" },
|
||||
)
|
||||
},
|
||||
)
|
||||
launchExternalActivity {
|
||||
saveAttachmentLauncher.launch(
|
||||
Intent(Intent.ACTION_CREATE_DOCUMENT).apply {
|
||||
addCategory(Intent.CATEGORY_OPENABLE)
|
||||
type =
|
||||
attachment.mimeType
|
||||
.substringBefore(';')
|
||||
.trim()
|
||||
.takeIf { it.isNotBlank() } ?: "application/octet-stream"
|
||||
putExtra(
|
||||
Intent.EXTRA_TITLE,
|
||||
attachment.name
|
||||
.substringAfterLast('/')
|
||||
.substringAfterLast('\\')
|
||||
.ifBlank { "download" },
|
||||
)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -754,14 +792,18 @@ fun ChatScreen(
|
||||
)
|
||||
}
|
||||
isListening = true
|
||||
speechLauncher.launch(intent)
|
||||
launchExternalActivity {
|
||||
speechLauncher.launch(intent)
|
||||
}
|
||||
} else {
|
||||
scrollScope.launch {
|
||||
snackbarHostState.showSnackbar(sttNotAvailableMsg)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
micPermissionLauncher.launch(Manifest.permission.RECORD_AUDIO)
|
||||
launchExternalActivity {
|
||||
micPermissionLauncher.launch(Manifest.permission.RECORD_AUDIO)
|
||||
}
|
||||
}
|
||||
},
|
||||
isListening = isListening,
|
||||
@@ -784,14 +826,34 @@ fun ChatScreen(
|
||||
photoFile,
|
||||
)
|
||||
pendingCameraUri = uri
|
||||
cameraLauncher.launch(uri)
|
||||
launchExternalActivity {
|
||||
cameraLauncher.launch(uri)
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
Log.e("ChatScreen", "Camera launch failed", e)
|
||||
}
|
||||
},
|
||||
onImageTap = { filePickerLauncher.launch("image/*") },
|
||||
onFileTap = { filePickerLauncher.launch("*/*") },
|
||||
onImageTap = {
|
||||
launchExternalActivity {
|
||||
filePickerLauncher.launch("image/*")
|
||||
}
|
||||
},
|
||||
onFileTap = {
|
||||
launchExternalActivity {
|
||||
filePickerLauncher.launch("*/*")
|
||||
}
|
||||
},
|
||||
onRemoveAttachment = viewModel::removeAttachment,
|
||||
onPreviewAttachment = { attachment ->
|
||||
if (attachment.isImage) {
|
||||
viewingImage =
|
||||
ImageViewerModel(
|
||||
model = attachment.uri,
|
||||
name = attachment.name,
|
||||
mimeType = attachment.mimeType,
|
||||
)
|
||||
}
|
||||
},
|
||||
// Composer toolbar wiring (PR 1)
|
||||
currentSessionModel = state.currentSessionModel,
|
||||
reasoningLevel = state.reasoningLevel,
|
||||
|
||||
@@ -1512,6 +1512,8 @@ class ChatViewModel(
|
||||
size: Long,
|
||||
) = attachmentsDelegate.addAttachment(uri, name, mimeType, size)
|
||||
|
||||
fun addAttachments(attachments: List<Attachment>) = attachmentsDelegate.addAttachments(attachments)
|
||||
|
||||
fun removeAttachment(index: Int) = attachmentsDelegate.removeAttachment(index)
|
||||
|
||||
fun openAttachment(attachment: Attachment) = mediaDelegate.openAttachment(attachment)
|
||||
|
||||
@@ -25,7 +25,9 @@ import androidx.compose.foundation.layout.size
|
||||
import androidx.compose.foundation.layout.width
|
||||
import androidx.compose.foundation.layout.widthIn
|
||||
import androidx.compose.foundation.lazy.LazyColumn
|
||||
import androidx.compose.foundation.lazy.LazyRow
|
||||
import androidx.compose.foundation.lazy.items
|
||||
import androidx.compose.foundation.lazy.itemsIndexed
|
||||
import androidx.compose.foundation.shape.RoundedCornerShape
|
||||
import androidx.compose.foundation.text.BasicTextField
|
||||
import androidx.compose.material.icons.Icons
|
||||
@@ -88,6 +90,7 @@ fun ChatInputBar(
|
||||
onImageTap: () -> Unit = {},
|
||||
onFileTap: () -> Unit = {},
|
||||
onRemoveAttachment: (Int) -> Unit = {},
|
||||
onPreviewAttachment: (Attachment) -> Unit = {},
|
||||
// NEW: composer toolbar wiring
|
||||
currentSessionModel: String? = null,
|
||||
reasoningLevel: String? = null,
|
||||
@@ -272,16 +275,17 @@ fun ChatInputBar(
|
||||
enter = fadeIn() + expandVertically(),
|
||||
exit = fadeOut() + shrinkVertically(),
|
||||
) {
|
||||
Row(
|
||||
LazyRow(
|
||||
modifier =
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
.padding(horizontal = 12.dp, vertical = 4.dp),
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
) {
|
||||
pendingAttachments.forEachIndexed { index, attachment ->
|
||||
itemsIndexed(pendingAttachments) { index, attachment ->
|
||||
AttachmentChip(
|
||||
attachment = attachment,
|
||||
onPreview = { onPreviewAttachment(attachment) },
|
||||
onRemove = { onRemoveAttachment(index) },
|
||||
)
|
||||
}
|
||||
@@ -475,6 +479,7 @@ fun ChatInputBar(
|
||||
@Composable
|
||||
fun AttachmentChip(
|
||||
attachment: Attachment,
|
||||
onPreview: () -> Unit,
|
||||
onRemove: () -> Unit,
|
||||
modifier: Modifier = Modifier,
|
||||
) {
|
||||
@@ -494,8 +499,10 @@ fun AttachmentChip(
|
||||
contentDescription = attachment.name,
|
||||
modifier =
|
||||
Modifier
|
||||
.size(24.dp)
|
||||
.clip(RoundedCornerShape(4.dp)),
|
||||
.size(40.dp)
|
||||
.clip(RoundedCornerShape(4.dp))
|
||||
.clickable(onClick = onPreview)
|
||||
.testTag("attachment_preview"),
|
||||
contentScale = ContentScale.Crop,
|
||||
)
|
||||
Spacer(modifier = Modifier.width(4.dp))
|
||||
|
||||
Reference in new issue
Block a user