adding support for OSMScout Server

[rinigus]

Hi again!

I am preparing for the first release of the server. I would suggest to add the support for it into Poor Maps via this PR. The reason is rather simple: I would like to avoid getting into file ownership conflicts by writing files into /usr/share/harbour-poor-maps from harbour-osmscout-server install. Maybe one day we would like to submit into the Store and such cross-writing may result in further problems.

What do you think?

Best wishes,

rinigus

PS: its a joy to write these small modules for Poor Maps - very well documented and easy to write! Thank you for that.

[otsaloma]

I'd rather have seen these be a plugin RPM package that you maintain yourself. I'm not familiar with the "file ownership conflicts" you refer to, but the Harbour issue is difficult to get around and reason enough for me to take these into the Poor Maps codebase.

So, yes, I'll take them, but it also means I need to do some quality policing before we're OK to merge. GitHub gives me access to your branch, so I can tweak the small stuff myself, but a couple questions for you.

1. I don't want these to visible in Poor Maps unless the user has OSMScout Server installed. I assume I can check for the existance of /usr/bin/harbour-osmscout-server to determine this?
2. What's the larger and non-larger tiles? Is it 512x512 and 256x256? Is there any reason a user would not want large tiles? I assume large tiles minimize the total rendering time?
3. For the geocoder and guide your description includes ["type", "admin_region", "object_id"]. Can you provide an example of what it looks like for a real query? Does the OSMScout database format contain only one level of region? Does object_id have some relevance for the user?

[rinigus]

I do understand your concerns. And, in general, we don't have to do anything in a rush and its easy to change this PR or even reject it, if its the best. Right now maybe we should consider this PR as a way I can ask for your help in the developing the link between the server and Poor Maps :)

With the "file ownership problem" I met a scenario where we would decide to move from one distribution model to another. Like, we distribute these files as a plugin for Poor Maps and the plugin will be writing the code under /usr/share/harbour-poor-maps. If later, it would be decided to merge it under Poor Maps proper, we suddenly have to deal with the devices where the same file is owned by two different RPMs (well, it would be in conflict, I guess).

No worries with the policing - that's expected. So, replies below:

1. You assume correctly. If you would start checking for other file, it may get against Store policies. RPM validator might consider that its an additional dependency and would reject you.
2. At present, I don't know what's an optimal tile size. On my device (Nexus 4), I use 1024x1024
3. Good question regarding object_id. Not sure its relevant. Examples attached.

I will try to release the server's first version this week. That release will be tailored towards map app developers, so you could try it yourself. I plan to release supporting libraries separately - so, no go with the Store (at least for now). examples.zip

best wishes,

rinigus

[otsaloma]

I do understand your concerns. And, in general, we don't have to do anything in a rush and its easy to change this PR or even reject it, if its the best.

While I like to yearn for elegant solutions, best is better than none, and I think there is much demand, so it's fine.

With the "file ownership problem" I met a scenario where we would decide to move from one distribution model to another.

OK, I understand. With traditional package management you'd just define a versioned conflict with the other package, but that too wouldn't work in Harbour or any simple tap-to-install context.

At present, I don't know what's an optimal tile size. On my device (Nexus 4), I use 1024x1024.

I'd rather not present the user with a choice they cannot be expected to understand. It will lead to e.g. not remembering which size they used last, which leads to not making use of previously rendered and cached tiles. I'd rather have us pick one size, or label them something understandable such as "phone" and "tablet". Once you make a release, I'll do some testing on the Jolla 1 and tablet before we make a decision.

Good question regarding object_id. Not sure its relevant. Examples attached.

It looks like title and admin_region are the only ones needed. I'd recommend small changes to your server-side code: keep the title short and no duplication between title and region. If the region is a string, then rather comma-separated, which is the usual display format. Alternatively, make it an array of the individual components.

{
    "title": "Raekoja",
    "admin_region": "Kesklinna linnaosa, Tallinna linn, Harju maakond, Eesti",
}

[rinigus]

On Tue, Oct 11, 2016 at 1:13 AM, Osmo Salomaa notifications@github.com wrote:

I do understand your concerns. And, in general, we don't have to do anything in a rush and its easy to change this PR or even reject it, if its the best.

While I like to yearn for elegant solutions, best is better than none, and I think there is much demand, so it's fine.

Maybe I did rush with this as PR. But let's see where it will bring us.

I'd rather not present the user with a choice they cannot be expected to

understand. It will lead to e.g. not remembering which size they used last, which leads to not making use of previously rendered and cached tiles. I'd rather have us pick one size, or label them something understandable such as "phone" and "tablet". Once you make a release, I'll do some testing on the Jolla 1 and tablet before we make a decision.

Agreed! In addition to performance there are also issues with labeling rendering that sometimes happen at the border of the tiles. The larger the tile, the less issues we have with labeling :) . But yes, tile size does require testing.

As you could see, the server has a flag "daylight" which can be either 0 or (larger than zero, 1 for example). In future, would be great to have some internal Poor Maps variable that is fed to the tile servers which could allow to switch the versions. That would be handy if you use Poor Maps for GPS navigation in a car. Surely, it is not trivial with the tile cache having to keep track on the vars, but it maybe worthwhile to think about it. For now, having two tile server configurations should be fine, though.

Good question regarding object_id. Not sure its relevant. Examples attached.

It looks like title and admin_region are the only ones needed. I'd recommend small changes to your server-side code: keep the title short and no duplication between title and region. If the region is a string, then rather comma-separated, which is the usual display format. Alternatively, make it an array of the individual components.

{ "title": "Raekoja", "admin_region": "Kesklinna linnaosa, Tallinna linn, Harju maakond, Eesti", }

I guess I would leave the object id in the server response, but we can ignore and not display it.

Comma separation: I would change it tonight.

Title: I'll file the issue on server's repo and let's see what I can do. We have to consider the cases where you look for an address (Streetname Housename) and would end up with the same name from different cities. By having Streetname Housename, City in the title helps to find the correct one easier. Maybe we should return to it later when you have a chance to test the current version on your devices?

Best wishes,

rinigus

[otsaloma]

Title: I'll file the issue on server's repo and let's see what I can do. We have to consider the cases where you look for an address (Streetname Housename) and would end up with the same name from different cities. By having Streetname Housename, City in the title helps to find the correct one easier. Maybe we should return to it later when you have a chance to test the current version on your devices?

Yes, we can return to that later. The title and admin_region division and duplication is not a blocker anyway.

[rinigus]

But let's wait for few days and then you would be able to test this PR against the server. There is one problem that I have seen already - Poor Maps does not contact the server if I search for the same Nearby locations as I have done already on one session. Maybe I am using cache variable wrong in my plugins? Tried to follow your code, but there might be some mistake there...

[otsaloma]

Poor Maps does not contact the server if I search for the same Nearby locations as I have done already on one session.

Yes, that's how the cache works. If you try to do the exact same search again, it will return the cached result, since it's a lot faster and also cheaper for service providers that count API calls and charge based on that.

If in your case, the data on disk might change between calls and thus the result, then it's probably best to just not use caching. Caching is probably not that relevant for a localhost server anyway, unless the database is somehow slow.

[rinigus]

I found the bug with the cache in the guide. But maybe I should comment them out. Let's see a bit later how fast is access for bigger countries.

[otsaloma]

I added a system to check the provider requirements and made some other small changes. Check that I didn't change anything for the worse. A couple notes:

• Your naming was inconsistent. I changed the name to "OSM Scout" as that looks human-readable to me. That's of course up to you -- if you prefer "OSMScout" or "OSM scout", make the change, but be consistent.
• In your ZIP-package of example results, the guide doesn't seem to return admin_region, but the provider code was and is set to parse it. I hope it will be included?
• I'll check the geocoder and guide descriptions better once I get to actual testing with actual data.
• The source field in those JSON files has been to indicate the service provider. The distinction is made because the name doesn't always imply it and in many cases the service provider doesn't request attribution and that goes only to OpenStreetMap. So, I changed to source to "OSM Scout offline server".

[rinigus]

I added a system to check the provider requirements and made some other small changes. Check that I didn't change anything for the worse. A couple notes:

-

Your naming was inconsistent. I changed the name to "OSM Scout" as that looks human-readable to me. That's of course up to you -- if you prefer "OSMScout" or "OSM scout", make the change, but be consistent.

Thank you for the suggestion! I'll make the change and use OSM Scout Server :)

-

In your ZIP-package of example results, the guide doesn't seem to return admin_region, but the provider code was and is set to parse it. I hope it will be included?

Yes, the admin_region is not included since I don't know how to do it (or how to do it fast without major search in the database). So, the patch to geocoder would break things, unfortunately. I am still rather new to libosmscout and it takes time to learn it. So, for now, I'll keep as it is and would write up an issue regarding it in github. Fortunately, the interest to the server seems to be large and I would probably get help on implementing it. Right now, I need to get it out of the door to let you and other developers test and see it in action.

-

I'll check the geocoder and guide descriptions better once I get to actual testing with actual data.

Thanks! I am preparing the packages right now.

-

The source field in those JSON files has been to indicate the service provider. The distinction is made because the name doesn't always imply it and in many cases the service provider doesn't request attribution and that goes only to OpenStreetMap. So, I changed to source to "OSM Scout offline server".

Thank you very much for all these changes!

rinigus

[rinigus]

Dear @otsaloma ,

I finally released the first version to OpenRepos, so you could try it. I very much hope it will work as intended :) .

I will start looking into routing now to be able to cover all the basic functionality. Then I could run through all functionality again and enhance it.

Thank you for your help!

rinigus

[otsaloma]

I noticed. Thanks. I should have time to test this weekend.

[otsaloma]

I added graceful error handling for the sometimes missing admin_region field. Don't mind the Travis CI failure, that's due to another provider's servers failing.

I wrote more detailed comments at TMO, assuming others are testing as well and would benefit from a more public discussion.

My take at the moment is that I'd like to see the large tile size be the only one. Do you agree? Once that's fixed, I could merge this. The rest of the issues I noticed during testing don't affect these provider files, but are for you to fix at your own pace on the server side.

[rinigus]

Re TMO: Excellent idea! I'll reply through that thread as well.

Re size: yes, I think we could just keep large size. I am using the one with the scale 4 and @2. So, it's 1024x1024.

Re admin_region: I'll explain it in TMO. [a bit later today]

In the search description you don't use type of the found object. I'd suggest to add that as well. While not pretty, it gives you an idea on what was found. For example, search could return with the same name place_village, wood, or waterway_stream. Maybe the same processing as in Guide can be added here as well? This type is used while searching for POI (a substring of inserted POI type is searched among these types, in case insensitive manner).

[otsaloma]

I made the changes. The diff on the tiles came out weird, but basically I just removed the regular and renamed large.

Is it OK by you now? I'd merge it in and make a release maybe next weekend.

[rinigus]

I'll look through it and your suggestions late tonight. Sorry for delay

rinigus

On Sun, Oct 16, 2016 at 5:13 PM, Osmo Salomaa notifications@github.com wrote:

I made the changes. The diff on the tiles came out weird, but basically I just removed the regular and renamed large.

Is it OK by you now? I'd merge it in and make a release maybe next weekend.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/otsaloma/poor-maps/pull/21#issuecomment-254049339, or mute the thread https://github.com/notifications/unsubscribe-auth/AO-p7RsGi-yH6AxHX66ZF-40Y45jBjivks5q0jD8gaJpZM4KSGJ9 .

[rinigus]

Sorry, have to postpone it. Would do it tomorrow when I can. Since we have few days before your planned release, I hope its OK.

[otsaloma]

Yes, it's fine, no hurry.

[rinigus]

I managed to test it and it all looks fine. Thank you very much for adding it into Poor Maps!