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

Add emulator gsm methods #228

Merged
merged 13 commits into from May 10, 2017
Merged

Add emulator gsm methods #228

merged 13 commits into from May 10, 2017

Conversation

vrunoa
Copy link
Contributor

@vrunoa vrunoa commented May 6, 2017

Add GSM methods supported by emulators

  • gsmCall
  • gsmSignal
  • gsmVoice

with this methods we can emulate phone calls, change the signal strength and the state of the gprs connection.

Copy link
Contributor

@imurchie imurchie left a comment

Choose a reason for hiding this comment

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

Wow! Awesome!

};

emuMethods.gsmSignal = async function (strength = 4) {
if (!(strength in [0, 1, 2, 3, 4])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Everything else is constants, maybe this array should be too?

@@ -23,6 +23,7 @@ const GSM_VOICE_STATES = [
GSM_VOICE_DENIED, GSM_VOICE_OFF,
GSM_VOICE_ON
];
const GSM_SIGNAL_STRENGTH = [0, 1, 2, 3, 4];
Copy link
Contributor

Choose a reason for hiding this comment

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

GSM_SIGNAL_STRENGTHS might be better to show this is an array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mykola-mokhnach what do you mean ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add an S to the end of the name.

};

emuMethods.gsmSignal = async function (strength = 4) {
if (!(strength in GSM_SIGNAL_STRENGTH)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it is better to use a single approach - either indexOf like above or in

@@ -43,4 +65,45 @@ emuMethods.sendSMS = async function (phoneNumber, message = '') {
await this.adbExecEmu(['sms', 'send', phoneNumber, message]);
};

emuMethods.gsmCall = async function (phoneNumber, action = '') {
if (GSM_CALL_ACTIONS.indexOf(action) === -1) {
log.errorAndThrow(`Invalid gsm action param ${action}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be useful to show the list of supported values in these error messages. This will help the client to put the correct value.

await this.adbExecEmu(['gsm', 'voice', state]);
};

emuMethods.GSM_CALL = GSM_CALL;
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any point to have duplicated constant declarations?

Copy link
Contributor

Choose a reason for hiding this comment

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

we might define all this like const emuMethods = {GSM_CALL: 'call' ...} and then use emuMethods.GSM_CALL... values in other constant declarations

@@ -77,7 +73,7 @@ emuMethods.gsmCall = async function (phoneNumber, action = '') {
};

emuMethods.gsmSignal = async function (strength = 4) {
if (!(strength in GSM_SIGNAL_STRENGTH)) {
if (!(strength in emuMethods.GSM_SIGNAL_STRENGTHS)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

almost perfect. Only one small comment left - usage of indexOf/in.

}
await this.adbExecEmu(['gsm', action, phoneNumber]);
};

emuMethods.gsmSignal = async function (strength = 4) {
if (!(strength in emuMethods.GSM_SIGNAL_STRENGTHS)) {
strength = parseInt(strength, 10);
if (emuMethods.GSM_SIGNAL_STRENGTHS.indexOf(strength) === -1) {
log.errorAndThrow(`Invalid signal strength param ${strength}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I mean the error message could be changed to

Invalid signal strength param ${strength}. Supported values: ${_.values(emuMethods.GSM_SIGNAL_STRENGTHS)}

@@ -109,5 +109,188 @@ describe('adb emulator commands', () => {
mocks.adb.verify();
});
}));
describe("gsm signal method", withMocks({adb}, (mocks) => {
it("should throw exception on invalid strength", async () => {
await adb.gsmSignal(5).should.eventually.be.rejectedWith("Invalid signal strength");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rejectedWith also supports regexp patterns

let emuMethods = {};
emuMethods.GSM_CALL_ACTIONS = {
'GSM_CALL' : 'call',
Copy link
Contributor

Choose a reason for hiding this comment

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

As with the other, no need to quote keys in object literal syntax, unless there is some special character (e.g., 'GSM-CALL' would need them, GSM_CALL does not).

@imurchie imurchie merged commit a9de860 into appium:master May 10, 2017
@imurchie
Copy link
Contributor

Published in appium-adb@2.20.1.

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.

None yet

3 participants