From 1a22854d05d567be453fa2aadb839769fceaa9b7 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Thu, 9 May 2024 16:08:00 -0400 Subject: [PATCH] refactor: Remove all usage of knownChannels enumerateEuiccChannels() should return all discovered channels on its own. Outside classes should never access the cached open channels directly. --- .../core/DefaultEuiccChannelManager.kt | 28 ++++++------- .../openeuicc/core/EuiccChannelManager.kt | 20 ++++++--- .../ui/DirectProfileDownloadActivity.kt | 12 +++--- .../im/angry/openeuicc/ui/MainActivity.kt | 7 ++-- .../angry/openeuicc/ui/SlotSelectFragment.kt | 41 +++++++++++++------ .../util/PrivilegedTelephonyUtils.kt | 4 +- 6 files changed, 66 insertions(+), 46 deletions(-) diff --git a/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt b/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt index eb19c36..1d6627d 100644 --- a/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt +++ b/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt @@ -21,7 +21,7 @@ open class DefaultEuiccChannelManager( const val TAG = "EuiccChannelManager" } - private val channels = mutableListOf() + private val channelCache = mutableListOf() private val lock = Mutex() @@ -39,13 +39,13 @@ open class DefaultEuiccChannelManager( private suspend fun tryOpenEuiccChannel(port: UiccPortInfoCompat): EuiccChannel? { lock.withLock { val existing = - channels.find { it.slotId == port.card.physicalSlotIndex && it.portId == port.portIndex } + channelCache.find { it.slotId == port.card.physicalSlotIndex && it.portId == port.portIndex } if (existing != null) { if (existing.valid && port.logicalSlotIndex == existing.logicalSlotId) { return existing } else { existing.close() - channels.remove(existing) + channelCache.remove(existing) } } @@ -57,7 +57,7 @@ open class DefaultEuiccChannelManager( val channel = euiccChannelFactory.tryOpenEuiccChannel(port) ?: return null if (channel.valid) { - channels.add(channel) + channelCache.add(channel) return channel } else { Log.i( @@ -128,7 +128,7 @@ open class DefaultEuiccChannelManager( override suspend fun waitForReconnect(physicalSlotId: Int, portId: Int, timeoutMillis: Long) { // If there is already a valid channel, we close it proactively // Sometimes the current channel can linger on for a bit even after it should have become invalid - channels.find { it.slotId == physicalSlotId && it.portId == portId }?.apply { + channelCache.find { it.slotId == physicalSlotId && it.portId == portId }?.apply { if (valid) close() } @@ -148,30 +148,26 @@ open class DefaultEuiccChannelManager( } } - override suspend fun enumerateEuiccChannels() { + override suspend fun enumerateEuiccChannels(): List = withContext(Dispatchers.IO) { - for (uiccInfo in uiccCards) { - for (port in uiccInfo.ports) { - if (tryOpenEuiccChannel(port) != null) { + uiccCards.flatMap { info -> + info.ports.mapNotNull { port -> + tryOpenEuiccChannel(port)?.also { Log.d( TAG, - "Found eUICC on slot ${uiccInfo.physicalSlotIndex} port ${port.portIndex}" + "Found eUICC on slot ${info.physicalSlotIndex} port ${port.portIndex}" ) } } } } - } - - override val knownChannels: List - get() = channels.toList() override fun invalidate() { - for (channel in channels) { + for (channel in channelCache) { channel.close() } - channels.clear() + channelCache.clear() euiccChannelFactory.cleanup() } } \ No newline at end of file diff --git a/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt b/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt index 9ec3030..5171779 100644 --- a/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt +++ b/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt @@ -1,12 +1,22 @@ package im.angry.openeuicc.core +/** + * EuiccChannelManager holds references to, and manages the lifecycles of, individual + * APDU channels to SIM cards. The find* methods will create channels when needed, and + * all opened channels will be held in an internal cache until invalidate() is called + * or when this instance is destroyed. + * + * To precisely control the lifecycle of this object itself (and thus its cached channels), + * all other compoents must access EuiccChannelManager objects through EuiccChannelManagerService. + * Holding references independent of EuiccChannelManagerService is unsupported. + */ interface EuiccChannelManager { - val knownChannels: List - /** - * Scan all possible sources for EuiccChannels and have them cached for future use + * Scan all possible sources for EuiccChannels, return them and have all + * scanned channels cached; these channels will remain open for the entire lifetime of + * this EuiccChannelManager object, unless disconnected externally or invalidate()'d */ - suspend fun enumerateEuiccChannels() + suspend fun enumerateEuiccChannels(): List /** * Wait for a slot + port to reconnect (i.e. become valid again) @@ -41,7 +51,7 @@ interface EuiccChannelManager { fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel? /** - * Invalidate all EuiccChannels previously known by this Manager + * Invalidate all EuiccChannels previously cached by this Manager */ fun invalidate() diff --git a/app-common/src/main/java/im/angry/openeuicc/ui/DirectProfileDownloadActivity.kt b/app-common/src/main/java/im/angry/openeuicc/ui/DirectProfileDownloadActivity.kt index dbdbe9d..9e79de6 100644 --- a/app-common/src/main/java/im/angry/openeuicc/ui/DirectProfileDownloadActivity.kt +++ b/app-common/src/main/java/im/angry/openeuicc/ui/DirectProfileDownloadActivity.kt @@ -9,23 +9,23 @@ import kotlinx.coroutines.withContext class DirectProfileDownloadActivity : BaseEuiccAccessActivity(), SlotSelectFragment.SlotSelectedListener, OpenEuiccContextMarker { override fun onInit() { lifecycleScope.launch { - withContext(Dispatchers.IO) { + val knownChannels = withContext(Dispatchers.IO) { euiccChannelManager.enumerateEuiccChannels() } when { - euiccChannelManager.knownChannels.isEmpty() -> { + knownChannels.isEmpty() -> { finish() } - euiccChannelManager.knownChannels.hasMultipleChips -> { - SlotSelectFragment.newInstance() + knownChannels.hasMultipleChips -> { + SlotSelectFragment.newInstance(knownChannels.sortedBy { it.logicalSlotId }) .show(supportFragmentManager, SlotSelectFragment.TAG) } else -> { // If the device has only one eSIM "chip" (but may be mapped to multiple slots), // we can skip the slot selection dialog since there is only one chip to save to. - onSlotSelected(euiccChannelManager.knownChannels[0].slotId, - euiccChannelManager.knownChannels[0].portId) + onSlotSelected(knownChannels[0].slotId, + knownChannels[0].portId) } } } diff --git a/app-common/src/main/java/im/angry/openeuicc/ui/MainActivity.kt b/app-common/src/main/java/im/angry/openeuicc/ui/MainActivity.kt index ab89bab..9befe57 100644 --- a/app-common/src/main/java/im/angry/openeuicc/ui/MainActivity.kt +++ b/app-common/src/main/java/im/angry/openeuicc/ui/MainActivity.kt @@ -95,9 +95,8 @@ open class MainActivity : BaseEuiccAccessActivity(), OpenEuiccContextMarker { } private suspend fun init() { - withContext(Dispatchers.IO) { - euiccChannelManager.enumerateEuiccChannels() - euiccChannelManager.knownChannels.forEach { + val knownChannels = withContext(Dispatchers.IO) { + euiccChannelManager.enumerateEuiccChannels().onEach { Log.d(TAG, "slot ${it.slotId} port ${it.portId}") Log.d(TAG, it.lpa.eID) // Request the system to refresh the list of profiles every time we start @@ -108,7 +107,7 @@ open class MainActivity : BaseEuiccAccessActivity(), OpenEuiccContextMarker { } withContext(Dispatchers.Main) { - euiccChannelManager.knownChannels.sortedBy { it.logicalSlotId }.forEach { channel -> + knownChannels.sortedBy { it.logicalSlotId }.forEach { channel -> spinnerAdapter.add(getString(R.string.channel_name_format, channel.logicalSlotId)) fragments.add(appContainer.uiComponentFactory.createEuiccManagementFragment(channel)) } diff --git a/app-common/src/main/java/im/angry/openeuicc/ui/SlotSelectFragment.kt b/app-common/src/main/java/im/angry/openeuicc/ui/SlotSelectFragment.kt index 8fb36d9..d1239c4 100644 --- a/app-common/src/main/java/im/angry/openeuicc/ui/SlotSelectFragment.kt +++ b/app-common/src/main/java/im/angry/openeuicc/ui/SlotSelectFragment.kt @@ -16,8 +16,14 @@ class SlotSelectFragment : BaseMaterialDialogFragment(), OpenEuiccContextMarker companion object { const val TAG = "SlotSelectFragment" - fun newInstance(): SlotSelectFragment { - return SlotSelectFragment() + fun newInstance(knownChannels: List): SlotSelectFragment { + return SlotSelectFragment().apply { + arguments = Bundle().apply { + putIntArray("slotIds", knownChannels.map { it.slotId }.toIntArray()) + putIntArray("logicalSlotIds", knownChannels.map { it.logicalSlotId }.toIntArray()) + putIntArray("portIds", knownChannels.map { it.portId }.toIntArray()) + } + } } } @@ -28,10 +34,10 @@ class SlotSelectFragment : BaseMaterialDialogFragment(), OpenEuiccContextMarker private lateinit var toolbar: Toolbar private lateinit var spinner: Spinner - private val channels: List by lazy { - (requireActivity() as BaseEuiccAccessActivity).euiccChannelManager - .knownChannels.sortedBy { it.logicalSlotId } - } + private lateinit var adapter: ArrayAdapter + private lateinit var slotIds: IntArray + private lateinit var logicalSlotIds: IntArray + private lateinit var portIds: IntArray override fun onCreateView( inflater: LayoutInflater, @@ -44,26 +50,35 @@ class SlotSelectFragment : BaseMaterialDialogFragment(), OpenEuiccContextMarker toolbar.setTitle(R.string.slot_select) toolbar.inflateMenu(R.menu.fragment_slot_select) - val adapter = ArrayAdapter(inflater.context, R.layout.spinner_item) + adapter = ArrayAdapter(inflater.context, R.layout.spinner_item) spinner = view.requireViewById(R.id.spinner) spinner.adapter = adapter - channels.forEach { channel -> - adapter.add(getString(R.string.channel_name_format, channel.logicalSlotId)) + return view + } + + override fun onStart() { + super.onStart() + + slotIds = requireArguments().getIntArray("slotIds")!! + logicalSlotIds = requireArguments().getIntArray("logicalSlotIds")!! + portIds = requireArguments().getIntArray("portIds")!! + + logicalSlotIds.forEach { id -> + adapter.add(getString(R.string.channel_name_format, id)) } toolbar.setNavigationOnClickListener { (requireActivity() as SlotSelectedListener).onSlotSelectCancelled() } toolbar.setOnMenuItemClickListener { - val channel = channels[spinner.selectedItemPosition] - (requireActivity() as SlotSelectedListener).onSlotSelected(channel.slotId, channel.portId) + val slotId = slotIds[spinner.selectedItemPosition] + val portId = portIds[spinner.selectedItemPosition] + (requireActivity() as SlotSelectedListener).onSlotSelected(slotId, portId) dismiss() true } - - return view } override fun onResume() { diff --git a/app/src/main/java/im/angry/openeuicc/util/PrivilegedTelephonyUtils.kt b/app/src/main/java/im/angry/openeuicc/util/PrivilegedTelephonyUtils.kt index 4675ab9..6a13c50 100644 --- a/app/src/main/java/im/angry/openeuicc/util/PrivilegedTelephonyUtils.kt +++ b/app/src/main/java/im/angry/openeuicc/util/PrivilegedTelephonyUtils.kt @@ -15,12 +15,12 @@ val TelephonyManager.dsdsEnabled: Boolean get() = activeModemCount >= 2 fun TelephonyManager.setDsdsEnabled(euiccManager: EuiccChannelManager, enabled: Boolean) { - runBlocking { + val knownChannels = runBlocking { euiccManager.enumerateEuiccChannels() } // Disable all eSIM profiles before performing a DSDS switch (only for internal eSIMs) - euiccManager.knownChannels.forEach { + knownChannels.forEach { if (!it.removable) { it.lpa.disableActiveProfileWithUndo() }