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

implement arrival time in url #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrisluedtke
Copy link

I opened an issue about this, but the fix was quite simple. Previously, I did not see any arrival time input to the "maps.googleapis.com/maps/api/distancematrix/xml?origins=" construction.

@Demetrio92
Copy link
Collaborator

Demetrio92 commented Jul 9, 2018

Hi @chrisluedtke, sorry for a late reply :D
I checked your code and there is an issue with it.

The API string cannot be fixed this way b/c either departure OR arrival can be provided, but not both.
From the Docs

arrival_time — Specifies the desired time of arrival for transit directions, in seconds since midnight, January 1, 1970 UTC. You can specify either departure_time or arrival_time, but not both. Note that arrival_time must be specified as an integer.

Please fix it, and add the check into the function, right now it just leads to an erroneous behaviour.
Here is a testcase for you:

library(gmapsdistance)

t <- c("00:30:00", "01:30:00","02:30:00","03:30:00","04:30:00","05:30:00","06:30:00","07:30:00","08:30:00","09:30:00","10:30:00","11:30:00","12:30:00","13:30:00","14:30:00","15:30:00","16:30:00","17:30:00","18:30:00","19:30:00","20:30:00","21:30:00","22:30:00","23:30:00")
x <- c()
for (i in t){
  results_new = gmapsdistance(
    origin = "32.79405+34.98957",
    destination = "32.0853+34.78177",
    mode = "driving",
    # dep_date = "2019-07-04",
    # dep_time = i,
    arr_date = "2019-07-04",
    arr_time = i,
    key = 'YOUR_KEY_HERE',
    traffic_model = 'pessimistic'
  )
  x <- c(x,results_new$Time )
}
x

See what's the difference between departure and arrival.

P.s. if you want to help with the fix, please pull the master again, there are some changes, and especially because of the whitespace merge isn't working perfectly.

@Demetrio92 Demetrio92 mentioned this pull request Jul 9, 2018
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.

2 participants