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

here geocoder url config parameter #4472

Merged
merged 8 commits into from
Jan 21, 2020
Merged

here geocoder url config parameter #4472

merged 8 commits into from
Jan 21, 2020

Conversation

jcardus
Copy link
Contributor

@jcardus jcardus commented Jan 20, 2020

Looks like it was hardcoded.

The here url has changed.

@jcardus jcardus closed this Jan 20, 2020
@jcardus
Copy link
Contributor Author

jcardus commented Jan 20, 2020

checkStyle failing...

@jcardus jcardus reopened this Jan 20, 2020
@jcardus jcardus requested a review from tananaev January 20, 2020 19:45
@tananaev
Copy link
Member

What's the new URL? I think we should set at least to be a default.

@jcardus
Copy link
Contributor Author

jcardus commented Jan 21, 2020

https://reverse.geocoder.api.here.com/6.2/reversegeocode.json

I put it on the unit test.

My api_key doesn't work with the old url.

By the way I'm having problems trying to fix the unit tests, It has to be run with the Conext initialized because of the "concrete map type error". But we need a settings file for that and I think it's too much for such a small test. Any idea on this?

@jcardus
Copy link
Contributor Author

jcardus commented Jan 21, 2020

sorry,
this is the new url:

https://reverse.geocoder.ls.hereapi.com/6.2/reversegeocode.json

return url;
private static String formatUrl(String url, String id, String key, String language) {
if (url == null) {
url = "https://reverse.geocoder.ls.hereapi.com/6.2/reversegeocode.json";
Copy link
Member

Choose a reason for hiding this comment

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

Please fix indentation here.

src/main/java/org/traccar/geocoder/HereGeocoder.java Outdated Show resolved Hide resolved
@@ -73,7 +73,7 @@ public void testBan() {
@Ignore
@Test
public void testHere() {
Geocoder geocoder = new HereGeocoder("", "", null, 0, new AddressFormat());
Geocoder geocoder = new HereGeocoder("https://reverse.geocoder.ls.hereapi.com/6.2/reversegeocode.json", "", "", null, 0, new AddressFormat());
Copy link
Member

Choose a reason for hiding this comment

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

URL here can be removed.

jcardus added 3 commits January 21, 2020 17:59
removed url from here geocode test
intellij uses 2 spaces, this project uses 4.
intellij uses 2 spaces, this project uses 4.
@@ -73,7 +73,7 @@ public void testBan() {
@Ignore
@Test
public void testHere() {
Geocoder geocoder = new HereGeocoder("", "", null, 0, new AddressFormat());
Geocoder geocoder = new HereGeocoder("", "", "", null, 0, new AddressFormat());
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right. Shouldn't the URL be null?

@tananaev tananaev merged commit 0c40b37 into traccar:master Jan 21, 2020
@tananaev
Copy link
Member

Merged, thanks.

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

2 participants