Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix getting the SSID via shell #494

Merged
merged 2 commits into from
Dec 14, 2024
Merged

Conversation

markozajc
Copy link
Contributor

I've fixed the command to get the current SSID via the root shell. Previously it was using the last line of the output, which in my case was a remembered network name, but not the currently connected one. I've tested the old and new commands on a few smartphones (via adb):

Old New
Samsung, 4.1.2
Sony, 4.1.2 ✔️
Huawei, 8.1.0 ✔️
LineageOS, 12.0 ✔️

Please keep in mind: 708b4c7 introduces a regression that prevents root SSID functionality from working entirely. I've figured out that isWifiNameByShellEnabled in getWifiSSID() is false with this change applied and true without it, but wasn't able to figure out why. Removing .filterNot { it.isWifiConnected && it.wifiName == null } entirely fixes the issue, but I did not include it in this PR because it likely undoes the fix for #472. I can open a new issue for this if you'd like.

@zaneschepke
Copy link
Owner

zaneschepke commented Dec 14, 2024

Hey! Thanks for this! I tested it and it seems to be working well. I'm not sure I follow on isWifiNameByShellEnabled regression. This is a setting on the auto-tunnel screen that is used to enabled this feature. I tested this setting as well and it seems to be working as expected. I'll merge this now and we can continue the discussion on this regression here.

@zaneschepke zaneschepke self-requested a review December 14, 2024 16:18
@zaneschepke zaneschepke merged commit b81f43c into zaneschepke:main Dec 14, 2024
@markozajc
Copy link
Contributor Author

When debugging this functionality, I placed a log statement above the isWifiNameByShellEnabled check in getWifiSSID:

private suspend fun getWifiSSID(networkCapabilities: NetworkCapabilities): String? {
    return withContext(ioDispatcher) {
        with(autoTunnelStateFlow.value.settings) {
            Timber.w("isWifiNameByShellEnabled: " + isWifiNameByShellEnabled); // added
            if (isWifiNameByShellEnabled) return@withContext rootShell.get().getCurrentWifiName()
            wifiService.getNetworkName(networkCapabilities)
        }.also {
            if (it?.contains(Constants.UNREADABLE_SSID) == true) {
                Timber.w("SSID unreadable: missing permissions")
            } else {
                Timber.i("Detected valid SSID")
            }
        }
    }
}

I then ran adb logcat | grep isWifiNameByShellEnabled. I noticed that when enabling Auto-tunneling, this would always print false for the first check:

W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: false
W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: true
W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: true
W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: true

Sometimes it would also print true a few times and immediately disable the tunnel, while other times it would only print false once and keep the tunnel on:

W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: false

I bisected the problem and found out that 708b4c7 is the first bad commit. Indeed, when I removed the filterNot call, the problem would be gone:

private fun combineNetworkEventsJob(): Flow<NetworkState> {
    return combine(
        wifiService.networkStatus,
        mobileDataService.networkStatus,
        ethernetService.networkStatus,
    ) { wifi, mobileData, ethernet ->
        NetworkState(
            wifi.isConnected,
            mobileData.isConnected,
            ethernet.isConnected,
            when (wifi) {
                is NetworkStatus.CapabilitiesChanged -> getWifiSSID(wifi.networkCapabilities)
                is NetworkStatus.Available -> autoTunnelStateFlow.value.networkState.wifiName
                is NetworkStatus.Unavailable -> null
            },
        )
    }.distinctUntilChanged() // changed
}

The log would look like this (sometimes only one line, sometimes two):

W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: true
W AutoTunnelService$getWifiSSID: isWifiNameByShellEnabled: true

The tunnel still turns on at first, but always turns off right after.

I tried figuring out why this happens, but I don't understand Room, which seems responsible for loading the settings, well enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants