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

Map management #185

Merged
merged 30 commits into from Dec 17, 2016
Merged

Map management #185

merged 30 commits into from Dec 17, 2016

Conversation

Karry
Copy link
Collaborator

@Karry Karry commented Dec 11, 2016

Hi Tim, sorry for this huge MR (again), but single class don't give complete picture...

Short story: These changes adds ability to view available maps on server and download it to local computer. Picture better thousand words:
map_downloader

Long story:

new main classes:
AvailableMapsModel
Tree model with maps available by configured providers (see Settings::GetMapProviders).
Every map provider have to expose list of maps by json.
FileDownloadJob
Simple utility class for download single file over http. It don't support downloading restart when connection is interrupted.
MapDownloadJob
Utility class for downloading map database described by AvailableMapsModelMap over http. When connection is interrupted, downloading will be retried after some backoff period.
MapManager
Manager of map databases. It provide database lookup (in databaseDirectories) and simple scheduler for downloading maps.
MapDownloadsModel
QML list model with currently downloaded maps. It provide methods (invocable from QML) for starting new map download.

@Framstag
Copy link
Owner

Is it possible that some builds break because you did not extend the autoconf based Makefiles with the new source files?

@Karry
Copy link
Collaborator Author

Karry commented Dec 11, 2016

Build should be ok now. I have to add one #if for older Qt versions...

@@ -276,8 +280,8 @@ public slots:
void SearchForLocations(const QString searchPattern, int limit);

protected:
QStringList databaseLookupDirs;

MapManager *mapManager;
Copy link
Owner

Choose a reason for hiding this comment

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

I would nowadays always use shared_ptr or unique_ptr for memory management.

}
databases.clear();
delete mapManager;
Copy link
Owner

Choose a reason for hiding this comment

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

See comment above. This code could be dropped then.

{
double num = size;
QStringList list;
list << "KiB" << "MiB" << "GiB" << "TiB";
Copy link
Owner

Choose a reason for hiding this comment

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

We already have an libosmscout function for this. See String.h ByteSizeToString().

@@ -1,6 +1,7 @@
[
{
"uri": "https://osmscout.karry.cz",
"listUri": "https://osmscout.karry.cz/latest.php?fromVersion=%1&toVersion=%2&locale=%3",
Copy link
Owner

Choose a reason for hiding this comment

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

You should place a README into the OSMScout2 directory, with a hint where to change this and what directory structure is to be expected.

@Framstag
Copy link
Owner

Just some minor comments, see inline comments to code changes.

@Karry
Copy link
Collaborator Author

Karry commented Dec 12, 2016

Thank you for comments. I will update this branch tomorrow.

@Karry
Copy link
Collaborator Author

Karry commented Dec 13, 2016

Hi Tim. I updated code and OSMScout2 readme: https://github.com/karry-space-with-my-second-forks/libosmscout/blob/635bc2d8cfd8af7174c6f16d8554ffc867325345/OSMScout2/README.md

Btw, AvailableMapsModel is still using pointers for internal model, but it is "recomended" way how to implement Qt models and its model object pointers are not available outside model... (You should not access index.internalPointer() outside the model :-) )

@Framstag Framstag merged commit 858fa95 into Framstag:master Dec 17, 2016
@Karry Karry deleted the map-management branch December 17, 2016 09:12
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