Update:Android:GUI/Internal: Show contextual GUI menu when receiving geo intent #987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…still needs cleanup and OSD disabling)
lains
added
enhancement
|
This is the implementation of what I was proposing in a comment on my previous PR (#812 (comment)) |
|
Hello All, I have an issue with this current implementation. |
|
I think I found a solution about access to |
jandegr
reviewed
Apr 19, 2020
| @@ -646,7 +646,7 @@ private void setmActivity(final Navit navit) { | |||
|
|
|||
| private native void motionCallback(long id, int x, int y); | |||
|
|
|||
| private native String getCoordForPoint(int x, int y, boolean absoluteCoord); | |||
| private static native String getCoordForPoint(int x, int y, boolean absoluteCoord); | |||
There was a problem hiding this comment.
Hi, can you give a motivation for this, does it solve or change anything ? thx.
There was a problem hiding this comment.
Hi, I searched a bit further and you have a point in changing it to static when it comes to MIM as described here #964. Just if you change it to static you will have to change jobject to jclass in the signature there https://github.com/navit-gps/navit/blob/trunk/navit/android.c#L325 for consistency, thx.
There was a problem hiding this comment.
Hi, yes, that was the reason for this change. This method does not access any member of the class.
I agree it has nothing to do with the feature in this PR. I just fixed this on the way while browsing through the code I wrote earlier in the previously related PR #794.
But you're right, this is not required to implement the feature described in this current PR.
There was a problem hiding this comment.
Thanks for the review, I've just changed this.
|
Unfortunately, circle CI build for Android fails due to an external issue: |
|
Getting there. OSD icons are not displayed anymore and internal GUI submenu for the specific geo coord is now activated properly. |
stealing internal menu bringup sequence from gui_internal_cmd_log()
|
I fixed the display of the internal menu by stealing menu bringup sequence from |
…ternal_enter(), invoked by gui_internal_show_coord_actions()
|
@lains still WIP? |
|
Most of the work is done, menu is displayed when navit is already running. |
|
Retested today, here is the crash I'm getting when navit is not yet started when the geo intent is fired: Using |
|
Hi
So apparently it does not survive the initial resize. I tested it with Navit already running before firing the intent and then it returned to mapview instead of handling the resize in gui internal as other gui screens do when rotating the device (causes a resize too). It might be usefull to debug this |
|
I gave it a try yesterday. I confirm (while digging a bit more the crash occuring when the intent fires while navit is not running) that there is a race condition between internal GUI startup and the intent processing in |
|
In order not to get the crash, it looks like the internal GUI contextual menu must only be invoked after |
|
Could someone give a try to this new version for Android and check that it is running better, without crash, whether navit is already started or not when the intent is fired? |
|
Hi, I got my build environment back up. I will start testing again to sort out the race condition at startup. |
…ub.com/lains/navit into geo-coord-show-actions-on-internal-gui
|
I'm having issues with navit crashing at startup, just as soon as I accept access to GPS and SD card. But it looks like I'm getting the same behaviour even when building from trunk, not only when including the code from this PR... |
|
Hello all. If anyone wants to test this, here is what to to: What needs to be tested:
|
lains
changed the title
|
Hi All, it looks like the feature in this PR is behaving as expected. Scanning a geo QR code while navigation is running, or when the navit application is not launched both work. Anyone interested in testing? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In my previous PR #812, the default action when receiving a geo intent (for example by scanning a geo QR code), is to start navigating to the provided location.
This PR aims to give the user more control over which action is started when navit receives a geo intent.
For this (and only implemented when using the internal GUI), I bring up the contextual submenu for a geo position in the internal GUI, and let the user select what to do with this position (add bookmark, set as waypoint, destination, search for nearby POIs), in the same way as when the geo pos is entered manually in the internal GUI.
This is only for Android, and if this feature is not implemented in the active GUI plugin (only implemented in internal GUI for now), the behaviour will revert back to set destination to the GPS coords.