organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues 2.7k Pull requests 182 Projects Releases 65 Wiki Activity Actions

[Draft] [gpx] Add gpx import #5166

Merged
cyber-toad merged 19 commits from gpx into master 2023-06-04 17:14:43 +00:00
Conversation 0 Commits 19 Files changed 27 +6910 -17
cyber-toad commented 2023-05-15 18:58:07 +00:00 (Migrated from github.com)
Copy link

Hi,
I try to finish the work started by @dvdmrtnz in #4067.
I made some tests and updated Manifest file but still was not able to have .gpx file associated with OrganicMaps. I asked on stackoverflow and searched on web, but was not able to find an answer.
@rtsisyk @arnaudvergnet - as Android experts could you give me any advice on it?
@biodranik - could you please advice, what minimal set of items is required to be implemented for merge? I checked your comment here #4067 so I think it should be enough to fix codestyle issues that you mentioned and add more tests (what do we want to check - large files, some details of GPX spec, something else)?

Closes #4067
Fixes #2953
Fixes #2681
Fixes #624

Hi, I try to finish the work started by @dvdmrtnz in #4067. I made some tests and updated Manifest file but still was not able to have .gpx file associated with OrganicMaps. I asked on stackoverflow and searched on web, but was not able to find an answer. @rtsisyk @arnaudvergnet - as Android experts could you give me any advice on it? @biodranik - could you please advice, what minimal set of items is required to be implemented for merge? I checked your comment here #4067 so I think it should be enough to fix codestyle issues that you mentioned and add more tests (what do we want to check - large files, some details of GPX spec, something else)? Closes #4067 Fixes #2953 Fixes #2681 Fixes #624
👍 8
biodranik (Migrated from github.com) reviewed 2023-05-15 21:17:48 +00:00
biodranik (Migrated from github.com) left a comment

Thanks for your efforts, it's a good start! Let's focus on properly testing and supporting different GPX files/versions and 1.1 spec, and clean the code a bit.

Thanks for your efforts, it's a good start! Let's focus on properly testing and supporting different GPX files/versions and 1.1 spec, and clean the code a bit.
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
biodranik (Migrated from github.com) commented 2023-05-15 20:54:16 +00:00

nit: use std::array<std::string_view> here, and add appropriate overload to jni::ToJavaStringArray

nit: use `std::array<std::string_view>` here, and add appropriate overload to jni::ToJavaStringArray
kml/kml_tests/gpx_tests.cpp
biodranik (Migrated from github.com) commented 2023-05-15 20:56:47 +00:00

Please find different examples of GPX files from different popular apps (some should be already in our issues), and make tests for them.

Also, please make a list of non-standard features that may be used in some GPX files (custom extensions, for example), that it would be useful to support, if not now, but in the future.

Please find different examples of GPX files from different popular apps (some should be already in our issues), and make tests for them. Also, please make a list of non-standard features that may be used in some GPX files (custom extensions, for example), that it would be useful to support, if not now, but in the future.
biodranik (Migrated from github.com) commented 2023-05-15 20:57:46 +00:00

Can we support GPX 1.0 and GPX 2.0? What elements are not supported now from the full spec? It would be great to list/track them in the related issue. What elements are supported by other apps?

Can we support GPX 1.0 and GPX 2.0? What elements are not supported now from the [full spec](https://www.topografix.com/GPX/1/1/)? It would be great to list/track them in the related issue. What elements are supported by other apps?
biodranik (Migrated from github.com) commented 2023-05-15 20:59:35 +00:00 
  char const * input = R"(<?xml version="1.0" encoding="UTF-8"?>
```suggestion char const * input = R"(<?xml version="1.0" encoding="UTF-8"?> ```
kml/serdes_gpx.cpp
biodranik (Migrated from github.com) commented 2023-05-15 21:01:17 +00:00

Can we use std::string_view instead? In any case, using namespace should be moved to the internal gpx namespace.

Can we use std::string_view instead? In any case, using namespace should be moved to the internal gpx namespace.
biodranik (Migrated from github.com) commented 2023-05-15 21:02:33 +00:00 
    m_geometryType = GEOMETRY_TYPE_POINT;

We don't use {} for one-liners, it makes code easier to read.

```suggestion m_geometryType = GEOMETRY_TYPE_POINT; ``` We don't use {} for one-liners, it makes code easier to read.
biodranik (Migrated from github.com) commented 2023-05-15 21:03:28 +00:00

Using "wpt"sv here and in other cases could make comparisons/parsing faster.

Using "wpt"sv here and in other cases could make comparisons/parsing faster.
biodranik (Migrated from github.com) commented 2023-05-15 21:07:06 +00:00

Can GetTagFromEnd also return std::string_view?

Can GetTagFromEnd also return std::string_view?
biodranik (Migrated from github.com) commented 2023-05-15 21:08:23 +00:00

Do you really need these tags? What does linter say?

Do you really need these tags? What does linter say?
kml/serdes_gpx.hpp
biodranik (Migrated from github.com) commented 2023-05-15 21:08:48 +00:00
```suggestion ```
biodranik (Migrated from github.com) commented 2023-05-15 21:08:57 +00:00 
  uint8_t m_viewportScale;
```suggestion uint8_t m_viewportScale; ```
biodranik (Migrated from github.com) commented 2023-05-15 21:09:36 +00:00

Is it a blind copy-paste? Are all these variables needed/used?

Is it a blind copy-paste? Are all these variables needed/used?
biodranik (Migrated from github.com) commented 2023-05-15 21:11:01 +00:00 
      // Print corrupted GPX file for debug and restore purposes.
      std::string gpxText;
      reader.ReadAsString(gpxText);
      if (!gpxText.empty() && gpxText[0] == '<')
        LOG(LWARNING, (gpxText));
      MYTHROW(DeserializeException, ("Could not parse GPX."));
```suggestion // Print corrupted GPX file for debug and restore purposes. std::string gpxText; reader.ReadAsString(gpxText); if (!gpxText.empty() && gpxText[0] == '<') LOG(LWARNING, (gpxText)); MYTHROW(DeserializeException, ("Could not parse GPX.")); ```
map/bookmark_helpers.cpp
biodranik (Migrated from github.com) commented 2023-05-15 21:13:01 +00:00

Imported GPX should be converted/stored as KML. Is this change really needed?

Imported GPX should be converted/stored as KML. Is this change really needed?
map/bookmark_helpers.hpp
biodranik (Migrated from github.com) commented 2023-05-15 21:14:28 +00:00 
  Binary,
  Gpx
```suggestion Binary, Gpx ```
map/bookmark_manager.cpp
@ -1875,0 +1877,4 @@
kmlData = LoadKmlFile(fileSavePath, KmlFileType::Text);
else if (ext == ".gpx")
kmlData = LoadKmlFile(fileSavePath, KmlFileType::Gpx);
else if (ext == ".kmb")
biodranik (Migrated from github.com) commented 2023-05-15 21:16:22 +00:00

What are the benefits of storing files internally in different formats? Why is it a bad idea to convert imported GPX files to KML ones and store them internally in KML?

What are the benefits of storing files internally in different formats? Why is it a bad idea to convert imported GPX files to KML ones and store them internally in KML?
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:04:57 +00:00
kml/serdes_gpx.hpp
cyber-toad (Migrated from github.com) commented 2023-05-16 20:04:57 +00:00

Yes, at least they are populated in the parser, I'll review their purpose.

Yes, at least they are populated in the parser, I'll review their purpose.
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:06:22 +00:00
kml/serdes_gpx.hpp
cyber-toad (Migrated from github.com) commented 2023-05-16 20:06:22 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:07:20 +00:00
kml/serdes_gpx.hpp
cyber-toad (Migrated from github.com) commented 2023-05-16 20:07:20 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:07:34 +00:00
kml/serdes_gpx.hpp
cyber-toad (Migrated from github.com) commented 2023-05-16 20:07:33 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:20:35 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-16 20:20:35 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:27:23 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-16 20:27:23 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-16 20:51:12 +00:00
map/bookmark_manager.cpp
@ -1875,0 +1877,4 @@
kmlData = LoadKmlFile(fileSavePath, KmlFileType::Text);
else if (ext == ".gpx")
kmlData = LoadKmlFile(fileSavePath, KmlFileType::Gpx);
else if (ext == ".kmb")
cyber-toad (Migrated from github.com) commented 2023-05-16 20:51:12 +00:00

I don't think that different formats are used internally, this parameter is used just to select a valid parser:

if (fileType == KmlFileType::Binary)
    {
      kml::binary::DeserializerKml des(*data);
      des.Deserialize(reader);
    }
    else if (fileType == KmlFileType::Text)
    {
      kml::DeserializerKml des(*data);
      des.Deserialize(reader);
    }
    else if (fileType == KmlFileType::Gpx)
    {
      kml::DeserializerGpx des(*data);
      des.Deserialize(reader);
    }
I don't think that different formats are used internally, this parameter is used just to select a valid parser: ``` if (fileType == KmlFileType::Binary) { kml::binary::DeserializerKml des(*data); des.Deserialize(reader); } else if (fileType == KmlFileType::Text) { kml::DeserializerKml des(*data); des.Deserialize(reader); } else if (fileType == KmlFileType::Gpx) { kml::DeserializerGpx des(*data); des.Deserialize(reader); } ```
cyber-toad (Migrated from github.com) reviewed 2023-05-16 21:02:50 +00:00
map/bookmark_helpers.hpp
cyber-toad (Migrated from github.com) commented 2023-05-16 21:02:50 +00:00

Fixed

Fixed
pm4rcin (Migrated from github.com) reviewed 2023-05-17 15:01:53 +00:00
kml/kml_tests/gpx_tests.cpp
pm4rcin (Migrated from github.com) commented 2023-05-17 15:01:53 +00:00

@biodranik just to note that there's only version 1.0 and 1.1 of GPX.

@biodranik just to note that there's only version 1.0 and 1.1 of GPX.
biodranik (Migrated from github.com) reviewed 2023-05-17 15:13:35 +00:00
kml/kml_tests/gpx_tests.cpp
biodranik (Migrated from github.com) commented 2023-05-17 15:13:35 +00:00

Correct, my mistake.

Correct, my mistake.
cyber-toad (Migrated from github.com) reviewed 2023-05-17 22:00:01 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-17 22:00:00 +00:00

I added several real-life tracks. I added very basic test to verify that files are parsed and we load track with waypoints.
Looks like files are more or less similar, except extensions in couple of them.
For extensions, I collected some of them on this page: https://github.com/cyber-toad/organicmaps/wiki/GPX. Not sure we have a place in UI to display them, except, maybe, color?

I added several real-life tracks. I added very basic test to verify that files are parsed and we load track with waypoints. Looks like files are more or less similar, except extensions in couple of them. For extensions, I collected some of them on this page: https://github.com/cyber-toad/organicmaps/wiki/GPX. Not sure we have a place in UI to display them, except, maybe, color?
cyber-toad (Migrated from github.com) reviewed 2023-05-17 22:02:37 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-17 22:02:37 +00:00

I added initial list here: https://github.com/cyber-toad/organicmaps/wiki/GPX
Looks like we don't have polygons, but for tracks they are not required.
Maybe load everything from metadata and put to track description?

I added initial list here: https://github.com/cyber-toad/organicmaps/wiki/GPX Looks like we don't have polygons, but for tracks they are not required. Maybe load everything from metadata and put to track description?
biodranik (Migrated from github.com) reviewed 2023-05-18 05:28:59 +00:00
kml/kml_tests/gpx_tests.cpp
biodranik (Migrated from github.com) commented 2023-05-18 05:28:59 +00:00

Thanks!

There is no need to do many tests for similar/same tags.

Was there a list of some GPX extensions/examples already on our Github?

If you collect them in your fork, please link it in the corresponding issue.

Only those extensions that can be easily mapped to our current KML should be supported (colors?), everything else can stay in the TODO list.

P.S. there are some unresolved comments.

Thanks! There is no need to do many tests for similar/same tags. Was there a list of some GPX extensions/examples already on our Github? If you collect them in your fork, please link it in the corresponding issue. Only those extensions that can be easily mapped to our current KML should be supported (colors?), everything else can stay in the TODO list. P.S. there are some unresolved comments.
biodranik (Migrated from github.com) reviewed 2023-05-18 05:29:55 +00:00
kml/kml_tests/gpx_tests.cpp
biodranik (Migrated from github.com) commented 2023-05-18 05:29:55 +00:00

There is an issue about polygons for KML, AFAIR. It would be great to mention/link there GPX polygons TODO too.

There is an issue about polygons for KML, AFAIR. It would be great to mention/link there GPX polygons TODO too.
cyber-toad (Migrated from github.com) reviewed 2023-05-18 07:26:24 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-18 07:26:24 +00:00

I added links to comments with requests here: https://github.com/cyber-toad/organicmaps/wiki/GPX
Will have a look at colors implementation.
I saw unresolved comments, need some time for c++ changes...

I added links to comments with requests here: https://github.com/cyber-toad/organicmaps/wiki/GPX Will have a look at colors implementation. I saw unresolved comments, need some time for c++ changes...
cyber-toad (Migrated from github.com) reviewed 2023-05-18 07:27:39 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-18 07:27:39 +00:00

Polygons added to https://github.com/cyber-toad/organicmaps/wiki/GPX

Polygons added to https://github.com/cyber-toad/organicmaps/wiki/GPX
cyber-toad (Migrated from github.com) reviewed 2023-05-18 09:49:32 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-18 09:49:32 +00:00

Looks like copy-paste from KML where they are used for some specific case and not used in GPX for now, so removed.

Looks like copy-paste from KML where they are used for some specific case and not used in GPX for now, so removed.
biodranik (Migrated from github.com) reviewed 2023-05-18 10:57:29 +00:00
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
biodranik (Migrated from github.com) commented 2023-05-18 10:57:29 +00:00

Upd: looks like ToJavaStringArray should already support std::arraystd::string_view

Upd: looks like ToJavaStringArray should already support std::array<std::string_view>
cyber-toad (Migrated from github.com) reviewed 2023-05-21 08:48:10 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-21 08:48:10 +00:00

Switched to string_view

Switched to string_view
cyber-toad (Migrated from github.com) reviewed 2023-05-21 09:22:21 +00:00
kml/serdes_gpx.hpp
cyber-toad (Migrated from github.com) commented 2023-05-21 09:22:21 +00:00

Unused variables are removed.

Unused variables are removed.
biodranik (Migrated from github.com) reviewed 2023-05-21 11:39:13 +00:00
map/bookmark_helpers.cpp
biodranik (Migrated from github.com) commented 2023-05-21 11:39:13 +00:00

Can you please clarify the current behavior? Are imported GPX always converted and stored internally as KML files? Then there's no need to patch kml load code.

Can you please clarify the current behavior? Are imported GPX always converted and stored internally as KML files? Then there's no need to patch kml load code.
cyber-toad (Migrated from github.com) reviewed 2023-05-21 14:37:27 +00:00
map/bookmark_helpers.cpp
cyber-toad (Migrated from github.com) commented 2023-05-21 14:37:26 +00:00

I checked in debugger and looks like extension is checked on early stage of file loader where we still have .gpx:

image

I checked in debugger and looks like extension is checked on early stage of file loader where we still have .gpx: ![image](https://github.com/organicmaps/organicmaps/assets/128428520/fb7a4e1b-f953-4f64-8334-a93caac685e1)
cyber-toad (Migrated from github.com) reviewed 2023-05-21 15:24:19 +00:00
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
cyber-toad (Migrated from github.com) commented 2023-05-21 15:24:18 +00:00

I replaced with

const std::array<std::string_view, 4> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };

but got an error:

jni_helper.hpp:89:32: error: no matching function for call to object of type '(lambda at /home/a/dev/organicmaps/android/jni/./app/organicmaps/core/jni_helper.hpp:108:22)'

so it is not supported for now?

I replaced with `const std::array<std::string_view, 4> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };` but got an error: `jni_helper.hpp:89:32: error: no matching function for call to object of type '(lambda at /home/a/dev/organicmaps/android/jni/./app/organicmaps/core/jni_helper.hpp:108:22)'` so it is not supported for now?
biodranik (Migrated from github.com) reviewed 2023-05-21 16:51:14 +00:00
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
biodranik (Migrated from github.com) commented 2023-05-21 16:51:13 +00:00

Did you made kKmzExtension and others also std::string_view ?

Did you made kKmzExtension and others also std::string_view ?
biodranik (Migrated from github.com) reviewed 2023-05-21 16:52:05 +00:00
map/bookmark_helpers.cpp
biodranik (Migrated from github.com) commented 2023-05-21 16:52:05 +00:00

Am I correctly understood, that this part of the code is used during the import too?

Am I correctly understood, that this part of the code is used _during the import_ too?
cyber-toad (Migrated from github.com) reviewed 2023-05-22 06:43:23 +00:00
map/bookmark_helpers.cpp
cyber-toad (Migrated from github.com) commented 2023-05-22 06:43:23 +00:00

Yes, it is executed when I import a track. Here is a stacktrace for this call.

[liborganicmaps.so] GetKMLPath(const std::__ndk1::basic_string<…> &) bookmark_helpers.cpp:295
[liborganicmaps.so] $_15::operator()() const bookmark_manager.cpp:1871
[liborganicmaps.so] std::__ndk1::__invoke<…>($_15 &) type_traits:3874
[liborganicmaps.so] std::__ndk1::__invoke_void_return_wrapper::__call<…>($_15 &) __functional_base:348
[liborganicmaps.so] std::__ndk1::__function::__alloc_func::operator()() functional:1557
[liborganicmaps.so] std::__ndk1::__function::__func::operator()() functional:1731
[liborganicmaps.so] std::__ndk1::__function::__value_func::operator()() const functional:1884
[liborganicmaps.so] std::__ndk1::function::operator()() const functional:2556
[liborganicmaps.so] base::thread_pool::delayed::ThreadPool::ProcessTasks() thread_pool_delayed.cpp:157
[liborganicmaps.so] std::__ndk1::__invoke<…>(void (base::thread_pool::delayed::ThreadPool::*&)(), base::thread_pool::delayed::ThreadPool *&) type_traits:3815
[liborganicmaps.so] std::__ndk1::__apply_functor<…>(void (base::thread_pool::delayed::ThreadPool::*&)(), std::__ndk1::tuple<…> &, std::__ndk1::__tuple_indices<…>, std::__ndk1::tuple<…> &&) functional:2853
[liborganicmaps.so] std::__ndk1::__bind::operator()<…>() functional:2886
[liborganicmaps.so] std::__ndk1::__invoke<…>(std::__ndk1::__bind<…> &) type_traits:3874
[liborganicmaps.so] std::__ndk1::__invoke_void_return_wrapper::__call<…>(std::__ndk1::__bind<…> &) __functional_base:348
[liborganicmaps.so] std::__ndk1::__function::__alloc_func::operator()() functional:1557
[liborganicmaps.so] std::__ndk1::__function::__func::operator()() functional:1731
[liborganicmaps.so] std::__ndk1::__function::__value_func::operator()() const functional:1884
[liborganicmaps.so] std::__ndk1::function::operator()() const functional:2556
[liborganicmaps.so] threads::SimpleThread::ThreadFunc(std::__ndk1::function<…> &&) thread.cpp:101
[liborganicmaps.so] std::__ndk1::__invoke<…>(void (*&&)(std::__ndk1::function<…> &&), std::__ndk1::__bind<…> &&) type_traits:3874
[liborganicmaps.so] std::__ndk1::__thread_execute<…>(std::__ndk1::tuple<…> &, std::__ndk1::__tuple_indices<…>) thread:273
[liborganicmaps.so] std::__ndk1::__thread_proxy<…>(void *) thread:284
[libc.so] __pthread_start(void*) 0x000000715d7a6cb4
[libc.so] __start_thread 0x000000715d744eb0
Yes, it is executed when I import a track. Here is a stacktrace for this call. ``` [liborganicmaps.so] GetKMLPath(const std::__ndk1::basic_string<…> &) bookmark_helpers.cpp:295 [liborganicmaps.so] $_15::operator()() const bookmark_manager.cpp:1871 [liborganicmaps.so] std::__ndk1::__invoke<…>($_15 &) type_traits:3874 [liborganicmaps.so] std::__ndk1::__invoke_void_return_wrapper::__call<…>($_15 &) __functional_base:348 [liborganicmaps.so] std::__ndk1::__function::__alloc_func::operator()() functional:1557 [liborganicmaps.so] std::__ndk1::__function::__func::operator()() functional:1731 [liborganicmaps.so] std::__ndk1::__function::__value_func::operator()() const functional:1884 [liborganicmaps.so] std::__ndk1::function::operator()() const functional:2556 [liborganicmaps.so] base::thread_pool::delayed::ThreadPool::ProcessTasks() thread_pool_delayed.cpp:157 [liborganicmaps.so] std::__ndk1::__invoke<…>(void (base::thread_pool::delayed::ThreadPool::*&)(), base::thread_pool::delayed::ThreadPool *&) type_traits:3815 [liborganicmaps.so] std::__ndk1::__apply_functor<…>(void (base::thread_pool::delayed::ThreadPool::*&)(), std::__ndk1::tuple<…> &, std::__ndk1::__tuple_indices<…>, std::__ndk1::tuple<…> &&) functional:2853 [liborganicmaps.so] std::__ndk1::__bind::operator()<…>() functional:2886 [liborganicmaps.so] std::__ndk1::__invoke<…>(std::__ndk1::__bind<…> &) type_traits:3874 [liborganicmaps.so] std::__ndk1::__invoke_void_return_wrapper::__call<…>(std::__ndk1::__bind<…> &) __functional_base:348 [liborganicmaps.so] std::__ndk1::__function::__alloc_func::operator()() functional:1557 [liborganicmaps.so] std::__ndk1::__function::__func::operator()() functional:1731 [liborganicmaps.so] std::__ndk1::__function::__value_func::operator()() const functional:1884 [liborganicmaps.so] std::__ndk1::function::operator()() const functional:2556 [liborganicmaps.so] threads::SimpleThread::ThreadFunc(std::__ndk1::function<…> &&) thread.cpp:101 [liborganicmaps.so] std::__ndk1::__invoke<…>(void (*&&)(std::__ndk1::function<…> &&), std::__ndk1::__bind<…> &&) type_traits:3874 [liborganicmaps.so] std::__ndk1::__thread_execute<…>(std::__ndk1::tuple<…> &, std::__ndk1::__tuple_indices<…>) thread:273 [liborganicmaps.so] std::__ndk1::__thread_proxy<…>(void *) thread:284 [libc.so] __pthread_start(void*) 0x000000715d7a6cb4 [libc.so] __start_thread 0x000000715d744eb0 ```
cyber-toad (Migrated from github.com) reviewed 2023-05-22 07:14:24 +00:00
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
cyber-toad (Migrated from github.com) commented 2023-05-22 07:14:24 +00:00

Thanks, it fixes the issue, but after this change I noticed that kKmlExtension is quite widely used in the code as std::string,
for example
std::string GenerateUniqueFileName(const std::string & path, std::string name, std::string const & ext = kKmlExtension);

auto const filePath = base::JoinPath(GetPlatform().TmpDir(), fileName + kKmlExtension);

etc. Do we want to rework all this code in scope of gpx change?

Thanks, it fixes the issue, but after this change I noticed that kKmlExtension is quite widely used in the code as std::string, for example `std::string GenerateUniqueFileName(const std::string & path, std::string name, std::string const & ext = kKmlExtension);` `auto const filePath = base::JoinPath(GetPlatform().TmpDir(), fileName + kKmlExtension);` etc. Do we want to rework all this code in scope of gpx change?
biodranik (Migrated from github.com) reviewed 2023-05-22 08:27:21 +00:00
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
biodranik (Migrated from github.com) commented 2023-05-22 08:27:21 +00:00

You can make a separate PR for it, if it's an easy-to-do change :) Or add a commit here. As I raised this issue, I'm ok with that approach ("fix some old code if you're already touching it anyway").

You can make a separate PR for it, if it's an easy-to-do change :) Or add a commit here. As I raised this issue, I'm ok with that approach ("fix some old code if you're already touching it anyway").
biodranik (Migrated from github.com) reviewed 2023-05-23 07:22:28 +00:00
kml/serdes_gpx.cpp
@ -0,0 +126,4 @@
{
ASSERT_LESS(n, m_tags.size(), ());
return m_tags[m_tags.size() - n - 1];
}
biodranik (Migrated from github.com) commented 2023-05-23 07:19:10 +00:00

auto const colorBytes?

auto const colorBytes?
biodranik (Migrated from github.com) commented 2023-05-23 07:20:38 +00:00

LOG(LWARNING, ("Invalid color value", value));

LOG(LWARNING, ("Invalid color value", value));
biodranik (Migrated from github.com) commented 2023-05-23 07:21:31 +00:00

Octal system is not understandable by many. Why not use just a plain 255 number?

Octal system is not understandable by many. Why not use just a plain 255 number?
kml/serdes_gpx.cpp
@ -0,0 +177,4 @@
m_data.m_bookmarksData.push_back(std::move(data));
}
else if (GEOMETRY_TYPE_LINE == m_geometryType)
biodranik (Migrated from github.com) commented 2023-05-23 07:21:57 +00:00

What is this color?

What is this color?
kml/serdes_gpx.cpp
biodranik (Migrated from github.com) commented 2023-05-23 07:22:24 +00:00

We don't use braces for one-line ifs and loops.

We don't use braces for one-line ifs and loops.
cyber-toad (Migrated from github.com) reviewed 2023-05-23 18:41:29 +00:00
kml/serdes_gpx.cpp
@ -0,0 +126,4 @@
{
ASSERT_LESS(n, m_tags.size(), ());
return m_tags[m_tags.size() - n - 1];
}
cyber-toad (Migrated from github.com) commented 2023-05-23 18:41:28 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-23 18:41:44 +00:00
kml/serdes_gpx.cpp
@ -0,0 +126,4 @@
{
ASSERT_LESS(n, m_tags.size(), ());
return m_tags[m_tags.size() - n - 1];
}
cyber-toad (Migrated from github.com) commented 2023-05-23 18:41:44 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-23 18:41:53 +00:00
kml/serdes_gpx.cpp
@ -0,0 +126,4 @@
{
ASSERT_LESS(n, m_tags.size(), ());
return m_tags[m_tags.size() - n - 1];
}
cyber-toad (Migrated from github.com) commented 2023-05-23 18:41:53 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-23 18:42:21 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-23 18:42:20 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-23 18:49:25 +00:00
kml/serdes_gpx.cpp
@ -0,0 +177,4 @@
m_data.m_bookmarksData.push_back(std::move(data));
}
else if (GEOMETRY_TYPE_LINE == m_geometryType)
cyber-toad (Migrated from github.com) commented 2023-05-23 18:49:24 +00:00

Yes, was not able to find it's usage, removed.

Yes, was not able to find it's usage, removed.
biodranik (Migrated from github.com) requested changes 2023-05-28 12:08:30 +00:00
biodranik (Migrated from github.com) left a comment

Thank you very much! We're almost there :)

Thank you very much! We're almost there :)
android/AndroidManifest.xml
biodranik (Migrated from github.com) commented 2023-05-28 11:23:29 +00:00

Can you please add three more layers of directories to gpx, kml and kmz? It could be an issue in your device if Android stores files "too deep" in many subfolders.

Can you please add three more layers of directories to gpx, kml and kmz? It could be an issue in your device if Android stores files "too deep" in many subfolders.
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
biodranik (Migrated from github.com) commented 2023-05-28 11:24:12 +00:00

Can you do this minor refactoring in a parallel PR?

Can you do this minor refactoring in a parallel PR?
data/gpx_test_data/color.gpx
@ -0,0 +3,4 @@
<metadata>
<name>new</name>
<author>
<name>gpx.studio</name>
biodranik (Migrated from github.com) commented 2023-05-28 11:27:55 +00:00

Can you please test tracks recorded with GPSLogger: https://play.google.com/store/apps/details?id=eu.basicairdata.graziano.gpslogger ?

I tried it and there were no any name displayed on the recorded track. Was it a bug or the name was missing? If name was missing, the following logic can be implemented:

  • If there is one track in the file without a name, then track uses the file name (without the extension).
  • If there are 2 or more tracks without names, then the file name + 1, 2, 3... indexes can be added to tracks, for easier UX in the bookmarks dialog.
Can you please test tracks recorded with GPSLogger: https://play.google.com/store/apps/details?id=eu.basicairdata.graziano.gpslogger ? I tried it and there were no any name displayed on the recorded track. Was it a bug or the name was missing? If name was missing, the following logic can be implemented: - If there is one track in the file without a name, then track uses the file name (without the extension). - If there are 2 or more tracks without names, then the file name + 1, 2, 3... indexes can be added to tracks, for easier UX in the bookmarks dialog.
iphone/Maps/OMaps.plist
@ -11,0 +15,4 @@
<string>GPS Exchange Format (GPX)</string>
<key>LSHandlerRank</key>
<string>Default</string>
<key>LSItemContentTypes</key>
biodranik (Migrated from github.com) commented 2023-05-28 11:34:42 +00:00

This entry doesn't look right or complete. Did you check https://github.com/osmandapp/OsmAnd-iOS/blob/master/OsmAnd%20Maps%20copy-Info.plist ?

This entry doesn't look right or complete. Did you check https://github.com/osmandapp/OsmAnd-iOS/blob/master/OsmAnd%20Maps%20copy-Info.plist ?
kml/kml_tests/CMakeLists.txt
@ -1,9 +1,10 @@
project(kml_tests)
set(SRC
biodranik (Migrated from github.com) commented 2023-05-28 11:35:19 +00:00

Let's sort names alphabetically.

Let's sort names alphabetically.
kml/kml_tests/gpx_tests.cpp
biodranik (Migrated from github.com) commented 2023-05-28 11:35:34 +00:00 
auto constexpr kDefaultCode = StringUtf8Multilang::kDefaultCode;
```suggestion auto constexpr kDefaultCode = StringUtf8Multilang::kDefaultCode; ```
biodranik (Migrated from github.com) commented 2023-05-28 11:36:33 +00:00

This is not our coding standard. It is described in docs. Please review the code and fix it everywhere.

kml::FileData loadGpxFromString(std::string const & content)
{
This is not our coding standard. It is described in docs. Please review the code and fix it everywhere. ```suggestion kml::FileData loadGpxFromString(std::string const & content) { ```
biodranik (Migrated from github.com) commented 2023-05-28 11:38:19 +00:00 
    kml::DeserializerGpx des(dataFromText);
    MemReader reader(content.data(), content.size());
```suggestion kml::DeserializerGpx des(dataFromText); MemReader reader(content.data(), content.size()); ```
biodranik (Migrated from github.com) commented 2023-05-28 11:39:14 +00:00 
  std::string const input = R"(<?xml version="1.0" encoding="UTF-8"?>
```suggestion std::string const input = R"(<?xml version="1.0" encoding="UTF-8"?> ```
biodranik (Migrated from github.com) commented 2023-05-28 11:39:59 +00:00

Please mark all const variables with a const keyword in all places.

  kml::FileData const dataFromText = loadGpxFromString(input);
Please mark all const variables with a `const` keyword in all places. ```suggestion kml::FileData const dataFromText = loadGpxFromString(input); ```
biodranik (Migrated from github.com) commented 2023-05-28 11:40:09 +00:00

ditto

ditto
kml/serdes_common.hpp
biodranik (Migrated from github.com) commented 2023-05-28 11:41:05 +00:00 
auto constexpr kDefaultLang = StringUtf8Multilang::kDefaultCode;
auto constexpr kDefaultTrackWidth = 5.0;
auto constexpr kDefaultTrackColor = 0x006ec7ff;
```suggestion auto constexpr kDefaultLang = StringUtf8Multilang::kDefaultCode; auto constexpr kDefaultTrackWidth = 5.0; auto constexpr kDefaultTrackColor = 0x006ec7ff; ```
biodranik (Migrated from github.com) commented 2023-05-28 11:41:23 +00:00 
}  // namespace kml
```suggestion } // namespace kml ```
kml/serdes_gpx.cpp
biodranik (Migrated from github.com) commented 2023-05-28 11:43:07 +00:00

constexpr here and below

constexpr here and below
biodranik (Migrated from github.com) commented 2023-05-28 11:45:00 +00:00 
}  // namespace gpx
```suggestion } // namespace gpx ```
biodranik (Migrated from github.com) commented 2023-05-28 11:52:37 +00:00 
}  // namespace kml
```suggestion } // namespace kml ```
biodranik (Migrated from github.com) commented 2023-05-28 11:54:02 +00:00 
      }
      else if (currTag == gpx::kDesc)
      {
```suggestion } else if (currTag == gpx::kDesc) { ```
biodranik (Migrated from github.com) commented 2023-05-28 11:57:03 +00:00

std::string_view objects are faster when passed by value than by reference.

bool GpxParser::Push(std::string_view tag)
std::string_view objects are faster when passed by value than by reference. ```suggestion bool GpxParser::Push(std::string_view tag) ```
biodranik (Migrated from github.com) commented 2023-05-28 12:04:26 +00:00 
void GpxParser::AddAttr(std::string attr, std::string const & value)
{
  strings::AsciiToLower(attr);

  if (GetTagFromEnd(0) == gpx::kWpt ||
      (GetTagFromEnd(0) == gpx::kTrkPt && GetTagFromEnd(1) == gpx::kTrkSeg) ||
      (GetTagFromEnd(0) == gpx::kRtePt && GetTagFromEnd(1) == gpx::kRte))
  {
    if (attr == "lat")
    {
      if (!strings::to_double(value, m_lat))
        LOG(LWARN, ("Invalid latitude:", value));  // TODO: Write a test for this case.
    }
    else if (attr == "long")
    {
      if (!strings::to_double(value, m_lon))
        LOG(LWARN, ("Invalid longitude:", value));  // TODO: Write a test for this case.
    }
  }
}
```suggestion void GpxParser::AddAttr(std::string attr, std::string const & value) { strings::AsciiToLower(attr); if (GetTagFromEnd(0) == gpx::kWpt || (GetTagFromEnd(0) == gpx::kTrkPt && GetTagFromEnd(1) == gpx::kTrkSeg) || (GetTagFromEnd(0) == gpx::kRtePt && GetTagFromEnd(1) == gpx::kRte)) { if (attr == "lat") { if (!strings::to_double(value, m_lat)) LOG(LWARN, ("Invalid latitude:", value)); // TODO: Write a test for this case. } else if (attr == "long") { if (!strings::to_double(value, m_lon)) LOG(LWARN, ("Invalid longitude:", value)); // TODO: Write a test for this case. } } } ```
biodranik (Migrated from github.com) commented 2023-05-28 12:04:39 +00:00 
std::string_view GpxParser::GetTagFromEnd(size_t n) const
```suggestion std::string_view GpxParser::GetTagFromEnd(size_t n) const ```
biodranik (Migrated from github.com) commented 2023-05-28 12:05:05 +00:00 
void GpxParser::Pop(std::string_view tag)
```suggestion void GpxParser::Pop(std::string_view tag) ```
biodranik (Migrated from github.com) commented 2023-05-28 12:05:46 +00:00 
      m_line.emplace_back(std::move(p));
```suggestion m_line.emplace_back(std::move(p)); ```
biodranik (Migrated from github.com) commented 2023-05-28 12:06:52 +00:00

Does it fit in 120 characters?

        if (data.m_name.size() == 1 && data.m_name.begin()->first == kDefaultLangCode && data.m_customName.empty())
Does it fit in 120 characters? ```suggestion if (data.m_name.size() == 1 && data.m_name.begin()->first == kDefaultLangCode && data.m_customName.empty()) ```
biodranik (Migrated from github.com) commented 2023-05-28 12:07:10 +00:00 
          data.m_customName = data.m_name;
```suggestion data.m_customName = data.m_name; ```
kml/serdes_gpx.cpp
@ -0,0 +70,4 @@
if (mercator::ValidX(m_org.x) && mercator::ValidY(m_org.y))
{
// Set default name.
if (m_name.empty())
biodranik (Migrated from github.com) commented 2023-05-28 11:56:05 +00:00

Should name be set even if there is no m_org point? Is it a possible scenario?

Should name be set even if there is no m_org point? Is it a possible scenario?
kml/serdes_gpx.cpp
@ -0,0 +95,4 @@
if (GetTagFromEnd(0) == gpx::kWpt)
m_geometryType = GEOMETRY_TYPE_POINT;
else if (GetTagFromEnd(0) == gpx::kTrkPt || GetTagFromEnd(0) == gpx::kRtePt)
m_geometryType = GEOMETRY_TYPE_LINE;
biodranik (Migrated from github.com) commented 2023-05-28 11:58:10 +00:00

What happens when both ifs above won't work? Is it possible with area-only gpx files, for example? Will the parser crash?

What happens when both ifs above won't work? Is it possible with area-only gpx files, for example? Will the parser crash?
kml/serdes_gpx.cpp
@ -0,0 +245,4 @@
DeserializerGpx::DeserializerGpx(FileData & fileData)
: m_fileData(fileData)
{
m_fileData = {};
biodranik (Migrated from github.com) commented 2023-05-28 11:53:11 +00:00

Why do you need to reset it after setting in L247 above? Is it a mistake?

Why do you need to reset it after setting in L247 above? Is it a mistake?
kml/serdes_gpx.hpp
biodranik (Migrated from github.com) commented 2023-05-28 11:50:36 +00:00 
}  // namespace kml
```suggestion } // namespace kml ```
kml/serdes_gpx.hpp
@ -0,0 +20,4 @@
{
namespace gpx
{
class GpxParser
biodranik (Migrated from github.com) commented 2023-05-28 11:42:36 +00:00

Can this class be inside kml::gpx namespace?

Can this class be inside kml::gpx namespace?
kml/serdes_gpx.hpp
@ -0,0 +85,4 @@
reader.ReadAsString(gpxText);
if (!gpxText.empty() && gpxText[0] == '<')
LOG(LWARNING, (gpxText));
MYTHROW(DeserializeException, ("Could not parse GPX."));
biodranik (Migrated from github.com) commented 2023-05-28 11:51:24 +00:00

Did you test if this exception is thrown correctly with a corrupted GPX?

Did you test if this exception is thrown correctly with a corrupted GPX?
map/bookmark_manager.cpp
biodranik (Migrated from github.com) commented 2023-05-28 11:46:30 +00:00

Note the most popular extension comes first.

      if (ext == ".kml")
        kmlData = LoadKmlFile(fileSavePath, KmlFileType::Text);
      else if (ext == ".gpx")
        kmlData = LoadKmlFile(fileSavePath, KmlFileType::Gpx);
      else if (ext == ".kmb")
        kmlData = LoadKmlFile(fileSavePath, KmlFileType::Binary);
      else
        ASSERT(false, ("Unsupported bookmarks extension", ext));
Note the most popular extension comes first. ```suggestion if (ext == ".kml") kmlData = LoadKmlFile(fileSavePath, KmlFileType::Text); else if (ext == ".gpx") kmlData = LoadKmlFile(fileSavePath, KmlFileType::Gpx); else if (ext == ".kmb") kmlData = LoadKmlFile(fileSavePath, KmlFileType::Binary); else ASSERT(false, ("Unsupported bookmarks extension", ext)); ```
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:38:10 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-28 19:38:09 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:38:41 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-28 19:38:41 +00:00

Yes, fixed

Yes, fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:39:13 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-28 19:39:13 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:39:42 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-28 19:39:42 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:40:11 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-28 19:40:11 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:41:11 +00:00
kml/kml_tests/CMakeLists.txt
@ -1,9 +1,10 @@
project(kml_tests)
set(SRC
cyber-toad (Migrated from github.com) commented 2023-05-28 19:41:11 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:42:59 +00:00
android/AndroidManifest.xml
cyber-toad (Migrated from github.com) commented 2023-05-28 19:42:59 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 19:46:15 +00:00
iphone/Maps/OMaps.plist
@ -11,0 +15,4 @@
<string>GPS Exchange Format (GPX)</string>
<key>LSHandlerRank</key>
<string>Default</string>
<key>LSItemContentTypes</key>
cyber-toad (Migrated from github.com) commented 2023-05-28 19:46:15 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-28 20:10:56 +00:00
data/gpx_test_data/color.gpx
@ -0,0 +3,4 @@
<metadata>
<name>new</name>
<author>
<name>gpx.studio</name>
cyber-toad (Migrated from github.com) commented 2023-05-28 20:10:55 +00:00

By some reason I can't install this app on my android phone - it is displayed as not available in Google Play, could you please share the recorded file here (all points could be removed for privacy, I'm just interested in metadata and track data).

Also I didn't get how do you want to use file name, I think it is not available in parser? Also during the load there is some artificial name: organicmaps/organicmaps#5166 ?

By some reason I can't install this app on my android phone - it is displayed as not available in Google Play, could you please share the recorded file here (all points could be removed for privacy, I'm just interested in metadata and track data). Also I didn't get how do you want to use file name, I think it is not available in parser? Also during the load there is some artificial name: https://git.omaps.dev/organicmaps/organicmaps/pulls/5166#discussion_r1199778440 ?
biodranik (Migrated from github.com) reviewed 2023-05-28 21:49:43 +00:00
data/gpx_test_data/color.gpx
@ -0,0 +3,4 @@
<metadata>
<name>new</name>
<author>
<name>gpx.studio</name>
biodranik (Migrated from github.com) commented 2023-05-28 21:49:42 +00:00

Looks like it was a file from another logger: https://gpslogger.app/
It has only points in the log, without any metadata, with a file name like 20230402.gpx:

<?xml version="1.0" encoding="UTF-8" ?><gpx version="1.0" creator="GPSLogger 126 - http://gpslogger.mendhak.com/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.topografix.com/GPX/1/0" xsi:schemaLocation="http://www.topografix.com/GPX/1/0 http://www.topografix.com/GPX/1/0/gpx.xsd"><time>2023-04-02T11:25:10.964Z</time><trk><trkseg><trkpt lat="...
Looks like it was a file from another logger: https://gpslogger.app/ It has only points in the log, without any metadata, with a file name like `20230402.gpx`: ``` <?xml version="1.0" encoding="UTF-8" ?><gpx version="1.0" creator="GPSLogger 126 - http://gpslogger.mendhak.com/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.topografix.com/GPX/1/0" xsi:schemaLocation="http://www.topografix.com/GPX/1/0 http://www.topografix.com/GPX/1/0/gpx.xsd"><time>2023-04-02T11:25:10.964Z</time><trk><trkseg><trkpt lat="... ```
cyber-toad (Migrated from github.com) reviewed 2023-05-29 15:11:53 +00:00
data/gpx_test_data/color.gpx
@ -0,0 +3,4 @@
<metadata>
<name>new</name>
<author>
<name>gpx.studio</name>
cyber-toad (Migrated from github.com) commented 2023-05-29 15:11:53 +00:00

Looks like it was a file from another logger: https://gpslogger.app/ It has only points in the log, without any metadata, with a file name like 20230402.gpx:

<?xml version="1.0" encoding="UTF-8" ?><gpx version="1.0" creator="GPSLogger 126 - http://gpslogger.mendhak.com/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.topografix.com/GPX/1/0" xsi:schemaLocation="http://www.topografix.com/GPX/1/0 http://www.topografix.com/GPX/1/0/gpx.xsd"><time>2023-04-02T11:25:10.964Z</time><trk><trkseg><trkpt lat="...

Ok, I see.
So we want to add filename as a parameter to
std::unique_ptr<kml::FileData> LoadKmlData(Reader const & reader, KmlFileType fileType)
method so it can pass it to GPX deserializer and it will fallback to it in case of missing name in file?

> Looks like it was a file from another logger: https://gpslogger.app/ It has only points in the log, without any metadata, with a file name like `20230402.gpx`: > > ``` > <?xml version="1.0" encoding="UTF-8" ?><gpx version="1.0" creator="GPSLogger 126 - http://gpslogger.mendhak.com/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.topografix.com/GPX/1/0" xsi:schemaLocation="http://www.topografix.com/GPX/1/0 http://www.topografix.com/GPX/1/0/gpx.xsd"><time>2023-04-02T11:25:10.964Z</time><trk><trkseg><trkpt lat="... > ``` Ok, I see. So we want to add filename as a parameter to `std::unique_ptr<kml::FileData> LoadKmlData(Reader const & reader, KmlFileType fileType)` method so it can pass it to GPX deserializer and it will fallback to it in case of missing name in file?
biodranik (Migrated from github.com) reviewed 2023-05-29 15:20:40 +00:00
data/gpx_test_data/color.gpx
@ -0,0 +3,4 @@
<metadata>
<name>new</name>
<author>
<name>gpx.studio</name>
biodranik (Migrated from github.com) commented 2023-05-29 15:20:40 +00:00

That can be an option, right.

Alternatively, is it better/easier to check if returned FileData has empty Category name, and fix it there?

That can be an option, right. Alternatively, is it better/easier to check if returned FileData has empty Category name, and fix it there?
cyber-toad (Migrated from github.com) reviewed 2023-05-30 06:21:56 +00:00
data/gpx_test_data/color.gpx
@ -0,0 +3,4 @@
<metadata>
<name>new</name>
<author>
<name>gpx.studio</name>
cyber-toad (Migrated from github.com) commented 2023-05-30 06:21:56 +00:00

Yes, thanks, it looks easier. I applied this change and added a test for this case.

Yes, thanks, it looks easier. I applied this change and added a test for this case.
cyber-toad (Migrated from github.com) reviewed 2023-05-30 06:29:35 +00:00
map/bookmark_manager.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 06:29:34 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 06:30:59 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 06:30:59 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 06:31:25 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 06:31:25 +00:00

Switched to string_view

Switched to string_view
cyber-toad (Migrated from github.com) reviewed 2023-05-30 06:32:33 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 06:32:32 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 06:34:45 +00:00
kml/serdes_gpx.hpp
cyber-toad (Migrated from github.com) commented 2023-05-30 06:34:44 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 07:01:20 +00:00
kml/serdes_gpx.cpp
@ -0,0 +95,4 @@
if (GetTagFromEnd(0) == gpx::kWpt)
m_geometryType = GEOMETRY_TYPE_POINT;
else if (GetTagFromEnd(0) == gpx::kTrkPt || GetTagFromEnd(0) == gpx::kRtePt)
m_geometryType = GEOMETRY_TYPE_LINE;
cyber-toad (Migrated from github.com) commented 2023-05-30 07:01:19 +00:00

I added a test - UNIT_TEST(Empty). Parser is not crashed, but if there are no points in track - it is not extracted.

I added a test - UNIT_TEST(Empty). Parser is not crashed, but if there are no points in track - it is not extracted.
biodranik (Migrated from github.com) reviewed 2023-05-30 09:13:19 +00:00
biodranik (Migrated from github.com) left a comment

Thanks! There are also some unresolved comments from the previous review. Can you please take a look at them here? organicmaps/organicmaps#5166/files#

We plan to release a beta soon, it would be great if these changes will be ready to test on a wider audience ;-)

Thanks! There are also some unresolved comments from the previous review. Can you please take a look at them here? https://git.omaps.dev/organicmaps/organicmaps/pulls/5166/files# We plan to release a beta soon, it would be great if these changes will be ready to test on a wider audience ;-)
iphone/Maps/OMaps.plist
biodranik (Migrated from github.com) commented 2023-05-30 09:08:52 +00:00

Indentation is wrong.

Indentation is wrong.
kml/kml_tests/gpx_tests.cpp
biodranik (Migrated from github.com) commented 2023-05-30 09:09:42 +00:00 
kml::FileData loadGpxFromFile(std::string const & file) {
  auto const fileName = GetPlatform().TestsDataPathForFile(file);
```suggestion kml::FileData loadGpxFromFile(std::string const & file) { auto const fileName = GetPlatform().TestsDataPathForFile(file); ```
biodranik (Migrated from github.com) commented 2023-05-30 09:10:31 +00:00

Can FileReader::ReadAsString() be used here?

Can FileReader::ReadAsString() be used here?
map/bookmark_helpers.cpp
biodranik (Migrated from github.com) commented 2023-05-30 09:11:33 +00:00 
void FillEmptyNames(std::unique_ptr<kml::FileData> & kmlData, std::string const & file)
```suggestion void FillEmptyNames(std::unique_ptr<kml::FileData> & kmlData, std::string const & file) ```
biodranik (Migrated from github.com) commented 2023-05-30 09:11:49 +00:00 
  auto const name = file.substr(start, end - start);
```suggestion auto const name = file.substr(start, end - start); ```
biodranik (Migrated from github.com) commented 2023-05-30 09:12:02 +00:00 
  auto const emptyNames = std::count_if(kmlData->m_tracksData.begin(), kmlData->m_tracksData.end(),
```suggestion auto const emptyNames = std::count_if(kmlData->m_tracksData.begin(), kmlData->m_tracksData.end(), ```
cyber-toad (Migrated from github.com) reviewed 2023-05-30 15:42:16 +00:00
map/bookmark_helpers.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 15:42:16 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 15:42:38 +00:00
map/bookmark_helpers.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 15:42:37 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 15:43:11 +00:00
map/bookmark_helpers.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 15:43:11 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 15:43:55 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 15:43:54 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 18:18:30 +00:00
iphone/Maps/OMaps.plist
cyber-toad (Migrated from github.com) commented 2023-05-30 18:18:30 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 20:00:00 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 20:00:00 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 20:02:16 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 20:02:16 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 20:04:05 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 20:04:05 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-30 20:07:39 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-30 20:07:39 +00:00

Added consts

Added consts
cyber-toad (Migrated from github.com) reviewed 2023-05-30 20:09:17 +00:00
kml/serdes_common.hpp
cyber-toad (Migrated from github.com) commented 2023-05-30 20:09:17 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-31 06:13:59 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-31 06:13:58 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-31 06:14:48 +00:00
kml/serdes_common.hpp
cyber-toad (Migrated from github.com) commented 2023-05-31 06:14:47 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-31 06:15:24 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-31 06:15:24 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-31 06:22:39 +00:00
kml/serdes_gpx.cpp
cyber-toad (Migrated from github.com) commented 2023-05-31 06:22:39 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-31 18:57:53 +00:00
android/jni/app/organicmaps/Framework.cpp
@ -1108,3 +1091,3 @@
{
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension };
const vector<string> exts = { kKmzExtension, kKmlExtension, kKmbExtension, kGpxExtension };
return jni::ToJavaStringArray(env, exts);
cyber-toad (Migrated from github.com) commented 2023-05-31 18:57:52 +00:00

ok, let's cover it separately once mandatory functionality is implemented

ok, let's cover it separately once mandatory functionality is implemented
cyber-toad (Migrated from github.com) reviewed 2023-05-31 19:02:29 +00:00
kml/serdes_gpx.hpp
@ -0,0 +20,4 @@
{
namespace gpx
{
class GpxParser
cyber-toad (Migrated from github.com) commented 2023-05-31 19:02:28 +00:00

Fixed

Fixed
cyber-toad (Migrated from github.com) reviewed 2023-05-31 19:47:25 +00:00
kml/kml_tests/gpx_tests.cpp
cyber-toad (Migrated from github.com) commented 2023-05-31 19:47:25 +00:00

Fixed

Fixed
biodranik (Migrated from github.com) reviewed 2023-05-31 21:42:58 +00:00
map/bookmark_manager.cpp
biodranik (Migrated from github.com) commented 2023-05-31 21:42:57 +00:00

This line fixes the broken loading of kmz files.

      if (ext == ".kml" || ext == ".kmz")
This line fixes the broken loading of kmz files. ```suggestion if (ext == ".kml" || ext == ".kmz") ```
cyber-toad (Migrated from github.com) reviewed 2023-06-01 05:04:06 +00:00
map/bookmark_manager.cpp
cyber-toad (Migrated from github.com) commented 2023-06-01 05:04:05 +00:00

thanks for suggestion, fixed

thanks for suggestion, fixed
cyber-toad (Migrated from github.com) reviewed 2023-06-04 08:31:59 +00:00
kml/serdes_gpx.hpp
@ -0,0 +85,4 @@
reader.ReadAsString(gpxText);
if (!gpxText.empty() && gpxText[0] == '<')
LOG(LWARNING, (gpxText));
MYTHROW(DeserializeException, ("Could not parse GPX."));
cyber-toad (Migrated from github.com) commented 2023-06-04 08:31:59 +00:00

I have a difficulty with testing it on Debug or with unit-test - it logs with error level if parser fails
coding/parse_xml.hpp:80
so both debug version on Android and unit test fails with "Abort. Log level is too serious" - what is a valid way to test it?

I have a difficulty with testing it on Debug or with unit-test - it logs with error level if parser fails coding/parse_xml.hpp:80 so both debug version on Android and unit test fails with "Abort. Log level is too serious" - what is a valid way to test it?
cyber-toad (Migrated from github.com) reviewed 2023-06-04 08:46:55 +00:00
kml/serdes_gpx.cpp
@ -0,0 +245,4 @@
DeserializerGpx::DeserializerGpx(FileData & fileData)
: m_fileData(fileData)
{
m_fileData = {};
cyber-toad (Migrated from github.com) commented 2023-06-04 08:46:55 +00:00

I think that it was copy-pasted from KML, where it was added here: b1ff412810 (diff-c16f904482bd8eb25901b3bee393895ac9744cd2495235cd1a2f6ecd509f8951R869)
I tried to remove it for gpx & kml and kml_tests passed, but I don't have a full understanding what happens with memory there - we just re-initialize memory passed to us to put deserializer result there?

I think that it was copy-pasted from KML, where it was added here: https://git.omaps.dev/organicmaps/organicmaps/commit/b1ff41281021baaf0170ca9716b22f1008c99719#diff-c16f904482bd8eb25901b3bee393895ac9744cd2495235cd1a2f6ecd509f8951R869 I tried to remove it for gpx & kml and kml_tests passed, but I don't have a full understanding what happens with memory there - we just re-initialize memory passed to us to put deserializer result there?
biodranik (Migrated from github.com) reviewed 2023-06-04 10:30:56 +00:00
kml/serdes_gpx.cpp
@ -0,0 +70,4 @@
if (mercator::ValidX(m_org.x) && mercator::ValidY(m_org.y))
{
// Set default name.
if (m_name.empty())
biodranik (Migrated from github.com) commented 2023-06-04 10:30:56 +00:00

The corrected question:
Should a similar empty name check be done for non-points (tracks) too, below in the GEOMETRY_TYPE_LINE if? How and when the default track name is generated if it's empty?

The corrected question: Should a similar empty name check be done for non-points (tracks) too, below in the GEOMETRY_TYPE_LINE if? How and when the default track name is generated if it's empty?
biodranik (Migrated from github.com) reviewed 2023-06-04 10:37:23 +00:00
kml/serdes_gpx.hpp
@ -0,0 +85,4 @@
reader.ReadAsString(gpxText);
if (!gpxText.empty() && gpxText[0] == '<')
LOG(LWARNING, (gpxText));
MYTHROW(DeserializeException, ("Could not parse GPX."));
biodranik (Migrated from github.com) commented 2023-06-04 10:37:23 +00:00 
  kml::FileData dataFromFile;
  try
  {
    kml::binary::DeserializerKml des(dataFromFile);
    FileReader reader(kmbFile);
    des.Deserialize(reader);
  }
  catch (FileReader::Exception const & exc)
  {
    TEST(false, ("Exception raised", exc.what()));
  }
``` kml::FileData dataFromFile; try { kml::binary::DeserializerKml des(dataFromFile); FileReader reader(kmbFile); des.Deserialize(reader); } catch (FileReader::Exception const & exc) { TEST(false, ("Exception raised", exc.what())); } ```
biodranik (Migrated from github.com) reviewed 2023-06-04 10:37:48 +00:00
kml/serdes_gpx.hpp
@ -0,0 +85,4 @@
reader.ReadAsString(gpxText);
if (!gpxText.empty() && gpxText[0] == '<')
LOG(LWARNING, (gpxText));
MYTHROW(DeserializeException, ("Could not parse GPX."));
biodranik (Migrated from github.com) commented 2023-06-04 10:37:48 +00:00

Use the necessary exception type if you expect it.

Use the necessary exception type if you expect it.
biodranik (Migrated from github.com) reviewed 2023-06-04 10:40:58 +00:00
kml/serdes_gpx.cpp
@ -0,0 +245,4 @@
DeserializerGpx::DeserializerGpx(FileData & fileData)
: m_fileData(fileData)
{
m_fileData = {};
biodranik (Migrated from github.com) commented 2023-06-04 10:40:57 +00:00

False alarm. This code correctly clears the output structure before writing to it.

False alarm. This code correctly clears the output structure before writing to it.
biodranik (Migrated from github.com) approved these changes 2023-06-04 17:00:48 +00:00
biodranik (Migrated from github.com) left a comment

Please add an exception test in a follow-up pull request. We'll prepare these changes for the release.

Please add an exception test in a follow-up pull request. We'll prepare these changes for the release.
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
biodranik
Labels
Clear labels   
Accessibility

issues affecting users with specific needs

  
Accessibility

issues affecting users with specific needs

  
Address

Issues with addresses, post codes, city/town/village borders

  
Address

Issues with addresses, post codes, city/town/village borders

  
Android

Android development

  
Android

Android development

  
Android Auto

Android Auto

  
Android Auto

Android Auto

  
Android Automotive (AAOS)

   
Android Automotive (AAOS)

   
API

Short Links, deep links and url schemes, including ge0 and om://

  
API

Short Links, deep links and url schemes, including ge0 and om://

  
AppGallery

Huawei AppGallery

  
AppGallery

Huawei AppGallery

  
AppStore

Apple AppStore

  
AppStore

Apple AppStore

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Blocker

An issue blocking the release

  
Blocker

An issue blocking the release

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Borders

Country borders and polygons

  
Borders

Country borders and polygons

  
Bug

Something isn't working

  
Bug

Something isn't working

  
Build

Compilation issue

  
Build

Compilation issue

  
CarPlay

CarPlay Integration

  
CarPlay

CarPlay Integration

  
Classificator

Classify object by types

  
Classificator

Classify object by types

  
Community

Community relations

  
Community

Community relations

  
Core

Cross-platform C++ libraries with a core functionality

  
Core

Cross-platform C++ libraries with a core functionality

  
CrashReports

Crash Reporting

  
CrashReports

Crash Reporting

  
Cycling

Bike, cycle, biking and related

  
Cycling

Bike, cycle, biking and related

  
Desktop

Desktop application

  
Desktop

Desktop application

  
DevEx

Developer Experience

  
DevEx

Developer Experience

  
DevOps

Automation and pipelines

  
DevOps

Automation and pipelines

  
dev_sandbox

A desktop app to debug rendering

  
dev_sandbox

A desktop app to debug rendering

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Documentation

Improvements or additions to documentation

  
Documentation

Improvements or additions to documentation

  
Downloader

Map dowloader

  
Downloader

Map dowloader

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Driving

Drive a car related issues

  
Driving

Drive a car related issues

  
Duplicate

This issue or pull request already exists

  
Duplicate

This issue or pull request already exists

  
Editor

OSM Editor

  
Editor

OSM Editor

  
Elevation

Isolines and altitude

  
Elevation

Isolines and altitude

  
Enhancement

New feature or request, an improvement of some existing feature

  
Enhancement

New feature or request, an improvement of some existing feature

  
Epic

A large body work

  
Epic

A large body work

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
F-Droid

F-Droid build and distribution

  
F-Droid

F-Droid build and distribution

  
Fonts

Font and text rendering

  
Fonts

Font and text rendering

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Fund

   
Fund

   
Generator

Related to generator tool.

  
Generator

Related to generator tool.

  
Good first issue

Good for newcomers

  
Good first issue

Good for newcomers

  
Google Play

Google Play

  
Google Play

Google Play

  
GPS

Location and positioning issues

  
GPS

Location and positioning issues

  
GSoC

Ideas for Google Summer of Code

  
GSoC

Ideas for Google Summer of Code

  
iCloud

   
iCloud

   
Icons

Map and app icons

  
Icons

Map and app icons

  
iOS

iOS development

  
iOS

iOS development

  
Legal

Legal aspects

  
Legal

Legal aspects

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux packaging

   
Linux packaging

   
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Map Data

OpenStreetMap data related issues

  
Map Data

OpenStreetMap data related issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Need Feedback

Further information is requested

  
Need Feedback

Further information is requested

  
Night Mode

Night or dark mode feature

  
Night Mode

Night or dark mode feature

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Outdoors

Hiking, mtb, 4x4...

  
Outdoors

Hiking, mtb, 4x4...

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
Privacy

Privacy

  
Privacy

Privacy

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Raw Idea

An idea that needs to be elaborated

  
Raw Idea

An idea that needs to be elaborated

  
Refactoring

   
Refactoring

   
Regional

This issue is different depending on the country or region

  
Regional

This issue is different depending on the country or region

  
Regression

A regression bug

  
Regression

A regression bug

  
Releases

Meta-tickets for tracking the release procedure

  
Releases

Meta-tickets for tracking the release procedure

  
RoboTest

Robo / Monkey Auto Tests

  
RoboTest

Robo / Monkey Auto Tests

  
Route Planning

Preview and plan your track

  
Route Planning

Preview and plan your track

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Ruler

Helicopter routing, "as a crow flies"

  
Ruler

Helicopter routing, "as a crow flies"

  
Search

Search

  
Search

Search

  
Security

Security improvements

  
Security

Security improvements

  
Styles

Map drawing styles

  
Styles

Map drawing styles

  
Tests

   
Tests

   
Track Recording

Track Recording

  
Track Recording

Track Recording

  
Translations

Localization and translations issues

  
Translations

Localization and translations issues

  
TTS

Text-to-Speech

  
TTS

Text-to-Speech

  
UI

User interface issues

  
UI

User interface issues

  
UX

User eXperience, an issue with usability

  
UX

User eXperience, an issue with usability

  
Walk Navigation

Foot pedestrian mode

  
Walk Navigation

Foot pedestrian mode

  
Watches

Android Watches or Apple Watches support

  
Watches

Android Watches or Apple Watches support

  
Web

https://organicmaps.app/

  
Web

https://organicmaps.app/

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Windows

Windows

  
Windows

Windows

  
Won't fix

Unfeasible or impossible to resolve

  
Won't fix

Unfeasible or impossible to resolve

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

No labels
Accessibility
Accessibility
Address
Address
Android
Android
Android Auto
Android Auto
Android Automotive (AAOS)
Android Automotive (AAOS)
API
API
AppGallery
AppGallery
AppStore
AppStore
Battery and Performance
Battery and Performance
Blocker
Blocker
Bookmarks and Tracks
Bookmarks and Tracks
Borders
Borders
Bug
Bug
Build
Build
CarPlay
CarPlay
Classificator
Classificator
Community
Community
Core
Core
CrashReports
CrashReports
Cycling
Cycling
Desktop
Desktop
DevEx
DevEx
DevOps
DevOps
dev_sandbox
dev_sandbox
Directions
Directions
Documentation
Documentation
Downloader
Downloader
Drape
Drape
Driving
Driving
Duplicate
Duplicate
Editor
Editor
Elevation
Elevation
Enhancement
Enhancement
Epic
Epic
External Map Datasets
External Map Datasets
F-Droid
F-Droid
Fonts
Fonts
Frequently User Reported
Frequently User Reported
Fund
Fund
Generator
Generator
Good first issue
Good first issue
Google Play
Google Play
GPS
GPS
GSoC
GSoC
iCloud
iCloud
Icons
Icons
iOS
iOS
Legal
Legal
Linux Desktop
Linux Desktop
Linux packaging
Linux packaging
Linux Phone
Linux Phone
Mac OS
Mac OS
Map Data
Map Data
Metro
Metro
Navigation
Navigation
Need Feedback
Need Feedback
Night Mode
Night Mode
NLnet 2024-06-281
NLnet 2024-06-281
No Feature Parity
No Feature Parity
Opening Hours
Opening Hours
Outdoors
Outdoors
POI Info
POI Info
Privacy
Privacy
Public Transport
Public Transport
Raw Idea
Raw Idea
Refactoring
Refactoring
Regional
Regional
Regression
Regression
Releases
Releases
RoboTest
RoboTest
Route Planning
Route Planning
Routing
Routing
Ruler
Ruler
Search
Search
Security
Security
Styles
Styles
Tests
Tests
Track Recording
Track Recording
Translations
Translations
TTS
TTS
UI
UI
UX
UX
Walk Navigation
Walk Navigation
Watches
Watches
Web
Web
Wikipedia
Wikipedia
Windows
Windows
Won't fix
Won't fix
World Map
World Map
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#5166
No description provided.
Delete branch "gpx"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?