[android] Rework Public API

2022-07-04T12:07:59.1767080Z In file included from /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/DeepLinkParser.mm:5:
2022-07-04T12:07:59.1768290Z /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: warning: enumeration value 'Coordinates' not handled in switch [-Wswitch]
2022-07-04T12:07:59.1769030Z   switch (type)
2022-07-04T12:07:59.1769510Z           ^
2022-07-04T12:07:59.1770170Z /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: note: add missing switch cases
2022-07-04T12:07:59.1770830Z   switch (type)
2022-07-04T12:07:59.1773320Z           ^
2022-07-04T12:07:59.1774680Z /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:15:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type]
2022-07-04T12:07:59.1776120Z }
2022-07-04T12:07:59.1777070Z ^
2022-07-04T12:08:04.4029950Z 1 warning and 1 error generated.

``` 2022-07-04T12:07:59.1767080Z In file included from /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/DeepLinkParser.mm:5: 2022-07-04T12:07:59.1768290Z /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: warning: enumeration value 'Coordinates' not handled in switch [-Wswitch] 2022-07-04T12:07:59.1769030Z switch (type) 2022-07-04T12:07:59.1769510Z ^ 2022-07-04T12:07:59.1770170Z /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: note: add missing switch cases 2022-07-04T12:07:59.1770830Z switch (type) 2022-07-04T12:07:59.1773320Z ^ 2022-07-04T12:07:59.1774680Z /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:15:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type] 2022-07-04T12:07:59.1776120Z } 2022-07-04T12:07:59.1777070Z ^ 2022-07-04T12:08:04.4029950Z 1 warning and 1 error generated. ```

biodranik commented

2022-07-04 21:31:21 +00:00

(Migrated from github.com)

Сложно ли нам пока это оставить и поддержать? Какие плюсы от выпиливания?

biodranik commented

2022-07-04 21:33:25 +00:00

(Migrated from github.com)

Зачем всё выпиливать? Как оно сейчас будет работать?

Может просто сделать ApiProcessor и там все наши штуки с om и mwm обрабатывать?

Зачем всё выпиливать? Как оно сейчас будет работать? Может просто сделать ApiProcessor и там все наши штуки с om и mwm обрабатывать?

biodranik commented

2022-07-04 21:34:34 +00:00

(Migrated from github.com)

Плохое название Position, кажется что это связано с текущей позицией пользователя. А нам надо, если я правильно понял, любая точка на карте. PointChooserMode более универсально.

biodranik commented

2022-07-04 21:35:11 +00:00

(Migrated from github.com)

            final Intent apiResult = new Intent();

```suggestion:-0+0 final Intent apiResult = new Intent(); ```

biodranik commented

2022-07-04 21:35:23 +00:00

(Migrated from github.com)

            final double[] center = Framework.nativeGetScreenRectCenter();

```suggestion:-0+0 final double[] center = Framework.nativeGetScreenRectCenter(); ```

biodranik commented

2022-07-04 21:36:48 +00:00

(Migrated from github.com)

Не люблю когда однострочные комментарии в сишном стиле, неудобно блоки кода коммментить.

biodranik commented

2022-07-04 21:38:34 +00:00

(Migrated from github.com)

Удаляй.

biodranik commented

2022-07-04 21:40:51 +00:00

(Migrated from github.com)

Зачем в сёрч баре отображать тайтл аппы, как будто оно было в поиске вбито? Надо тогда двойной режим для сёрчбара делать, то он апи, то он поиск, и визуально различать, например цветом. Но лучше может для апи свой тулбар сверху показывать, отдельный? На нём можно будет лепить любые кастомные апи штуки тогда.

biodranik commented

2022-07-04 21:41:37 +00:00

(Migrated from github.com)

crosshair?

biodranik commented

2022-07-04 21:42:19 +00:00

(Migrated from github.com)

Ты эти классы выпилил, как теперь они обрабатываются?

rtsisyk commented

2022-07-05 06:03:56 +00:00

Author

Owner

Зачем поддерживать 3 разные версии одного и того же? Какой смысл?

rtsisyk commented

2022-07-05 06:04:39 +00:00

Author

Owner

Данные intents дублируют om:// urls. Просто еще один способ.

rtsisyk commented

2022-07-05 06:05:32 +00:00

Author

Owner

В текущем коде элемент называется PositionChooser. Мне еще и этот код переименовывать весь?

rtsisyk commented

2022-07-05 06:06:40 +00:00

Author

Owner

И чего делать надо?

rtsisyk commented

2022-07-05 06:07:02 +00:00

Author

Owner

Ну ок.

rtsisyk commented

2022-07-05 06:07:45 +00:00

Author

Owner

Через OpenUrlTask. Данный код дублирует om:// URLs чуть более, чем полностью.

rtsisyk commented

2022-07-05 06:08:36 +00:00

Author

Owner

Мне тоже кажется это странным. Но как можно видеть, здесь я просто не стал удалять старое. Могу, наверное, удалить эту часть.

rtsisyk commented

2022-07-05 06:08:57 +00:00

Author

Owner

Ок

rtsisyk commented

2022-07-05 07:15:36 +00:00

Author

Owner

requested review from @biodranik

requested review from `@biodranik`

biodranik commented

2022-07-06 00:08:35 +00:00

(Migrated from github.com)

Например, какие-то сторонние аппы, которые уже использовали ту старую апи.

biodranik commented

2022-07-06 00:09:05 +00:00

(Migrated from github.com)

Так если через интенты вызывать, всё будет сейчас работать?

biodranik commented

2022-07-06 00:09:45 +00:00

(Migrated from github.com)

Использовать //

biodranik commented

2022-07-06 00:10:29 +00:00

(Migrated from github.com)

Если переименовывать, то лучше уже везде crosshair, будет проще отлаживаться.

biodranik commented

2022-07-06 00:12:27 +00:00

(Migrated from github.com)

Review: Commented

Позже проверю на устройстве. Все тестовые ссылки на https://omaps.app/test работают? Надо туда добавить crosshair.

**Review:** Commented Позже проверю на устройстве. Все тестовые ссылки на https://omaps.app/test работают? Надо туда добавить crosshair.

biodranik commented

2022-07-08 22:36:11 +00:00

(Migrated from github.com)

registerForActivityResult.launch(request)

    private val request = Intent(Intent.ACTION_VIEW, Uri.parse("om://crosshair")).apply {
        putExtra(PICK_POINT, true)
        putExtra(TITLE, "Select an area")
    }

Посылаюю запрос (на онклике кнопки)
Открывается OM, всё хорошо работает
Выбросить OM из стека
Попадаю в тестовое приложение снова
Снова посылаю запрос - меня сразу возвращает обратно в приложение

``` registerForActivityResult.launch(request) private val request = Intent(Intent.ACTION_VIEW, Uri.parse("om://crosshair")).apply { putExtra(PICK_POINT, true) putExtra(TITLE, "Select an area") } ``` 1. Посылаюю запрос (на онклике кнопки) 2. Открывается OM, всё хорошо работает 3. Выбросить OM из стека 4. Попадаю в тестовое приложение снова 5. Снова посылаю запрос - меня сразу возвращает обратно в приложение

rtsisyk commented

2022-07-16 15:16:02 +00:00

Author

Owner

Есть реальные примеры? Не хочется замораживать этот код полностью для мифической совместимости с maps.me.

rtsisyk commented

2022-07-16 15:17:09 +00:00

Author

Owner

Да. Интенты будут с action=VIEW и url=om://map. Параметры передаются через URL. Удаляемый код делал custom action и параметры передавал в самом intent. По сути это дублировало друг друга.

rtsisyk commented

2022-07-16 15:19:12 +00:00

Author

Owner

OK

rtsisyk commented

2022-07-16 15:20:37 +00:00

Author

Owner

Эта опция говорит о том, что необходимо включить режим выбора точки. Режим выбора точки есть в нескольких API:

om://map - юзер ищет на карте, открывает place page и возвращается в оригинальное приложение
om://search - тоже самое, но с открытием поиска
om://crosshair - выбор точки с прицелом.

Наверное, текущее название наиболее понятное?

Эта опция говорит о том, что необходимо включить режим выбора точки. Режим выбора точки есть в нескольких API: - om://map - юзер ищет на карте, открывает place page и возвращается в оригинальное приложение - om://search - тоже самое, но с открытием поиска - om://crosshair - выбор точки с прицелом. Наверное, текущее название наиболее понятное?

rtsisyk commented

2022-07-16 20:20:12 +00:00

Author

Owner

requested review from @biodranik

requested review from `@biodranik`

rtsisyk commented

2022-07-17 18:44:45 +00:00

Author

Owner

Поменял.

rtsisyk commented

2022-07-17 18:46:32 +00:00

Author

Owner

По данной апишке вызывалось om://map и om://route соответственно.
Они просто дублировали om:// API, используя другой способ передачи параметров.
Можно, но зачем здесь две дублирующие друг друга апишки?

1. По данной апишке вызывалось om://map и om://route соответственно. 2. Они просто дублировали om:// API, используя другой способ передачи параметров. 3. Можно, но зачем здесь две дублирующие друг друга апишки?

biodranik commented

2022-07-18 20:45:57 +00:00

(Migrated from github.com)

Ладно, если кто-то использовал, то напишут нам в поддержку.

biodranik commented

2022-07-18 20:46:30 +00:00

(Migrated from github.com)

Но нам разве по аналогии не нужно прописывать подобные интент-фильтры? Почему?

biodranik commented

2022-07-18 20:47:01 +00:00

(Migrated from github.com)

Зачем эта строка здесь?

biodranik commented

2022-07-18 20:47:34 +00:00

(Migrated from github.com)

Java_com_mapswithme_maps_Framework_nativeGetParsedAppTitle(JNIEnv * env, jclass)

```suggestion:-0+0 Java_com_mapswithme_maps_Framework_nativeGetParsedAppTitle(JNIEnv * env, jclass) ```

biodranik commented

2022-07-18 20:48:24 +00:00

(Migrated from github.com)

Это точно поддерживается везде, начиная с API 21 (Android 5)?

biodranik commented

2022-07-18 20:55:15 +00:00

(Migrated from github.com)

    m_appTitle = value;

```suggestion:-0+0 m_appTitle = value; ```

biodranik commented

2022-07-18 20:56:00 +00:00

(Migrated from github.com)

Зачем явные цифры? В плюсах всегда всё с нуля.

biodranik commented

2022-07-18 20:57:39 +00:00

(Migrated from github.com)

Review: Approved

Мелкие комментарии и можно мержить и тестить.

**Review:** Approved Мелкие комментарии и можно мержить и тестить.

biodranik commented

2022-07-18 20:57:39 +00:00

(Migrated from github.com)

approved this merge request

biodranik commented

2022-07-21 17:40:28 +00:00

(Migrated from github.com)

Review: Changes requested

@rtsisyk Исправь ошибки компиляции:

In file included from /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/DeepLinkParser.mm:5:
/Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: warning: enumeration value 'Crosshair' not handled in switch [-Wswitch]
  switch (type)
          ^
/Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: note: add missing switch cases
  switch (type)
          ^
/Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:15:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type]
}
^
1 warning and 1 error generated.

**Review:** Changes requested @rtsisyk Исправь ошибки компиляции: ``` In file included from /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/DeepLinkParser.mm:5: /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: warning: enumeration value 'Crosshair' not handled in switch [-Wswitch] switch (type) ^ /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:8:11: note: add missing switch cases switch (type) ^ /Users/runner/work/organicmaps/organicmaps/iphone/CoreApi/CoreApi/DeepLink/Data/DeeplinkUrlType.h:15:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type] } ^ 1 warning and 1 error generated. ```

rtsisyk commented

2022-07-23 06:20:07 +00:00

Author

Owner

Да пусть будет явно чтобы сигнализировать, что эти значения что-то значат.

rtsisyk commented

2022-07-23 06:23:28 +00:00

Author

Owner

Артефакт

rtsisyk commented

2022-07-23 06:23:43 +00:00

Author

Owner

Открытие URL в любой версии поддерживается.

rtsisyk commented

2022-07-23 06:24:05 +00:00

Author

Owner

requested review from @biodranik

requested review from `@biodranik`

biodranik commented

2022-07-23 10:55:55 +00:00

(Migrated from github.com)

Если оставить явно, то придётся менять цифры при удалении или добавлении значения, они не будут идти по-порядку. Если хочешь как комментарий сделать, так и напиши // = 0

biodranik commented

2022-07-23 10:56:38 +00:00

(Migrated from github.com)

Зачем тут и ниже новые строки?

biodranik commented

2022-07-23 10:58:05 +00:00

(Migrated from github.com)

Я про интенты как раз и спрашиваю, а не про урлы.

biodranik commented

2022-07-23 11:01:13 +00:00

(Migrated from github.com)

title и appid всегда не null? Не будет креша если пустые?

biodranik commented

2022-07-23 11:02:55 +00:00

(Migrated from github.com)

Review: Commented

Как title передать из урла? Зачем переименовывать из name?
Склеишь некоторые коммиты, чтобы было логично и удобно откатываемо?

**Review:** Commented 1. Как title передать из урла? Зачем переименовывать из name? 2. Склеишь некоторые коммиты, чтобы было логично и удобно откатываемо?

rtsisyk commented

2022-07-23 11:47:38 +00:00

Author

Owner

Просто приходит null.

rtsisyk commented

2022-07-23 12:18:54 +00:00

Author

Owner

Текущий код должен работать вообще в любой версии Android.

rtsisyk commented

2022-07-23 12:21:32 +00:00

Author

Owner

Убрал.

rtsisyk commented

2022-07-23 12:22:14 +00:00

Author

Owner

Цифры в любом случае надо просинхронизировать с Android кодом.

rtsisyk commented

2022-07-23 12:28:10 +00:00

Author

Owner

Review: Commented

Squashed && Rebased

**Review:** Commented Squashed && Rebased

rtsisyk commented

2022-07-23 12:29:21 +00:00

Author

Owner

Как title передать из урла?

?apptitle=xxx

Зачем переименовывать из name?

Везде в коде и по сути это Title, лишь сам параметр назывался Name.

> Как title передать из урла? ?apptitle=xxx > Зачем переименовывать из name? Везде в коде и по сути это Title, лишь сам параметр назывался Name.

biodranik commented

2022-07-23 17:51:10 +00:00

(Migrated from github.com)

Везде в коде и по сути это Title, лишь сам параметр назывался Name.

Это сломает старые апишки, какие бы они ни были, и усложнит миграцию.

> Везде в коде и по сути это Title, лишь сам параметр назывался Name. Это сломает старые апишки, какие бы они ни были, и усложнит миграцию.

biodranik commented

2022-07-23 17:53:11 +00:00

(Migrated from github.com)

Review: Changes requested

@rtsisyk Почини, пожалуйста, сломанный тест

FAILED
map_tests/mwm_url_tests.cpp:356 TEST(api.GetAppTitle() == "Google")  Google

**Review:** Changes requested @rtsisyk Почини, пожалуйста, сломанный тест ``` FAILED map_tests/mwm_url_tests.cpp:356 TEST(api.GetAppTitle() == "Google") Google ```

rtsisyk commented

2022-07-25 14:07:38 +00:00

Author

Owner

requested review from @biodranik

requested review from `@biodranik`

Rows
Columns

[android] Rework Public API #2907