perf(#281): fix PERF-13/14/15/16 — main-thread settings, 100Hz polling, leaked scope, WS disconnect (#359)
* perf(#281): fix PERF-13/14/15/16 — main-thread settings load, 100Hz polling, leaked scope, WS disconnect * fix(#281): fix SettingsViewModelTest failures from async init dispatcher --------- Co-authored-by: M57 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
9297413b2a
commit
3399880621
5 files changed
+78
-43
No files matched your search
@@ -9,7 +9,11 @@ import androidx.core.app.NotificationCompat
|
|||||||
import androidx.core.app.RemoteInput
|
import androidx.core.app.RemoteInput
|
||||||
import com.m57.hermescontrol.R
|
import com.m57.hermescontrol.R
|
||||||
import com.m57.hermescontrol.data.ws.HermesWsClient
|
import com.m57.hermescontrol.data.ws.HermesWsClient
|
||||||
|
import kotlinx.coroutines.CoroutineScope
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.SupervisorJob
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
|
import kotlinx.coroutines.withTimeout
|
||||||
|
|
||||||
open class NotificationReplyReceiver : BroadcastReceiver() {
|
open class NotificationReplyReceiver : BroadcastReceiver() {
|
||||||
companion object {
|
companion object {
|
||||||
@@ -17,6 +21,10 @@ open class NotificationReplyReceiver : BroadcastReceiver() {
|
|||||||
const val EXTRA_SESSION_ID = "extra_session_id"
|
const val EXTRA_SESSION_ID = "extra_session_id"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reusable scope for async reply processing — avoids creating a new
|
||||||
|
// unmanaged CoroutineScope per broadcast fire. (PERF-15)
|
||||||
|
private val replyScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test-friendly wrapper for [BroadcastReceiver.goAsync] which is `final`
|
* Test-friendly wrapper for [BroadcastReceiver.goAsync] which is `final`
|
||||||
* (Java) and cannot be mocked or overridden directly. Tests override this
|
* (Java) and cannot be mocked or overridden directly. Tests override this
|
||||||
@@ -49,9 +57,9 @@ open class NotificationReplyReceiver : BroadcastReceiver() {
|
|||||||
|
|
||||||
if (!replyText.isNullOrBlank() && !sessionId.isNullOrBlank()) {
|
if (!replyText.isNullOrBlank() && !sessionId.isNullOrBlank()) {
|
||||||
val pendingResult = goAsyncCompat()
|
val pendingResult = goAsyncCompat()
|
||||||
kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.Dispatchers.IO).launch {
|
replyScope.launch {
|
||||||
try {
|
try {
|
||||||
kotlinx.coroutines.withTimeout(5000L) {
|
withTimeout(5000L) {
|
||||||
val db =
|
val db =
|
||||||
com.m57.hermescontrol.data.local.HermesDatabase
|
com.m57.hermescontrol.data.local.HermesDatabase
|
||||||
.get(context)
|
.get(context)
|
||||||
|
|||||||
@@ -797,10 +797,11 @@ private fun StreamingBubbleWithTypingEffect(
|
|||||||
|
|
||||||
// Timer that ticks at the configured delay, incrementing the visible word
|
// Timer that ticks at the configured delay, incrementing the visible word
|
||||||
// count each tick. Stops ticking when streaming ends, then shows all words.
|
// count each tick. Stops ticking when streaming ends, then shows all words.
|
||||||
|
// Optimized: split is only called when waiting for new content, not per tick.
|
||||||
LaunchedEffect(Unit) {
|
LaunchedEffect(Unit) {
|
||||||
|
var wordCount = 0
|
||||||
while (true) {
|
while (true) {
|
||||||
val words = currentContent.value.split(" ")
|
if (visibleWordCount < wordCount) {
|
||||||
if (visibleWordCount < words.size) {
|
|
||||||
delay(currentDelayMs.value.toLong())
|
delay(currentDelayMs.value.toLong())
|
||||||
visibleWordCount++
|
visibleWordCount++
|
||||||
} else {
|
} else {
|
||||||
@@ -808,7 +809,11 @@ private fun StreamingBubbleWithTypingEffect(
|
|||||||
onAnimationComplete()
|
onAnimationComplete()
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
delay(10)
|
// Only split when we need to check for new content arriving
|
||||||
|
val words = currentContent.value.split(" ")
|
||||||
|
wordCount = words.size
|
||||||
|
if (visibleWordCount < wordCount) continue
|
||||||
|
delay(100) // was 10 — reduced from 100Hz to 10Hz
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
visibleWordCount = Int.MAX_VALUE
|
visibleWordCount = Int.MAX_VALUE
|
||||||
|
|||||||
@@ -1020,7 +1020,8 @@ class ChatViewModel(
|
|||||||
override fun onCleared() {
|
override fun onCleared() {
|
||||||
super.onCleared()
|
super.onCleared()
|
||||||
pendingCleanupJob?.cancel()
|
pendingCleanupJob?.cancel()
|
||||||
wsClient.disconnect()
|
// PERF-16: Don't disconnect the global HermesWsClient singleton when
|
||||||
|
// leaving the Chat screen — it's used by background notification reply.
|
||||||
}
|
}
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import com.m57.hermescontrol.data.remote.safeApiCall
|
|||||||
import com.m57.hermescontrol.theme.BottomNavDisplayMode
|
import com.m57.hermescontrol.theme.BottomNavDisplayMode
|
||||||
import com.m57.hermescontrol.theme.ThemePreference
|
import com.m57.hermescontrol.theme.ThemePreference
|
||||||
import com.m57.hermescontrol.theme.ThemePreset
|
import com.m57.hermescontrol.theme.ThemePreset
|
||||||
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
@@ -38,47 +39,57 @@ data class SettingsUiState(
|
|||||||
val bottomNavDisplayMode: BottomNavDisplayMode = BottomNavDisplayMode.ICON_AND_TEXT,
|
val bottomNavDisplayMode: BottomNavDisplayMode = BottomNavDisplayMode.ICON_AND_TEXT,
|
||||||
)
|
)
|
||||||
|
|
||||||
class SettingsViewModel : ViewModel() {
|
class SettingsViewModel(
|
||||||
|
private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
|
||||||
|
) : ViewModel() {
|
||||||
private val _uiState = MutableStateFlow(SettingsUiState())
|
private val _uiState = MutableStateFlow(SettingsUiState())
|
||||||
val uiState: StateFlow<SettingsUiState> = _uiState.asStateFlow()
|
val uiState: StateFlow<SettingsUiState> = _uiState.asStateFlow()
|
||||||
|
|
||||||
init {
|
init {
|
||||||
|
viewModelScope.launch(ioDispatcher) {
|
||||||
loadSettings()
|
loadSettings()
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private fun loadSettings() {
|
private suspend fun loadSettings() {
|
||||||
val selectedId = AuthManager.getSelectedProfileId()
|
val selectedId = AuthManager.getSelectedProfileId()
|
||||||
|
val host = AuthManager.getHost()
|
||||||
|
val port = AuthManager.getPort().toString()
|
||||||
|
val token = AuthManager.getToken() ?: ""
|
||||||
|
val autoReconnect = AuthManager.isAutoReconnect()
|
||||||
|
val themePreference = AuthManager.getThemePreference()
|
||||||
|
val useDynamicColors = AuthManager.isUseDynamicColors()
|
||||||
|
val themePreset = AuthManager.getThemePreset()
|
||||||
|
val selectedNavItems = AuthManager.getBottomNavItems()
|
||||||
|
val typingEffectEnabled = AuthManager.isTypingEffectEnabled()
|
||||||
|
val typingEffectDelayMs = AuthManager.getTypingEffectDelayMs()
|
||||||
|
val profiles = AuthManager.getConnectionProfiles()
|
||||||
|
val bottomNavDisplayMode = AuthManager.getBottomNavDisplayMode()
|
||||||
|
val renameProfileName =
|
||||||
|
profiles.firstOrNull { p -> p.id == selectedId }?.name ?: ""
|
||||||
_uiState.update {
|
_uiState.update {
|
||||||
it.copy(
|
it.copy(
|
||||||
host = AuthManager.getHost(),
|
host = host,
|
||||||
port = AuthManager.getPort().toString(),
|
port = port,
|
||||||
token = AuthManager.getToken() ?: "",
|
token = token,
|
||||||
autoReconnect = AuthManager.isAutoReconnect(),
|
autoReconnect = autoReconnect,
|
||||||
// B6 (Jun 18 2026, kanban t_86e9be9b): restore user's theme
|
themePreference = themePreference,
|
||||||
// choice from persistent storage on init. Previously this slot
|
useDynamicColors = useDynamicColors,
|
||||||
// was missing — always defaulted to ThemePreference.SYSTEM.
|
themePreset = themePreset,
|
||||||
themePreference = AuthManager.getThemePreference(),
|
selectedNavItems = selectedNavItems,
|
||||||
useDynamicColors = AuthManager.isUseDynamicColors(),
|
typingEffectEnabled = typingEffectEnabled,
|
||||||
themePreset = AuthManager.getThemePreset(),
|
typingEffectDelayMs = typingEffectDelayMs,
|
||||||
selectedNavItems = AuthManager.getBottomNavItems(),
|
profiles = profiles,
|
||||||
typingEffectEnabled = AuthManager.isTypingEffectEnabled(),
|
|
||||||
typingEffectDelayMs = AuthManager.getTypingEffectDelayMs(),
|
|
||||||
profiles = AuthManager.getConnectionProfiles(),
|
|
||||||
selectedProfileId = selectedId,
|
selectedProfileId = selectedId,
|
||||||
renameProfileName =
|
renameProfileName = renameProfileName,
|
||||||
AuthManager
|
bottomNavDisplayMode = bottomNavDisplayMode,
|
||||||
.getConnectionProfiles()
|
|
||||||
.firstOrNull { p ->
|
|
||||||
p.id == selectedId
|
|
||||||
}?.name ?: "",
|
|
||||||
bottomNavDisplayMode = AuthManager.getBottomNavDisplayMode(),
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fun selectProfile(profileId: String?) {
|
fun selectProfile(profileId: String?) {
|
||||||
AuthManager.setSelectedProfileId(profileId)
|
AuthManager.setSelectedProfileId(profileId)
|
||||||
loadSettings()
|
viewModelScope.launch(ioDispatcher) { loadSettings() }
|
||||||
ApiClient.rebuild()
|
ApiClient.rebuild()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -95,7 +106,7 @@ class SettingsViewModel : ViewModel() {
|
|||||||
if (it.id == currentId) it.copy(name = newName) else it
|
if (it.id == currentId) it.copy(name = newName) else it
|
||||||
}
|
}
|
||||||
AuthManager.saveConnectionProfiles(updatedProfiles)
|
AuthManager.saveConnectionProfiles(updatedProfiles)
|
||||||
loadSettings()
|
viewModelScope.launch(ioDispatcher) { loadSettings() }
|
||||||
}
|
}
|
||||||
|
|
||||||
fun deleteProfile(profileId: String) {
|
fun deleteProfile(profileId: String) {
|
||||||
@@ -105,7 +116,7 @@ class SettingsViewModel : ViewModel() {
|
|||||||
if (AuthManager.getSelectedProfileId() == profileId) {
|
if (AuthManager.getSelectedProfileId() == profileId) {
|
||||||
AuthManager.setSelectedProfileId(null)
|
AuthManager.setSelectedProfileId(null)
|
||||||
}
|
}
|
||||||
loadSettings()
|
viewModelScope.launch(ioDispatcher) { loadSettings() }
|
||||||
ApiClient.rebuild()
|
ApiClient.rebuild()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -207,9 +218,11 @@ class SettingsViewModel : ViewModel() {
|
|||||||
AuthManager.setTypingEffectDelayMs(state.typingEffectDelayMs)
|
AuthManager.setTypingEffectDelayMs(state.typingEffectDelayMs)
|
||||||
ApiClient.rebuild()
|
ApiClient.rebuild()
|
||||||
|
|
||||||
|
viewModelScope.launch(ioDispatcher) {
|
||||||
loadSettings()
|
loadSettings()
|
||||||
_uiState.update { it.copy(isSaved = true, testResult = null) }
|
_uiState.update { it.copy(isSaved = true, testResult = null) }
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fun testConnection() {
|
fun testConnection() {
|
||||||
val state = _uiState.value
|
val state = _uiState.value
|
||||||
@@ -225,7 +238,7 @@ class SettingsViewModel : ViewModel() {
|
|||||||
|
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
val result =
|
val result =
|
||||||
withContext(Dispatchers.IO) {
|
withContext(ioDispatcher) {
|
||||||
safeApiCall { ApiClient.hermesApi.getStatus() }
|
safeApiCall { ApiClient.hermesApi.getStatus() }
|
||||||
}
|
}
|
||||||
when (result) {
|
when (result) {
|
||||||
|
|||||||
@@ -39,6 +39,12 @@ class SettingsViewModelTest {
|
|||||||
ConnectionProfile("prof-2", "Home", "10.0.0.2", 9220),
|
ConnectionProfile("prof-2", "Home", "10.0.0.2", 9220),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
private fun createViewModel(): SettingsViewModel {
|
||||||
|
val vm = SettingsViewModel(ioDispatcher = testDispatcher)
|
||||||
|
testDispatcher.scheduler.advanceUntilIdle()
|
||||||
|
return vm
|
||||||
|
}
|
||||||
|
|
||||||
@Before
|
@Before
|
||||||
fun setUp() {
|
fun setUp() {
|
||||||
Dispatchers.setMain(testDispatcher)
|
Dispatchers.setMain(testDispatcher)
|
||||||
@@ -95,7 +101,7 @@ class SettingsViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
storedSelectedProfileId = "prof-1"
|
storedSelectedProfileId = "prof-1"
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
val state = viewModel.uiState.value
|
val state = viewModel.uiState.value
|
||||||
|
|
||||||
assertEquals(2, state.profiles.size)
|
assertEquals(2, state.profiles.size)
|
||||||
@@ -107,7 +113,7 @@ class SettingsViewModelTest {
|
|||||||
fun testLoadSettings_noSelectedProfile_renameEmpty() {
|
fun testLoadSettings_noSelectedProfile_renameEmpty() {
|
||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
assertEquals("", viewModel.uiState.value.renameProfileName)
|
assertEquals("", viewModel.uiState.value.renameProfileName)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -115,9 +121,10 @@ class SettingsViewModelTest {
|
|||||||
fun testSelectProfile_updatesAndRebuildsApi() {
|
fun testSelectProfile_updatesAndRebuildsApi() {
|
||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
|
|
||||||
viewModel.selectProfile("prof-2")
|
viewModel.selectProfile("prof-2")
|
||||||
|
testDispatcher.scheduler.advanceUntilIdle()
|
||||||
|
|
||||||
verify { AuthManager.setSelectedProfileId("prof-2") }
|
verify { AuthManager.setSelectedProfileId("prof-2") }
|
||||||
verify { ApiClient.rebuild() }
|
verify { ApiClient.rebuild() }
|
||||||
@@ -130,9 +137,10 @@ class SettingsViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
storedSelectedProfileId = "prof-1"
|
storedSelectedProfileId = "prof-1"
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
|
|
||||||
viewModel.selectProfile(null)
|
viewModel.selectProfile(null)
|
||||||
|
testDispatcher.scheduler.advanceUntilIdle()
|
||||||
|
|
||||||
verify { AuthManager.setSelectedProfileId(null) }
|
verify { AuthManager.setSelectedProfileId(null) }
|
||||||
verify { ApiClient.rebuild() }
|
verify { ApiClient.rebuild() }
|
||||||
@@ -141,7 +149,7 @@ class SettingsViewModelTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testOnRenameProfileNameChange_updatesState() {
|
fun testOnRenameProfileNameChange_updatesState() {
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
viewModel.onRenameProfileNameChange("New Name")
|
viewModel.onRenameProfileNameChange("New Name")
|
||||||
assertEquals("New Name", viewModel.uiState.value.renameProfileName)
|
assertEquals("New Name", viewModel.uiState.value.renameProfileName)
|
||||||
assertFalse(viewModel.uiState.value.isSaved)
|
assertFalse(viewModel.uiState.value.isSaved)
|
||||||
@@ -152,7 +160,7 @@ class SettingsViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
viewModel.onRenameProfileNameChange("Renamed Work")
|
viewModel.onRenameProfileNameChange("Renamed Work")
|
||||||
viewModel.renameProfile()
|
viewModel.renameProfile()
|
||||||
|
|
||||||
@@ -164,7 +172,7 @@ class SettingsViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
viewModel.onRenameProfileNameChange(" ")
|
viewModel.onRenameProfileNameChange(" ")
|
||||||
viewModel.renameProfile()
|
viewModel.renameProfile()
|
||||||
|
|
||||||
@@ -173,7 +181,7 @@ class SettingsViewModelTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testRenameProfile_noSelectedProfile_doesNothing() {
|
fun testRenameProfile_noSelectedProfile_doesNothing() {
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
viewModel.onRenameProfileNameChange("New Name")
|
viewModel.onRenameProfileNameChange("New Name")
|
||||||
viewModel.renameProfile()
|
viewModel.renameProfile()
|
||||||
|
|
||||||
@@ -185,7 +193,7 @@ class SettingsViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
|
|
||||||
viewModel.deleteProfile("prof-1")
|
viewModel.deleteProfile("prof-1")
|
||||||
|
|
||||||
@@ -200,7 +208,7 @@ class SettingsViewModelTest {
|
|||||||
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
every { AuthManager.getConnectionProfiles() } returns testProfiles
|
||||||
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
every { AuthManager.getSelectedProfileId() } returns "prof-1"
|
||||||
|
|
||||||
val viewModel = SettingsViewModel()
|
val viewModel = createViewModel()
|
||||||
|
|
||||||
viewModel.deleteProfile("prof-2")
|
viewModel.deleteProfile("prof-2")
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user