Refactor network connection setting on Android #865
Conversation
|
|
||
| /** | ||
| * 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.
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.
wifi should work. for data there is a comment, that it only works on emulator and on rooted devices
| * @return true if airplane mode is enabled. | ||
| */ | ||
| public boolean isAirplaneModeEnabled() { | ||
| return (bitMask & AIRPLANE_MODE_MASK) != 0; |
There was a problem hiding this comment.
It seems that all these bitwise operations can be simplified via java.utils.BitSet
There was a problem hiding this comment.
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.
ok, np. However from my point of view bs.set(AIRPLANE_MODE_BIT, false) looks more naturaly than this.bitMask = bitMask & ~AIRPLANE_MODE_MASK; (Maybe I just don't like bitwise logic :)
| .withAirplaneModeDisabled() | ||
| .build()); | ||
| ConnectionState state = driver.getConnection(); | ||
| assertTrue(!state.isAirplaneModeEnabled() && !state.isWiFiEnabled() && !state.isDataEnabled()); |
There was a problem hiding this comment.
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.
Here also we might need to remove the additional checks.
There was a problem hiding this comment.
These I'd prefer to keep
|
Rewieving... |
There was a problem hiding this comment.
I have checked these changes on an emulator of Android 6. Tests were passed. The design is looking good to me.
@SrinivasanTarget if you are agree so lets merge it.
Change list
With the previous implementation it was only possible to enable connections, but not disable. I think the current approach will make the feature much more flexible than it was before.
Types of changes