[Replicant] [PATCH] Native Android WiFi with external dongle
Denis 'GNUtoo' Carikli
GNUtoo at cyberdimension.org
Thu Sep 19 03:20:19 UTC 2019
Hi,
I'm really sorry for the delay.
Thanks a lot for working on adding native WiFi support for external
dongles.
I've some comments on the patch below, in order to improve it (and send
a second version when this is done).
> Native Android WiFi with external dongle
Maybe something like would be better:
'Add native Android support for external WiFi dongles'
On Fri, 30 Aug 2019 18:26:58 +0000
Belgin ?tirbu <belginstirbu at hotmail.com> wrote:
> This patch allows choosing to use the proprietary firmware blob and
> internal WiFi device if the proprietary firmware blob is found.
> If proprietary firmware is not found, the patch causes Android's
> WiFi State Machine to try to use the external dongle.
I think that the 'blob' work is unnecessary.
Something like that could be better:
> With this patch, if the nonfree firmware is found, the internal WiFi
> is used, otherwise the WiFi state machine tries to use the external
> WiFi adapter.
> The problem with this patch is that Android's WiFi State Machine
> tries to enable Bluetooth and IP filters, causing errors in
> wpa_supplicant, and after 5 errors, wpa_supplicant thinks
> the driver hanged so it disconnects. The workaround to this
> is to add a boolean in the WifiStateMachine class that
> represents if the connection is via dongle, and avoid
> enabling Bluetooth and IP filters in case it is.
Here it doesn't try to enable Bluetooth. Instead it deactivates and
activate back a (Linux?) feature called Bluetooth Coexistence.
Hardware is complicated...
The issue is that WiFi and Bluetooth can use the same radio frequencies
(2.4GHz) and probably sometimes even the same antenna.
So if Both the WiFi and Bluetooth try to transmit at the same time,
they would interfere with each other, and the resulting signal being
transmitted would be of very bad quality.
So in Linux there is a mechanism to deal with that that is called
Bluetooth coexistance. There is more background information on it here:
https://wireless.kernel.org/en/users/Documentation/Bluetooth-coexistence
As for why it's enabled I've seen this commit inside the same
repository (frameworks/opt/net/wifi):
> commit 550b2696366dcad08c392c3617a18f70b6e7edf0
> Author: Nalla Kartheek <karthe at codeaurora.org>
> Date: Mon Jul 27 12:48:39 2015 +0530
>
> Wi-Fi: Set BTCOEXMODE_DISABLED irrespective of BT's connection
> state
> BTCOEXMODE_DISABLE has to be triggered during the DHCP phase even
> on an active BT connection to ensure that Wi-Fi is given
> preference over the BT.
> This commit ensures the same
>
> Change-Id: I000a79cc3f13c2a91287386558294f120ff54e78
However I am not familiar enough with the Linux WiFi stack to have more
insights than what the commit says.
> This is a workaround because the dongle might actually have
> Bluetooth, but I don't know what a proper fix to this would be.
Do you have a log of the errors you just mentioned above?
I've looked at 5.3, and the ath9k_htc driver has a btcoex_enable option
in its module. However I failed to figure out what was the
default (in a reasonable time) in the code as it's not set, and
module_param_named ends up using some compiler/linker annotations:
> __attribute__ ((unused,__section__
> ("__param"),aligned(sizeof(void *)))) \
> = { __param_str_##name, THIS_MODULE, ops,
> \ VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
But if I load the ath9k_htc module on 5.2, the default seem to be 0.
So we might also be able to workaround the issue by setting some kernel
parameter like ath9k_htc.btcoex_enable=1
> Signed-off-by: belginstirbu <belginstirbu at hotmail.com>
> ---
> .../com/android/server/wifi/WifiStateMachine.java | 134
> +++++++++++++-------- 1 file changed, 87 insertions(+), 47
> deletions(-)
>
> diff --git
> a/service/java/com/android/server/wifi/WifiStateMachine.java
> b/service/java/com/android/server/wifi/WifiStateMachine.java index
> 38d0ac8..2acda79 100644 ---
> a/service/java/com/android/server/wifi/WifiStateMachine.java +++
> b/service/java/com/android/server/wifi/WifiStateMachine.java @@
> -198,7 +198,8 @@ public class WifiStateMachine extends StateMachine
> implements WifiNative.WifiPno private ConnectivityManager mCm;
> private DummyWifiLogger mWifiLogger; private WifiApConfigStore
> mWifiApConfigStore;
> - private final boolean mP2pSupported;
> + private boolean mP2pSupported;
> + private boolean mDongleConnected;
> private boolean mIbssSupported;
> private final AtomicBoolean mP2pConnected = new
> AtomicBoolean(false); private boolean mTemporarilyDisconnectWifi =
> false; @@ -1155,6 +1156,8 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno mP2pSupported =
> mContext.getPackageManager().hasSystemFeature(
> PackageManager.FEATURE_WIFI_DIRECT);
> + mDongleConnected = false;
> +
> mWifiNative = new WifiNative(mInterfaceName);
> mWifiConfigStore = new WifiConfigStore(context,this,
> mWifiNative); mWifiAutoJoinController = new
> WifiAutoJoinController(context, this, @@ -5262,8 +5265,8 @@ public
> class WifiStateMachine extends StateMachine implements
> WifiNative.WifiPno private void
> handleSupplicantConnectionLoss(boolean killSupplicant) { /* Socket
> connection can be lost when we do a graceful shutdown
> - * or when the driver is hung. Ensure supplicant is stopped
> here.
> - */
> + * or when the driver is hung. Ensure supplicant is stopped
> here.
> + */
The lines look identical but they are not.
Previously there was 8 spaces before the '*' which was changed by a Tab
and a space. Replacing that with 8 spaces should make that change
disappear from the diff.
> if (killSupplicant) {
> mWifiMonitor.killSupplicant(mP2pSupported);
> }
> @@ -5274,27 +5277,31 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno
> void handlePreDhcpSetup() {
> mDhcpActive = true;
> - // Disable the coexistence mode
> - mWifiNative.setBluetoothCoexistenceMode(
> +
> + if(false == mDongleConnected)
Using 'if(mDongleConnected == false)' instead should look better as it
is closer to English.
> + {
> + // Disable the coexistence mode
> + mWifiNative.setBluetoothCoexistenceMode(
> mWifiNative.BLUETOOTH_COEXISTENCE_MODE_DISABLED);
>
> - // Disable power save and suspend optimizations during DHCP
> - // Note: The order here is important for now. Brcm driver
> changes
> - // power settings when we control suspend mode optimizations.
> - // TODO: Remove this comment when the driver is fixed.
> - setSuspendOptimizationsNative(SUSPEND_DUE_TO_DHCP, false);
> - mWifiNative.setPowerSave(false);
> + // Disable power save and suspend optimizations during
> DHCP
> + // Note: The order here is important for now. Brcm
> driver changes
> + // power settings when we control suspend mode
> optimizations.
> + // TODO: Remove this comment when the driver is fixed.
> + setSuspendOptimizationsNative(SUSPEND_DUE_TO_DHCP,
> false);
> + mWifiNative.setPowerSave(false);
> + }
>
> - // Update link layer stats
> - getWifiLinkLayerStats(false);
> + // Update link layer stats
> + getWifiLinkLayerStats(false);
Here the lines also look identical. Replacing the tab that is at the
beginning of the line by 8 spaces should remove that from the diff.
> - /* P2p discovery breaks dhcp, shut it down in order to get
> through this */
> - Message msg = new Message();
> - msg.what = WifiP2pServiceImpl.BLOCK_DISCOVERY;
> - msg.arg1 = WifiP2pServiceImpl.ENABLED;
> - msg.arg2 = DhcpStateMachine.CMD_PRE_DHCP_ACTION_COMPLETE;
> - msg.obj = mDhcpStateMachine;
> - mWifiP2pChannel.sendMessage(msg);
> + /* P2p discovery breaks dhcp, shut it down in order to get
> through this */
> + Message msg = new Message();
> + msg.what = WifiP2pServiceImpl.BLOCK_DISCOVERY;
> + msg.arg1 = WifiP2pServiceImpl.ENABLED;
> + msg.arg2 = DhcpStateMachine.CMD_PRE_DHCP_ACTION_COMPLETE;
> + msg.obj = mDhcpStateMachine;
> + mWifiP2pChannel.sendMessage(msg);
> }
Here the lines also look identical. Replacing the tab that is at the
beginning of the line by 8 spaces should remove that from the diff.
> @@ -5337,15 +5344,18 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno }
>
> void handlePostDhcpSetup() {
> - /* Restore power save and suspend optimizations */
> - setSuspendOptimizationsNative(SUSPEND_DUE_TO_DHCP, true);
> - mWifiNative.setPowerSave(true);
> + if(false == mDongleConnected)
Using 'if(mDongleConnected == false)' instead should look better as it
is closer to English.
> + {
> + /* Restore power save and suspend optimizations */
> + setSuspendOptimizationsNative(SUSPEND_DUE_TO_DHCP, true);
> + mWifiNative.setPowerSave(true);
>
> -
> mWifiP2pChannel.sendMessage(WifiP2pServiceImpl.BLOCK_DISCOVERY,
> WifiP2pServiceImpl.DISABLED);
> +
> mWifiP2pChannel.sendMessage(WifiP2pServiceImpl.BLOCK_DISCOVERY,
> WifiP2pServiceImpl.DISABLED);
> - // Set the coexistence mode back to its default value
> - mWifiNative.setBluetoothCoexistenceMode(
> + // Set the coexistence mode back to its default value
> + mWifiNative.setBluetoothCoexistenceMode(
> mWifiNative.BLUETOOTH_COEXISTENCE_MODE_SENSE);
> + }
>
> mDhcpActive = false;
> }
> @@ -6047,7 +6057,13 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno
> * Avoids issues with drivers that do not handle
> interface down
> * on a running supplicant properly.
> */
> +
> + mDongleConnected = false;
> +
The like above shows in red in 'git show HEAD' because it has no text
but there is invisible spaces and tabs. Removing the invisible spaces
and tab fixes that.
> mWifiMonitor.killSupplicant(mP2pSupported);
> +
Same here: There are also invisible spaces and tabs to remove.
> + mP2pSupported =
> mContext.getPackageManager().hasSystemFeature(
> + PackageManager.FEATURE_WIFI_DIRECT);
>
> if (mWifiNative.loadDriver()) {
> try {
> @@ -6096,7 +6112,23 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno }
> } else {
> loge("Failed to load driver");
Maybe that could be changed to something like:
'loge("Failed to load native driver");'
> - setWifiState(WifiManager.WIFI_STATE_FAILED);
> +
> + loge("Trying to load external wifi dongle");
And that could be improved with something like 'Now trying to load
[...]' or 'Trying to load [...] instead'.
> +
> + mP2pSupported = false;
> +
There is the same above: invisible spaces and tabs to remove.
> + mWifiMonitor.killSupplicant(mP2pSupported);
> +
And again, there is some invisible spaces and tabs to remove.
> + if
> (mWifiNative.startSupplicant(mP2pSupported)) {
> + setWifiState(WIFI_STATE_ENABLING);
> + if (DBG) log("Supplicant start
> successful");
> + mWifiMonitor.startMonitoring();
> + mDongleConnected = true;
> + transitionTo(mSupplicantStartingState);
> + } else {
> + loge("Failed to start supplicant!");
> +
> setWifiState(WifiManager.WIFI_STATE_FAILED);
> + }
> }
> break;
> case CMD_START_AP:
> @@ -6527,24 +6559,28 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno mInDelayedStop = false;
> mDelayedStopCounter++;
> updateBatteryWorkSource(null);
> - /**
> - * Enable bluetooth coexistence scan mode when bluetooth
> connection is active.
> - * When this mode is on, some of the low-level scan
> parameters used by the
> - * driver are changed to reduce interference with
> bluetooth
> - */
> -
> mWifiNative.setBluetoothCoexistenceScanMode(mBluetoothConnectionActive);
> - /* initialize network state */
> - setNetworkDetailedState(DetailedState.DISCONNECTED);
>
> - /* Remove any filtering on Multicast v6 at start */
> - mWifiNative.stopFilteringMulticastV6Packets();
> -
> - /* Reset Multicast v4 filtering state */
> - if (mFilteringMulticastV4Packets.get()) {
> - mWifiNative.startFilteringMulticastV4Packets();
> - } else {
> - mWifiNative.stopFilteringMulticastV4Packets();
> - }
> + if(false == mDongleConnected)
Using 'if(mDongleConnected == false)' instead should look better as it
is closer to English.
> + {
> + /**
> + * Enable bluetooth coexistence scan mode when
> bluetooth connection is active.
> + * When this mode is on, some of the low-level scan
> parameters used by the
> + * driver are changed to reduce interference with
> bluetooth
> + */
> +
> mWifiNative.setBluetoothCoexistenceScanMode(mBluetoothConnectionActive);
> + /* initialize network state */
> + setNetworkDetailedState(DetailedState.DISCONNECTED);
> +
> + /* Remove any filtering on Multicast v6 at start */
> + mWifiNative.stopFilteringMulticastV6Packets();
> +
> + /* Reset Multicast v4 filtering state */
> + if (mFilteringMulticastV4Packets.get()) {
> + mWifiNative.startFilteringMulticastV4Packets();
> + } else {
> + mWifiNative.stopFilteringMulticastV4Packets();
> + }
> + }
>
> mDhcpActive = false;
>
> @@ -6647,9 +6683,13 @@ public class WifiStateMachine extends
> StateMachine implements WifiNative.WifiPno }
> break;
> case CMD_BLUETOOTH_ADAPTER_STATE_CHANGE:
> - mBluetoothConnectionActive = (message.arg1 !=
> - BluetoothAdapter.STATE_DISCONNECTED);
> -
> mWifiNative.setBluetoothCoexistenceScanMode(mBluetoothConnectionActive);
> + if(false == mDongleConnected)
Using 'if(mDongleConnected == false)' instead should look better as it
is closer to English.
> + {
> + mBluetoothConnectionActive = (message.arg1 !=
> +
> BluetoothAdapter.STATE_DISCONNECTED);
> +
> mWifiNative.setBluetoothCoexistenceScanMode(mBluetoothConnectionActive);
> + }
> +
The like above shows in red in 'git show HEAD' because it has no text
but there is invisible spaces and tabs. Removing the invisible spaces
and tab fixes that.
Thanks a lot for the patch.
I'm looking forward for a second version with the comments above being
addressed.
Denis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.osuosl.org/pipermail/replicant/attachments/20190919/e392c405/attachment.asc>
More information about the Replicant
mailing list