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

Fixes in PolylineWalker #5135

Merged
merged 4 commits into from
Sep 3, 2016
Merged

Fixes in PolylineWalker #5135

merged 4 commits into from
Sep 3, 2016

Conversation

th3w4y
Copy link
Contributor

@th3w4y th3w4y commented Sep 3, 2016

Scenario:

  • when there is no cached Polyline we get a None back from the .get_alt()
  • when altitude is 0 we get 0 from .get_alt

Issue:

  • In polylineWalker I was checking if .get_alt() or uniform(min_alt, max_lat) that clearly fails when altitude is zero.

Fixes:

  • will fallback to read the previous position of the bot in the alt 0 or None case
  • set the speed in PolylineWalker and passthrough to StepWalker

Additional very Important Fixes:

  • Polyline Walker should return False if final destination not reached (might have teleported in some follow workers)
  • fixed cache validation.. (this was causing API quota reached)

@mention-bot
Copy link

@th3w4y, thanks for your PR! By analyzing the annotation information on this pull request, we identified @mjmadsen and @solderzzc to be potential reviewers

@th3w4y th3w4y changed the title Fix altitude 0 in PolylineWalker Fix altitude 0, and speed in PolylineWalker Sep 3, 2016
@mjmadsen
Copy link
Contributor

mjmadsen commented Sep 3, 2016

@th3w4y Thank you.

@th3w4y th3w4y changed the title Fix altitude 0, and speed in PolylineWalker Fixes in PolylineWalker Sep 3, 2016
step = super(PolylineWalker, self).step()
if not (distance(self.pol_lat, self.pol_lon, self.dest_lat, self.dest_lng) > 10 and step):
return False

Copy link
Contributor

@julienlavergne julienlavergne Sep 3, 2016

Choose a reason for hiding this comment

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

There is branch without any return statement.
10 meters is also a problem for me. There might be less than that between the last steps of the polyline. I would prefer we make sure we reached the final step of the polyline.

Copy link
Contributor Author

@th3w4y th3w4y Sep 3, 2016

Choose a reason for hiding this comment

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

I corrected it..

@mjmadsen
Copy link
Contributor

mjmadsen commented Sep 3, 2016

@th3w4y can you rebase? @solderzzc put some changes in to polyline_walker

@th3w4y
Copy link
Contributor Author

th3w4y commented Sep 3, 2016

@mjmadsen i rebased...

@julienlavergne
Copy link
Contributor

With these changes, now the problem is that movements are chaotics just like the screenshot of @solderzzc .
So there is still something to fix :P.

@th3w4y
Copy link
Contributor Author

th3w4y commented Sep 3, 2016

@anakin5 check last commit I believe this was causing the whole thing... 8f7f22c

This should fix also the @solderzzc screenshot he attached #5132

@julienlavergne
Copy link
Contributor

Yes looks better :)

@mjmadsen
Copy link
Contributor

mjmadsen commented Sep 3, 2016

👍

We agree this is in a working state now? :)

Approved with PullApprove

@julienlavergne
Copy link
Contributor

Yes, it is working fine

@mjmadsen mjmadsen merged commit e7a94ab into PokemonGoF:dev Sep 3, 2016
@th3w4y th3w4y deleted the fix_alt_zero branch September 4, 2016 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants