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

feat: Add support of extended Android geolocation #1492

Merged
merged 2 commits into from
Jul 1, 2021

Conversation

mykola-mokhnach
Copy link
Contributor

Change list

Adds an API that allows to also provide speed and satellites parameters for setGeolocation API on Android platform

Types of changes

  • 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)

Details

Based on appium/appium-adb#571

@mykola-mokhnach mykola-mokhnach merged commit a1e1d1d into appium:master Jul 1, 2021
@mykola-mokhnach mykola-mokhnach deleted the geo_loc branch July 1, 2021 18:52
@mykola-mokhnach
Copy link
Contributor Author

FYI @KazuCocoa @ki4070ma

@dr29bart
Copy link
Contributor

dr29bart commented Oct 6, 2021

Any plans to release a new java-client with this functionality?
cc @SrinivasanTarget

@SrinivasanTarget
Copy link
Member

@dr29bart will release to maven central tomorrow. You can till then still use latest commit through jitpack.io.

* @param longitude longitude value
* @param latitude latitude value
*/
public AndroidGeoLocation(double longitude, double latitude) {
Copy link
Contributor

@dr29bart dr29bart Oct 6, 2021

Choose a reason for hiding this comment

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

@mykola-mokhnach does it make sense to match order of parameters with org.openqa.selenium.html5.Location

 public Location(double latitude, double longitude, double altitude) {
    this.latitude = latitude;
    this.longitude = longitude;
    this.altitude = altitude;
  }

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this is needed as altitude is optional. And for optional args I'd prefer to use builder methods

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I meant order of latitude and then longitude parameters.
existing code in 7.5.1

 driver.setLocation(new Location(gps.getLatitude(), gps.getLongitude(), 240));

new version will require parameters swap:

driver.setLocation(new AndroidGeoLocation(gps.getLongitude(), gps.getLatitude());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd say it is easy to change this while it's not published yet. Let us know if you want to prepare a PR @dr29bart

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.

4 participants