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 sensors support #555

Merged
merged 14 commits into from
Nov 16, 2019
Merged

Conversation

vrunoa
Copy link
Contributor

@vrunoa vrunoa commented Jul 27, 2019

Prepare Android driver to support sensors set for Emulators implemented on ADB
appium/appium-adb#456

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

left a comment, otherwise lgtm

lib/commands/actions.js Outdated Show resolved Hide resolved
test/unit/commands/actions-specs.js Show resolved Hide resolved
* @param {number|string} value - Number to set as the sensor value.
* @throws {Error} - If deviceType is not an emulator
*/
commands.sensorSet = async function sensorSet (sensorType, value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

which external API endpoint is supposed to trigger this method?

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 this is something I want to discuss with the community. We have two options here;

  1. using one endpoint per type of sensor. For ex; W3C already has a method for light sensor in progress. https://www.w3.org/TR/ambient-light/
  2. using one endpoint for all sensors.
    I'd like to hear you guys feedback. I'll probably start the conversation later today in slack

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say it is always safe to have a "mobile:" endpoint for such purpose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vrunoa vrunoa changed the title Add sensors support wip: Add sensors support Aug 18, 2019
@KazuCocoa
Copy link
Member

We should require ^7.9.0 in https://github.com/appium/appium-android-driver/blob/master/package.json#L39 for sensor

@mykola-mokhnach
Copy link
Contributor

LGTM. Do not forget to add the same mobile command to uia2 and espresso after this PR is published

@vrunoa vrunoa changed the title wip: Add sensors support Add sensors support Oct 29, 2019
@mykola-mokhnach mykola-mokhnach merged commit b095421 into appium:master Nov 16, 2019
@imurchie imurchie mentioned this pull request Dec 11, 2019
10 tasks
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