Add handlers for gsm, network, power and sms command by SrinivasanTarget · Pull Request #834

Add handlers for gsm, network, power and sms command by SrinivasanTarget · Pull Request #834 · appium/java-client

Conversation

Change list

Add handlers for gsm, network, power and sms command
https://github.com/appium/appium-base-driver/blob/master/lib/protocol/routes.js#L366-L375

Types of changes

What types of changes are you proposing/introducing to Java client?
Put an x in the boxes that apply

No changes in production code.
Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

@mykola-mokhnach ping


		private final String gsmcall;

		GsmCallActions(String gsmcall) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this constructor is redundant. All enum elements have name() and ordinal() properties. That's pretty all you need. Same comment to other enums

		CommandExecutionHelper.execute(this, toggleLocationServicesCommand());
		}

		/**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather put these methods into separate interface named as SupportsSpecialEmulatorCommands

		*
		* @return a key-value pair. The key is the command name. The value is a {@link Map} command arguments.
		*/
		public static Map.Entry<String, Map<String, ?>> powerACCommand(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these methods have one extra space between the return type and the method name

		/**
		* Emulate power state change on the connected emulator.
		*
		* @param powerACState One of available Power AC state.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather put a direct link to the corresponding enum here

It looks like we need to add at least one assert to each test to make Codacy happy

Yeah but i'm not sure what to assert here? Do you have any better suggestion? @mykola-mokhnach

try {
  smth();
} catch (Exception e) {
  fail(e)
}

try maybe this workaround

		try {
		driver.sendSMS("11111111", "call");
		} catch (Exception e) {
		assertFalse( "method works only in emulators", true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use fail call instead

		* @param phoneNumber The phone number of the caller.
		* @param gsmCallActions One of available {@link GsmCallActions} values.
		*/
		default void setGsmCall(String phoneNumber, GsmCallActions gsmCallActions) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makeGsmCall would be a better name

		* @param gsmSignalStrength One of available {@link GsmSignalStrength} values.
		*/
		default void setGsmSignalStrength(GsmSignalStrength gsmSignalStrength) {
		CommandExecutionHelper.execute( this, gsmSignalStrengthCommand(gsmSignalStrength));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space before this

Conversation

Change list

Types of changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2 participants