[android] Show the speed limit like in iOS #3817
Conversation
|
I have concerns about the whole value fitting once the numbers are in double- or triple-digits. Need to try it while driving and eventually adjust the font size for the display. |
It could be easily tested on Android Emulator. |
It's not about Android Emulator but this being a Friday night, the end of a work week. Simple solutions seem the hardest to think of right now. ;) |
|
You can import GPX/KML file (from Google Earth for example) or create a route like I did. I hope you understand from the screenshot: |
|
Thanks, @pm4rcin, I did not know about that feature. I did test in the most simple way possible, by setting the string to "180 (130)" and checking it on the phone. |
Seems fairly straightforward by following the instructions. I did not experience any issues. The sub-repos need to be fetched and the That's a really cool tip about the driving simulation. I'll have to try that. OsmAnd can create GPX routes easily. |
|
Can you add [android] before PR name to trigger Android workflow ;) |
alensiljak
changed the title
|
@pm4rcin, could you tell me how to get the route (GPX) into Organic Maps, please? If I choose OM for opening the GPX file from i.e. Material Files, nothing happens. And I don't see options to import directly from inside the app. |
|
@alensiljak I don't think GPX import is supported in any way currently. See #624 |
|
What is this |
Oh, right. The speed is apparently expressed in meters per second. I need to convert it to |
|
The decimals are removed for display. That would explain the slight differences in numbers. |
|
Nice! It'd be good to display the speed limit as iOS does, in the format |
|
@alensiljak you can convert GPX to KML using any online convertor. |
|
Guys, thanks for all the tips so far! I now have a decent setup and toolbox, and can hopefully debug and check other issues that I experience, like speed cameras. The conversion and formatting are now in place and I'd say the PR is functionally ready. Please review. Edit: FYI, I found https://gpx.studio/ to be quite convenient in providing GPX routes with defined travel speed. This is quite helpful when combined with Android Studio feature of simulating location and movement. Thanks, @pm4rcin! |
|
Oh, the code formatting sucks. The change is now really hard to identify on GitHub. I've tried throwing an Exception and then removed that code, reformatting the file to fix the spacing. Now the whole file is indicated as changed. :S |
|
The question why, I believe I have answered in my comment above. |
|
@alensiljak I have cleaned up the patch diff --git a/android/jni/app/organicmaps/Framework.cpp b/android/jni/app/organicmaps/Framework.cpp
index db284b9d64..9cf36b600a 100644
--- a/android/jni/app/organicmaps/Framework.cpp
+++ b/android/jni/app/organicmaps/Framework.cpp
@@ -1185,7 +1185,7 @@ Java_app_organicmaps_Framework_nativeGetRouteFollowingInfo(JNIEnv * env, jclass)
"(Ljava/lang/String;Ljava/lang/String;"
"Ljava/lang/String;Ljava/lang/String;"
"Ljava/lang/String;Ljava/lang/String;DIIIII"
- "[Lapp/organicmaps/routing/SingleLaneInfo;ZZ)V");
+ "[Lapp/organicmaps/routing/SingleLaneInfo;ZZD)V");
vector<routing::FollowingInfo::SingleLaneInfoClient> const & lanes = info.m_lanes;
jobjectArray jLanes = nullptr;
@@ -1221,7 +1221,8 @@ Java_app_organicmaps_Framework_nativeGetRouteFollowingInfo(JNIEnv * env, jclass)
jni::ToJavaString(env, info.m_turnUnitsSuffix), jni::ToJavaString(env, info.m_sourceName),
jni::ToJavaString(env, info.m_displayedStreetName), info.m_completionPercent, info.m_turn,
info.m_nextTurn, info.m_pedestrianTurn, info.m_exitNum, info.m_time, jLanes,
- static_cast<jboolean>(isSpeedCamLimitExceeded), static_cast<jboolean>(shouldPlaySignal));
+ static_cast<jboolean>(isSpeedCamLimitExceeded), static_cast<jboolean>(shouldPlaySignal),
+ info.m_speedLimitMps);
ASSERT(result, (jni::DescribeException()));
return result;
}
diff --git a/android/res/layout/layout_nav_bottom_numbers.xml b/android/res/layout/layout_nav_bottom_numbers.xml
index b4614f7f3d..aea50575fd 100644
--- a/android/res/layout/layout_nav_bottom_numbers.xml
+++ b/android/res/layout/layout_nav_bottom_numbers.xml
@@ -13,7 +13,7 @@
android:id="@+id/speed_view_container"
android:layout_width="0dp"
android:layout_height="match_parent"
- android:layout_weight="1"
+ android:layout_weight="1.2"
android:background="@drawable/speed_cams_bg"
android:gravity="center"
android:minWidth="@dimen/nav_numbers_side_min_width"
@@ -28,8 +28,7 @@
android:includeFontPadding="false"
android:lines="1"
android:textAppearance="@style/MwmTextAppearance.NavMenu.Number"
- tools:text="999" />
-
+ tools:text="999 (999)" />
<!-- Speed -->
<TextView
android:id="@+id/speed_dimen"
@@ -48,7 +47,7 @@
<RelativeLayout
android:layout_width="0dp"
android:layout_height="match_parent"
- android:layout_weight="1.5"
+ android:layout_weight="1.2"
android:gravity="center"
android:minWidth="@dimen/nav_numbers_side_min_width"
android:paddingStart="@dimen/nav_numbers_margin"
diff --git a/android/src/app/organicmaps/routing/RoutingInfo.java b/android/src/app/organicmaps/routing/RoutingInfo.java
index 23d0503d26..972b06588e 100644
--- a/android/src/app/organicmaps/routing/RoutingInfo.java
+++ b/android/src/app/organicmaps/routing/RoutingInfo.java
@@ -27,6 +27,7 @@ public class RoutingInfo
public final CarDirection nextCarDirection;
public final int exitNum;
public final SingleLaneInfo[] lanes;
+ public final double speedLimit;
// For pedestrian routing.
public final PedestrianTurnDirection pedestrianTurnDirection;
private final boolean speedLimitExceeded;
@@ -157,7 +158,7 @@ public class RoutingInfo
public RoutingInfo(String distToTarget, String units, String distTurn, String turnSuffix, String currentStreet, String nextStreet, double completionPercent,
int vehicleTurnOrdinal, int vehicleNextTurnOrdinal, int pedestrianTurnOrdinal, int exitNum,
int totalTime, SingleLaneInfo[] lanes, boolean speedLimitExceeded,
- boolean shouldPlayWarningSignal)
+ boolean shouldPlayWarningSignal, double speedLimit)
{
this.distToTarget = distToTarget;
this.targetUnits = units;
@@ -174,6 +175,7 @@ public class RoutingInfo
this.pedestrianTurnDirection = PedestrianTurnDirection.values()[pedestrianTurnOrdinal];
this.speedLimitExceeded = speedLimitExceeded;
this.shouldPlayWarningSignal = shouldPlayWarningSignal;
+ this.speedLimit = speedLimit;
}
public boolean isSpeedLimitExceeded()
diff --git a/android/src/app/organicmaps/widget/menu/NavMenu.java b/android/src/app/organicmaps/widget/menu/NavMenu.java
index 32cd16d337..bff3a7d6b8 100644
--- a/android/src/app/organicmaps/widget/menu/NavMenu.java
+++ b/android/src/app/organicmaps/widget/menu/NavMenu.java
@@ -208,11 +208,49 @@ public class NavMenu
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
+ // (temporarily) show the speed limit together with the current speed.
+ String speedIndicator = formatSpeed(speedAndUnits, info);
+ mSpeedValue.setText(speedIndicator);
+
mSpeedUnits.setText(speedAndUnits.second);
- mSpeedValue.setText(speedAndUnits.first);
mSpeedViewContainer.setActivated(info.isSpeedLimitExceeded());
}
+ private String formatSpeed(Pair<String, String> speedAndUnits, RoutingInfo info)
+ {
+ double currentSpeedMps = Double.parseDouble(speedAndUnits.first);
+ String speedIndicator = String.format("%.0f", currentSpeedMps);
+
+ double speedLimit = convertMpsIntoUnits(info.speedLimit, speedAndUnits.second);
+ if (speedLimit > 0.0)
+ {
+ speedIndicator += String.format("/%.0f", speedLimit);
+ }
+
+ return speedIndicator;
+ }
+
+ private double convertMpsIntoUnits(double speedInMps, String units)
+ {
+ double result;
+
+ switch (units)
+ {
+ case "km/h":
+ // 1 m/s = 3600s/1000m = 3.6 km/h
+ result = speedInMps * 3.6;
+ break;
+ case "mph":
+ // 1 m/s = 2.2369 mph
+ result = speedInMps * 2.2369;
+ break;
+ default:
+ // should throw an exception. Use km/h as a workaround.
+ result = speedInMps * 3.6;
+ }
+ return result;
+ }
+
public void update(@NonNull RoutingInfo info)
{
updateSpeedView(info); |
Thank you so much! That saves a ton of time. What are the next steps? |
|
You can reset your local branch and save the diff to the file and then |
alensiljak
force-pushed
the
speed-limit-android
branch
from
dc22ff6
to
77153c2
Compare
|
I'm getting |
|
The PR seems to have closed automatically when the branch was reset. |
alensiljak
requested review from
RedAuburn,
biodranik and
pm4rcin
and removed request for
biodranik,
RedAuburn and
pm4rcin
Signed-off-by: Alen Šiljak <dev@alensiljak.eu.org>
| result = speedInMps * 3.6; | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Why not to use existing approach? Like:
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
There was a problem hiding this comment.
In this case, the unit is already known to the caller and that would somewhat obfuscate the point of the function, I think.
There was a problem hiding this comment.
Is this duplication really necessary?
| if (info.speedLimit > 0.0) | ||
| { | ||
| double speedLimit = convertMpsIntoUnits(info.speedLimit, speedAndUnits.second); | ||
| speedIndicator += String.format("/%.0f", speedLimit); |
There was a problem hiding this comment.
[Pure suggestion as I would love to see this PR merged even without it and I wouldn't slow down the reviewing process]
iOS pr use space around the slash:
| speedIndicator += String.format("/%.0f", speedLimit); | |
| speedIndicator += String.format(" / %.0f", speedLimit); |
There was a problem hiding this comment.
iOS use space around the slash:
The initial implementation was like that. Unfortunately, it takes too much screen space, in my opinion. Hence I narrowed it down.
There was a problem hiding this comment.
Oh ! ok. Sorry I didn't see this from the history because of the force push.
Is there anything I can do to help the merge of this pr ?
Speed limit information is one of key features I really love to see in Organic Maps
| result = speedInMps * 3.6; | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Is this duplication really necessary?
|
Looks like the issue was fixed |
|
@TheAdventurer64 no, this PR is for Android |
|
Just wanted to inform you that maps.me has implemented this feature.
And when you are too fast, it changes the color:
|
They're currently proprietary. "Their" source code is 2 years old already. Organic Maps is the fork of the Maps.me repo. |
|
@alensiljak what is the current status here? |
Hi. I'm probably the wrong person to ask. I've implemented a simple display (i.e. "50/70 kmh") of the speed limit, if available. |
|
An other PR to implement speed limit was already started here #5233 |
"Already"? The new one certainly looks the way it should. |
rtsisyk
changed the title
We will merge this text version first and think about a new visual signs later. |
|
I think this PR can be closed There is a PR with UI implementation of speed limit: #7005 |
Implements #952, text-only