From 26d303ebbde2ec6d92d495434fde978b115df91d Mon Sep 17 00:00:00 2001 From: Gaurav Ujjwal Date: Fri, 28 Jan 2022 21:26:53 +0530 Subject: [PATCH] Refactor connection state Until now, connection state was managed in VncClient. This was fine before introduction of SSH Tunnel, but now, state in VncClient may not fully reflect the state transitions. Moreover, most of the time we simply check whether we are connected or not. Now, simple boolean state is managed by VncClient, and more granular state is managed by VncViewModel. --- .../java/com/gaurav/avnc/vnc/VncClientTest.kt | 6 +- .../gaurav/avnc/ui/vnc/CredentialFragment.kt | 5 +- .../java/com/gaurav/avnc/ui/vnc/Dispatcher.kt | 5 +- .../com/gaurav/avnc/ui/vnc/VncActivity.kt | 11 +- .../com/gaurav/avnc/ui/vnc/gl/Renderer.kt | 3 +- .../com/gaurav/avnc/viewmodel/VncViewModel.kt | 81 ++++++++------ .../java/com/gaurav/avnc/vnc/Messenger.kt | 2 +- .../java/com/gaurav/avnc/vnc/VncClient.kt | 105 +++--------------- app/src/main/res/layout/activity_vnc.xml | 18 +-- 9 files changed, 85 insertions(+), 151 deletions(-) diff --git a/app/src/androidTest/java/com/gaurav/avnc/vnc/VncClientTest.kt b/app/src/androidTest/java/com/gaurav/avnc/vnc/VncClientTest.kt index 3ada22e..5cece43 100644 --- a/app/src/androidTest/java/com/gaurav/avnc/vnc/VncClientTest.kt +++ b/app/src/androidTest/java/com/gaurav/avnc/vnc/VncClientTest.kt @@ -10,7 +10,6 @@ package com.gaurav.avnc.vnc import com.gaurav.avnc.TestServer import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -23,7 +22,6 @@ class VncClientTest { override fun onCredentialRequired() = UserCredential() override fun onFramebufferUpdated() {} override fun onFramebufferSizeChanged(width: Int, height: Int) {} - override fun onClientStateChanged(newState: VncClient.State) {} override fun onGotXCutText(text: String) { cutText = text } @@ -47,8 +45,8 @@ class VncClientTest { private fun connect() { server.start() - assertTrue(client.connect(server.host, server.port)) - assertTrue(client.processServerMessage(10000)) + client.connect(server.host, server.port) + client.processServerMessage(10000) } diff --git a/app/src/main/java/com/gaurav/avnc/ui/vnc/CredentialFragment.kt b/app/src/main/java/com/gaurav/avnc/ui/vnc/CredentialFragment.kt index 67ee6d2..6a9162d 100644 --- a/app/src/main/java/com/gaurav/avnc/ui/vnc/CredentialFragment.kt +++ b/app/src/main/java/com/gaurav/avnc/ui/vnc/CredentialFragment.kt @@ -19,7 +19,6 @@ import com.gaurav.avnc.databinding.FragmentCredentialBinding import com.gaurav.avnc.model.LoginInfo import com.gaurav.avnc.viewmodel.VncViewModel import com.gaurav.avnc.vnc.UserCredential -import com.gaurav.avnc.vnc.VncClient import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.google.android.material.textfield.TextInputLayout @@ -79,8 +78,8 @@ class CredentialFragment : DialogFragment() { */ private fun scheduleCredentialSave(viewModel: VncViewModel, cred: UserCredential) { with(viewModel) { - clientState.observeForever { - if (it == VncClient.State.Connected) { + state.observeForever { + if (it == VncViewModel.State.Connected) { profile.username = cred.username profile.password = cred.password saveProfile() diff --git a/app/src/main/java/com/gaurav/avnc/ui/vnc/Dispatcher.kt b/app/src/main/java/com/gaurav/avnc/ui/vnc/Dispatcher.kt index d31cd80..a36d465 100644 --- a/app/src/main/java/com/gaurav/avnc/ui/vnc/Dispatcher.kt +++ b/app/src/main/java/com/gaurav/avnc/ui/vnc/Dispatcher.kt @@ -13,7 +13,6 @@ import com.gaurav.avnc.ui.vnc.Dispatcher.SwipeAction import com.gaurav.avnc.viewmodel.VncViewModel import com.gaurav.avnc.vnc.Messenger import com.gaurav.avnc.vnc.PointerButton -import com.gaurav.avnc.vnc.VncClient import kotlin.math.abs /** @@ -282,13 +281,13 @@ class Dispatcher(private val activity: VncActivity) { syncPending = true syncPosition() } - viewModel.clientState.observe(activity) { + viewModel.state.observe(activity) { syncPosition() } } private fun syncPosition() { - if (syncPending && viewModel.clientState.value == VncClient.State.Connected) { + if (syncPending && viewModel.client.connected) { pointerPosition.apply { x = viewModel.frameState.fbWidth / 2 y = viewModel.frameState.fbHeight / 2 diff --git a/app/src/main/java/com/gaurav/avnc/ui/vnc/VncActivity.kt b/app/src/main/java/com/gaurav/avnc/ui/vnc/VncActivity.kt index bb5eebf..4dfc924 100644 --- a/app/src/main/java/com/gaurav/avnc/ui/vnc/VncActivity.kt +++ b/app/src/main/java/com/gaurav/avnc/ui/vnc/VncActivity.kt @@ -35,7 +35,6 @@ import com.gaurav.avnc.model.ServerProfile import com.gaurav.avnc.util.Experimental import com.gaurav.avnc.util.SamsungDex import com.gaurav.avnc.viewmodel.VncViewModel -import com.gaurav.avnc.vnc.VncClient import com.gaurav.avnc.vnc.VncUri import kotlinx.coroutines.delay import java.lang.ref.WeakReference @@ -91,7 +90,7 @@ class VncActivity : AppCompatActivity() { viewModel.frameViewRef = WeakReference(binding.frameView) viewModel.credentialRequest.observe(this) { showCredentialDialog() } viewModel.sshHostKeyVerifyRequest.observe(this) { showHostKeyDialog() } - viewModel.clientState.observe(this) { onClientStateChanged(it) } + viewModel.state.observe(this) { onClientStateChanged(it) } viewModel.initConnection(profile) //Should be called after observers has been setup } @@ -157,8 +156,8 @@ class VncActivity : AppCompatActivity() { private fun closeDrawers() = binding.drawerLayout.closeDrawers() - private fun onClientStateChanged(newState: VncClient.State) { - if (newState == VncClient.State.Connected) { + private fun onClientStateChanged(newState: VncViewModel.State) { + if (newState == VncViewModel.State.Connected) { if (!viewModel.pref.runInfo.hasConnectedSuccessfully) { viewModel.pref.runInfo.hasConnectedSuccessfully = true @@ -254,7 +253,7 @@ class VncActivity : AppCompatActivity() { View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY window.decorView.apply { - if (viewModel.clientState.value == VncClient.State.Connected) + if (viewModel.client.connected) systemUiVisibility = systemUiVisibility or flags else systemUiVisibility = systemUiVisibility and flags.inv() @@ -286,7 +285,7 @@ class VncActivity : AppCompatActivity() { } private fun enterPiPMode() { - val canEnter = viewModel.pref.viewer.pipEnabled && viewModel.client.state == VncClient.State.Connected + val canEnter = viewModel.pref.viewer.pipEnabled && viewModel.client.connected if (canEnter && Build.VERSION.SDK_INT >= 26) { diff --git a/app/src/main/java/com/gaurav/avnc/ui/vnc/gl/Renderer.kt b/app/src/main/java/com/gaurav/avnc/ui/vnc/gl/Renderer.kt index 0cefd88..a39a5d0 100644 --- a/app/src/main/java/com/gaurav/avnc/ui/vnc/gl/Renderer.kt +++ b/app/src/main/java/com/gaurav/avnc/ui/vnc/gl/Renderer.kt @@ -12,7 +12,6 @@ import android.opengl.GLES20.* import android.opengl.GLSurfaceView import android.opengl.Matrix import com.gaurav.avnc.viewmodel.VncViewModel -import com.gaurav.avnc.vnc.VncClient import javax.microedition.khronos.egl.EGLConfig import javax.microedition.khronos.opengles.GL10 @@ -72,7 +71,7 @@ class Renderer(val viewModel: VncViewModel) : GLSurfaceView.Renderer { override fun onDrawFrame(gl: GL10?) { glClear(GL_COLOR_BUFFER_BIT) - if (viewModel.client.state != VncClient.State.Connected) + if (!viewModel.client.connected) return if (state.vpWidth == 0f || state.vpHeight == 0f) diff --git a/app/src/main/java/com/gaurav/avnc/viewmodel/VncViewModel.kt b/app/src/main/java/com/gaurav/avnc/viewmodel/VncViewModel.kt index 55d6312..adde545 100644 --- a/app/src/main/java/com/gaurav/avnc/viewmodel/VncViewModel.kt +++ b/app/src/main/java/com/gaurav/avnc/viewmodel/VncViewModel.kt @@ -65,24 +65,48 @@ import java.lang.ref.WeakReference */ class VncViewModel(app: Application) : BaseViewModel(app), VncClient.Observer { + /** + * Connection lifecycle: + * + * Created + * | + * v + * Connecting ----------+ + * | | + * v | + * Connected | + * | | + * v | + * Disconnected <-------+ + * + */ + enum class State { + Created, + Connecting, + Connected, + Disconnected, + } + val client = VncClient(this) - /** - * Client state is exposed as live data here to allow dynamic data binding - * from layouts. - */ - val clientState = MutableLiveData(client.state) - - /** - * Reason for [clientState] being [VncClient.State.Disconnected]. - */ - val disconnectReason = MutableLiveData("") - /** * [ServerProfile] used for current connection. */ var profile = ServerProfile() + /** + * We have two places for connection state (both are synced): + * + * [VncClient.connected] - Simple boolean state, used most of the time + * [state] - More granular, used by observers & data binding + */ + val state = MutableLiveData(State.Created) + + /** + * Reason for disconnecting. + */ + val disconnectReason = MutableLiveData("") + /** * Fired when [VncClient] has asked for credential. It is used to * show Credentials dialog to user and return the result to receiver @@ -146,21 +170,16 @@ class VncViewModel(app: Application) : BaseViewModel(app), VncClient.Observer { * Connection management **************************************************************************/ - /** - * Because [initConnection] can be called multiple times due to activity restarts, - * initialization inside it is performed only once. - */ - private var initialized = false - /** * Initialize VNC connection using given profile. + * [initConnection] can be called multiple times due to activity restarts. */ fun initConnection(profile: ServerProfile) { - if (initialized) + if (state.value != State.Created) return - initialized = true this.profile = profile + state.value = State.Connecting launchConnection() } @@ -175,12 +194,12 @@ class VncViewModel(app: Application) : BaseViewModel(app), VncClient.Observer { processMessages() }.onFailure { - client.disconnect() - if (it is IOException) - disconnectReason.postValue(it.message) + if (it is IOException) disconnectReason.postValue(it.message) Log.e("ReceiverCoroutine", "Connection failed", it) } + state.postValue(State.Disconnected) + //Wait until activity is finished and viewmodel is cleaned up. runCatching { awaitCancellation() } cleanup() @@ -208,16 +227,13 @@ class VncViewModel(app: Application) : BaseViewModel(app), VncClient.Observer { else -> throw IOException("Unknown Channel: ${profile.channelType}") } - if (client.state != VncClient.State.Connected) - throw IOException(client.getLastErrorStr()) + state.postValue(State.Connected) + sendClipboardText() //Initial sync } private fun processMessages() { - while (viewModelScope.isActive && client.processServerMessage()) { - //Message Loop - } - - disconnectReason.postValue(client.getLastErrorStr()) + while (viewModelScope.isActive) + client.processServerMessage() } private fun cleanup() { @@ -320,13 +336,6 @@ class VncViewModel(app: Application) : BaseViewModel(app), VncClient.Observer { receiveClipboardText(text) } - override fun onClientStateChanged(newState: VncClient.State) { - clientState.postValue(newState) - - if (newState == VncClient.State.Connected) - sendClipboardText() //Initial sync - } - override fun onFramebufferSizeChanged(width: Int, height: Int) { viewModelScope.launch(Dispatchers.Main) { frameState.setFramebufferSize(width.toFloat(), height.toFloat()) diff --git a/app/src/main/java/com/gaurav/avnc/vnc/Messenger.kt b/app/src/main/java/com/gaurav/avnc/vnc/Messenger.kt index c820a7d..cb162f5 100644 --- a/app/src/main/java/com/gaurav/avnc/vnc/Messenger.kt +++ b/app/src/main/java/com/gaurav/avnc/vnc/Messenger.kt @@ -78,7 +78,7 @@ class Messenger(private val client: VncClient) { } fun sendKey(keySym: Int, isDown: Boolean): Boolean { - if (client.state != VncClient.State.Connected) + if (!client.connected) return false execute { client.sendKeyEvent(keySym, isDown) } diff --git a/app/src/main/java/com/gaurav/avnc/vnc/VncClient.kt b/app/src/main/java/com/gaurav/avnc/vnc/VncClient.kt index ed6af58..6a92406 100644 --- a/app/src/main/java/com/gaurav/avnc/vnc/VncClient.kt +++ b/app/src/main/java/com/gaurav/avnc/vnc/VncClient.kt @@ -2,6 +2,7 @@ package com.gaurav.avnc.vnc import androidx.annotation.Keep import com.gaurav.avnc.vnc.VncClient.Observer +import java.io.IOException import java.nio.ByteBuffer import java.nio.charset.StandardCharsets @@ -35,33 +36,9 @@ import java.nio.charset.StandardCharsets */ class VncClient(private val observer: Observer) { - /** - * [VncClient] Lifecycle: - * - * Created ---------------+ - * | | - * v | - * Connecting ----------+ | - * | | | - * v | | - * Connected | | - * | | | - * v | | - * Disconnected <-------+ | - * | | - * v | - * Destroyed <-------------+ - */ - enum class State { - Created, - Connecting, - Connected, - Disconnected, - Destroyed - } - /** * Interface for event observer. + * DO NOT throw exceptions from these methods. */ interface Observer { fun onPasswordRequired(): String @@ -69,7 +46,6 @@ class VncClient(private val observer: Observer) { fun onGotXCutText(text: String) fun onFramebufferUpdated() fun onFramebufferSizeChanged(width: Int, height: Int) - fun onClientStateChanged(newState: State) //fun onBell() //fun onCursorMoved(x: Int, y: Int): Boolean @@ -86,14 +62,9 @@ class VncClient(private val observer: Observer) { throw RuntimeException("Could not create native rfbClient!") } - /** - * Current client state. - */ - var state: State = State.Created - private set(value) { - field = value - observer.onClientStateChanged(value) - } + @Volatile + var connected = false + private set /** * Name of remote desktop @@ -112,14 +83,10 @@ class VncClient(private val observer: Observer) { /** * Setup different properties for this client. - * Only valid in [State.Connected] state. * * @param securityType RFB security type to use. */ fun configure(viewOnly: Boolean, securityType: Int, useLocalCursor: Boolean) { - if (state != State.Created) - return - viewOnlyMode = viewOnly nativeConfigure(nativePtr, securityType, useLocalCursor) } @@ -130,42 +97,25 @@ class VncClient(private val observer: Observer) { /** * Initializes VNC connection. - * - * @return true if initialization was successful */ - fun connect(host: String, port: Int): Boolean { - if (state != State.Created) - return false - - state = State.Connecting - - if (nativeInit(nativePtr, host, port)) { - state = State.Connected - return true - } else { - state = State.Disconnected - return false - } + fun connect(host: String, port: Int) { + connected = nativeInit(nativePtr, host, port) + if (!connected) throw IOException(nativeGetLastErrorStr()) } /** - * Waits for incoming server message, parses it and then invokes appropriate - * callbacks. + * Waits for incoming server message, parses it and then invokes appropriate callbacks. * * @param uSecTimeout Timeout in microseconds. - * @return true if message was successfully handled or no message was received within timeout, - * false otherwise. */ - fun processServerMessage(uSecTimeout: Int = 1000000): Boolean { - if (state != State.Connected) - return false + fun processServerMessage(uSecTimeout: Int = 1000000) { + if (!connected) + return if (!nativeProcessServerMessage(nativePtr, uSecTimeout)) { - disconnect() //Reason: Msg processing will only fail on irrecoverable errors - return false + connected = false + throw IOException(nativeGetLastErrorStr()) } - - return true } /** @@ -209,30 +159,11 @@ class VncClient(private val observer: Observer) { fun uploadFrameTexture() = nativeUploadFrameTexture(nativePtr) /** - * Marks this client as 'disconnected'. - * - * This doesn't actually release the resources from server but - * simply updates the state. [cleanup] is used for that. - */ - fun disconnect() { - state = State.Disconnected - } - - /** - * Returns a string describing last error. - * Only valid in disconnected state. - */ - fun getLastErrorStr() = nativeGetLastErrorStr() - - /** - * Releases all resource (native & managed) currently held. - * After cleanup, this client MUST NOT be used anymore. + * Release all resources allocated by the client. + * DO NOT use this client after [cleanup]. */ fun cleanup() { - if (state == State.Destroyed) - return - - state = State.Destroyed + connected = false nativeCleanup(nativePtr) } @@ -241,7 +172,7 @@ class VncClient(private val observer: Observer) { * events to remote server. */ private inline fun executeSend(block: () -> Unit) { - if (state == State.Connected && !viewOnlyMode) + if (connected && !viewOnlyMode) block() } diff --git a/app/src/main/res/layout/activity_vnc.xml b/app/src/main/res/layout/activity_vnc.xml index 0db465a..b90f21c 100644 --- a/app/src/main/res/layout/activity_vnc.xml +++ b/app/src/main/res/layout/activity_vnc.xml @@ -12,7 +12,7 @@ - + + app:isVisible="@{viewModel.state == State.Connected}"> @@ -99,7 +99,7 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:background="?colorSurface" - app:isVisible="@{viewModel.clientState != State.Connected}"> + app:isVisible="@{viewModel.state != State.Connected}"> @@ -117,7 +117,7 @@ android:layout_height="60dp" android:importantForAccessibility="no" android:src="@drawable/ic_warning" - app:isVisible="@{viewModel.clientState == State.Disconnected}" + app:isVisible="@{viewModel.state == State.Disconnected}" app:layout_constraintBottom_toTopOf="@id/client_state" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -128,8 +128,8 @@ android:layout_width="wrap_content" android:layout_height="wrap_content" android:layout_margin="@dimen/margin_normal" - android:text="@{viewModel.clientState.toString()}" - app:isVisible="@{viewModel.clientState != State.Created}" + android:text="@{viewModel.state.toString()}" + app:isVisible="@{viewModel.state != State.Created}" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -144,7 +144,7 @@ android:gravity="center" android:text='@{ "( " + viewModel.disconnectReason + " )"}' android:textAppearance="@style/TextAppearance.AppCompat.Caption" - app:isVisible="@{viewModel.clientState == State.Disconnected && !viewModel.disconnectReason.empty}" + app:isVisible="@{viewModel.state == State.Disconnected && !viewModel.disconnectReason.empty}" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/client_state" @@ -156,7 +156,7 @@ android:background="@drawable/bg_circular_button" android:contentDescription="@string/desc_retry_connection_btn" android:src="@drawable/ic_refresh" - app:isVisible="@{viewModel.clientState == State.Disconnected}" + app:isVisible="@{viewModel.state == State.Disconnected}" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent"