-
-
Notifications
You must be signed in to change notification settings - Fork 764
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
Refactor network connection setting on Android #865
Changes from 2 commits
90e8902
d7eeb61
5eaa7cc
99c0f7e
90dd25b
28ea442
a78d5e5
2cec8d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* See the NOTICE file distributed with this work for additional | ||
* information regarding copyright ownership. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package io.appium.java_client.android.connection; | ||
|
||
public class ConnectionState { | ||
public static final int AIRPLANE_MODE_MASK = 0b001; | ||
public static final int WIFI_MASK = 0b010; | ||
public static final int DATA_MASK = 0b100; | ||
|
||
private final int bitMask; | ||
|
||
public int getBitMask() { | ||
return bitMask; | ||
} | ||
|
||
public ConnectionState(int bitMask) { | ||
this.bitMask = bitMask; | ||
} | ||
|
||
/** | ||
* @return true if airplane mode is enabled. | ||
*/ | ||
public boolean isAirplaneModeEnabled() { | ||
return (bitMask & AIRPLANE_MODE_MASK) != 0; | ||
} | ||
|
||
/** | ||
* @return true if Wi-Fi connection is enabled. | ||
*/ | ||
public boolean isWiFiEnabled() { | ||
return (bitMask & WIFI_MASK) != 0; | ||
} | ||
|
||
/** | ||
* @return true if data connection is enabled. | ||
*/ | ||
public boolean isDataEnabled() { | ||
return (bitMask & DATA_MASK) != 0; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
/* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* See the NOTICE file distributed with this work for additional | ||
* information regarding copyright ownership. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package io.appium.java_client.android.connection; | ||
|
||
import static io.appium.java_client.android.connection.ConnectionState.AIRPLANE_MODE_MASK; | ||
import static io.appium.java_client.android.connection.ConnectionState.DATA_MASK; | ||
import static io.appium.java_client.android.connection.ConnectionState.WIFI_MASK; | ||
|
||
public class ConnectionStateBuilder { | ||
private int bitMask; | ||
|
||
/** | ||
* Initializes connection state builder with the default value (all off). | ||
*/ | ||
public ConnectionStateBuilder() { | ||
this.bitMask = 0; | ||
} | ||
|
||
/** | ||
* Initializes connection state builder with the the predefined bit mask. | ||
* This constructor might be handy to change an existing connection state. | ||
* | ||
* @param bitMask the actual initial state bit mask to set | ||
*/ | ||
public ConnectionStateBuilder(int bitMask) { | ||
this.bitMask = bitMask; | ||
} | ||
|
||
/** | ||
* Initializes connection state builder with the the predefined bit mask. | ||
* This constructor might be handy to change an existing connection state. | ||
* | ||
* @param state the actual initial state to set | ||
*/ | ||
public ConnectionStateBuilder(ConnectionState state) { | ||
this(state.getBitMask()); | ||
} | ||
|
||
/** | ||
* Sets airplane mode to enabled state if it was disabled. | ||
* This option only works up to Android 6. | ||
* Enabling the airplane mode on the device will automatically | ||
* disable Wi-Fi and data connections. | ||
* | ||
* @return self instance for chaining | ||
*/ | ||
public ConnectionStateBuilder withAirplaneModeEnabled() { | ||
this.bitMask = bitMask | AIRPLANE_MODE_MASK; | ||
return this; | ||
} | ||
|
||
/** | ||
* Sets airplane mode to disabled state if it was enabled. | ||
* This option only works up to Android 6. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If i remember right, even data and wifi through set/get connection wasn't working with Android 7. Also i thought retrieving connection state was only possible through shell. may be i'm wrong. Let me try this today. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wifi should work. for data there is a comment, that it only works on emulator and on rooted devices |
||
* | ||
* @return self instance for chaining | ||
*/ | ||
public ConnectionStateBuilder withAirplaneModeDisabled() { | ||
this.bitMask = bitMask & ~AIRPLANE_MODE_MASK; | ||
return this; | ||
} | ||
|
||
/** | ||
* Sets Wi-Fi connection mode to enabled state if it was disabled. | ||
* | ||
* @return self instance for chaining | ||
*/ | ||
public ConnectionStateBuilder withWiFiEnabled() { | ||
this.bitMask = bitMask | WIFI_MASK; | ||
return this; | ||
} | ||
|
||
/** | ||
* Sets Wi-Fi connection mode to disabled state if it was enabled. | ||
* | ||
* @return self instance for chaining | ||
*/ | ||
public ConnectionStateBuilder withWiFiDisabled() { | ||
this.bitMask = bitMask & ~WIFI_MASK; | ||
return this; | ||
} | ||
|
||
/** | ||
* Sets data connection mode to enabled state if it was disabled. | ||
* This option only works on rooted devices or on emulators. | ||
* | ||
* @return self instance for chaining | ||
*/ | ||
public ConnectionStateBuilder withDataEnabled() { | ||
this.bitMask = bitMask | DATA_MASK; | ||
return this; | ||
} | ||
|
||
/** | ||
* Sets data connection mode to disabled state if it was enabled. | ||
* This option only works on rooted devices or on emulators. | ||
* | ||
* @return self instance for chaining | ||
*/ | ||
public ConnectionStateBuilder withDataDisabled() { | ||
this.bitMask = bitMask & ~DATA_MASK; | ||
return this; | ||
} | ||
|
||
/** | ||
* Builds connection state instance, which is ready to be passed as Appium server parameter. | ||
* | ||
* @return ConnectionState instance | ||
*/ | ||
public ConnectionState build() { | ||
return new ConnectionState(bitMask); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,10 @@ | |
|
||
package io.appium.java_client.android; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertTrue; | ||
|
||
import io.appium.java_client.android.connection.ConnectionState; | ||
import io.appium.java_client.android.connection.ConnectionStateBuilder; | ||
import org.junit.FixMethodOrder; | ||
import org.junit.Test; | ||
import org.junit.runners.MethodSorters; | ||
|
@@ -26,23 +28,32 @@ | |
public class AndroidConnectionTest extends BaseAndroidTest { | ||
|
||
@Test public void test1() { | ||
driver.setConnection(Connection.WIFI); | ||
assertEquals(Connection.WIFI, | ||
driver.getConnection()); | ||
driver.setConnection(new ConnectionStateBuilder() | ||
.withWiFiEnabled() | ||
.build()); | ||
assertTrue(driver.getConnection().isWiFiEnabled()); | ||
} | ||
|
||
@Test public void test2() { | ||
driver.setConnection(Connection.NONE); | ||
assertEquals(Connection.NONE, | ||
driver.getConnection()); | ||
driver.setConnection(Connection.AIRPLANE); | ||
assertEquals(Connection.AIRPLANE, | ||
driver.getConnection()); | ||
driver.setConnection(new ConnectionStateBuilder() | ||
.withAirplaneModeDisabled() | ||
.build()); | ||
ConnectionState state = driver.getConnection(); | ||
assertTrue(!state.isAirplaneModeEnabled() && !state.isWiFiEnabled() && !state.isDataEnabled()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. check for wifi and data is not necessary i believe as disabling airplane mode will only disable airplane mode incase if it was already enabled and leave's the rest of connection as is. Isn't it? Enabling airplane mode will disable both data and wifi but disabling airplane will not disable/enable rest. Correct me if i'm wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here also we might need to remove the additional checks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These I'd prefer to keep |
||
driver.setConnection(new ConnectionStateBuilder() | ||
.withAirplaneModeEnabled() | ||
.build()); | ||
state = driver.getConnection(); | ||
assertTrue(state.isAirplaneModeEnabled() && !state.isWiFiEnabled() && !state.isDataEnabled()); | ||
} | ||
|
||
@Test public void test3() { | ||
driver.setConnection(Connection.ALL); | ||
assertEquals(Connection.ALL, | ||
driver.getConnection()); | ||
driver.setConnection(new ConnectionStateBuilder() | ||
.withAirplaneModeDisabled() | ||
.withWiFiEnabled() | ||
.withDataEnabled() | ||
.build()); | ||
ConnectionState state = driver.getConnection(); | ||
assertTrue(!state.isAirplaneModeEnabled() && state.isWiFiEnabled() && state.isDataEnabled()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that all these bitwise operations can be simplified via
java.utils.BitSet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you find the current implementation not readable enough? I think BitSet usage might add an unnecessary overhead, since the actual count of bitwise operations here is pretty limited anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, np. However from my point of view
bs.set(AIRPLANE_MODE_BIT, false)
looks more naturaly thanthis.bitMask = bitMask & ~AIRPLANE_MODE_MASK;
(Maybe I just don't like bitwise logic :)