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

[android] Show the speed limit like in iOS #3817

Closed
alensiljak wants to merge 2 commits from speed-limit-android into master
pull from: speed-limit-android
merge into: organicmaps:master
Conversation 25 Commits 2 Files changed 4 +48 -7
alensiljak commented 2022-11-04 20:54:55 +00:00 (Migrated from github.com)
Copy link

Implements #952, text-only

Implements #952, text-only
👍 3
alensiljak commented 2022-11-04 20:57:41 +00:00 (Migrated from github.com)
Copy link

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.

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.
pm4rcin commented 2022-11-04 21:06:06 +00:00 (Migrated from github.com)
Copy link

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.

> 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.
alensiljak commented 2022-11-04 21:08:26 +00:00 (Migrated from github.com)
Copy link

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. ;)

> 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. ;)
😆 1
pm4rcin commented 2022-11-04 21:18:54 +00:00 (Migrated from github.com)
Copy link

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:
1667596674-wayshot
EDIT: And you have to speed up simulation from menu so the car moves faster in emulator. I'll try to test by myself if I manage to patch and compile the thing (but can't promise it because I'm new to Android development).

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: ![1667596674-wayshot](https://user-images.githubusercontent.com/37148802/200076333-8f1be325-6dcc-4966-84e1-b335cfe8118a.png) EDIT: And you have to speed up simulation from menu so the car moves faster in emulator. I'll try to test by myself if I manage to patch and compile the thing (but can't promise it because I'm new to Android development).
alensiljak commented 2022-11-04 21:37:30 +00:00 (Migrated from github.com)
Copy link

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.

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.
alensiljak commented 2022-11-04 21:53:23 +00:00 (Migrated from github.com)
Copy link

I'll try to test by myself if I manage to patch and compile the thing (but can't promise it because I'm new to Android development).

Seems fairly straightforward by following the instructions. I did not experience any issues. The sub-repos need to be fetched and the configure.sh needs to be run in Linux environment, i.e. WSL if on Windows.

That's a really cool tip about the driving simulation. I'll have to try that. OsmAnd can create GPX routes easily.

> I'll try to test by myself if I manage to patch and compile the thing (but can't promise it because I'm new to Android development). Seems fairly straightforward by following the instructions. I did not experience any issues. The sub-repos need to be fetched and the `configure.sh` needs to be run in Linux environment, i.e. WSL if on Windows. That's a really cool tip about the driving simulation. I'll have to try that. OsmAnd can create GPX routes easily.
Jean-BaptisteC commented 2022-11-04 22:07:04 +00:00 (Migrated from github.com)
Copy link

Can you add [android] before PR name to trigger Android workflow ;)
More information here

Can you add [android] before PR name to trigger Android workflow ;) More information [here](https://github.com/organicmaps/organicmaps/blob/master/docs/PR_GUIDE.md)
👍 1
alensiljak commented 2022-11-04 22:16:33 +00:00 (Migrated from github.com)
Copy link

@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.
This would be really handy in order to test the issues with speed cameras that I've noticed.

@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. This would be really handy in order to test the issues with speed cameras that I've noticed.
pm4rcin commented 2022-11-04 22:45:38 +00:00 (Migrated from github.com)
Copy link

@alensiljak I don't think GPX import is supported in any way currently. See organicmaps/organicmaps#624

@alensiljak I don't think GPX import is supported in any way currently. See https://git.omaps.dev/organicmaps/organicmaps/issues/624
👍 1
pm4rcin commented 2022-11-04 23:48:05 +00:00 (Migrated from github.com)
Copy link

What is this (36)? It's on motorway with 130pkh limit so it displays a different thing than wanted. When on 80kph limit it's (22) and on 50kph it's (14). Something's wrong with your PR but it's close to displaying correct thing.
1667605175-wayshot

What is this `(36)`? It's on motorway with 130pkh limit so it displays a different thing than wanted. When on 80kph limit it's `(22)` and on 50kph it's `(14)`. Something's wrong with your PR but it's close to displaying correct thing. ![1667605175-wayshot](https://user-images.githubusercontent.com/37148802/200090238-ece2f097-321f-4661-96d7-13de17147c8a.png)
alensiljak commented 2022-11-04 23:51:44 +00:00 (Migrated from github.com)
Copy link

What is this (36)?

Oh, right. The speed is apparently expressed in meters per second. I need to convert it to km/h the selected unit.

> What is this `(36)`? Oh, right. The speed is apparently expressed in meters per second. I need to convert it to ~km/h~ the selected unit.
alensiljak commented 2022-11-05 00:01:07 +00:00 (Migrated from github.com)
Copy link

The decimals are removed for display. That would explain the slight differences in numbers.

<img src="https://user-images.githubusercontent.com/462445/200091061-9342764e-b7a5-4241-926d-4ecc12c5fee7.png" width="300px" /> The decimals are removed for display. That would explain the slight differences in numbers.
RedAuburn commented 2022-11-05 08:08:53 +00:00
Member
Copy link

Nice! It'd be good to display the speed limit as iOS does, in the format currentSpeed/speedLimit (iOS PR for reference: organicmaps/organicmaps#2963)

Nice! It'd be good to display the speed limit as iOS does, in the format `currentSpeed/speedLimit` (iOS PR for reference: https://git.omaps.dev/organicmaps/organicmaps/pulls/2963)
👍 2
biodranik commented 2022-11-05 08:56:16 +00:00 (Migrated from github.com)
Copy link

@alensiljak you can convert GPX to KML using any online convertor.

@alensiljak you can convert GPX to KML using any online convertor.
alensiljak commented 2022-11-05 13:20:01 +00:00 (Migrated from github.com)
Copy link

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!

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!
alensiljak commented 2022-11-05 13:30:19 +00:00 (Migrated from github.com)
Copy link

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

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
biodranik (Migrated from github.com) requested changes 2022-11-06 09:21:31 +00:00
biodranik (Migrated from github.com) left a comment

Why did you change the indentation? We have some rules for code formatting, please try to follow them. There are some code formatters at tools/android/formatter for example.

Why did you change the indentation? We have some rules for code formatting, please try to follow them. There are some code formatters at tools/android/formatter for example.
alensiljak commented 2022-11-07 09:26:24 +00:00 (Migrated from github.com)
Copy link

The question why, I believe I have answered in my comment above.
I have now applied the project formatter. The XML file has changed more than expected. Something might be missing there. I see the value 4 for continuation indent.

The question why, I believe I have answered in my [comment](https://git.omaps.dev/organicmaps/organicmaps/pulls/3817#issuecomment-1304547291) above. I have now applied the project formatter. The XML file has changed more than expected. Something might be missing there. I see the value 4 for continuation indent.
biodranik (Migrated from github.com) requested changes 2022-11-07 09:28:50 +00:00
biodranik (Migrated from github.com) left a comment

I see only unnecessary formatting changes, without an actual improvement.

I see only unnecessary formatting changes, without an actual improvement.
android/res/layout/layout_nav_bottom_numbers.xml
biodranik (Migrated from github.com) commented 2022-11-07 09:24:02 +00:00

In all other files, the indent is 2 spaces

In all other files, the indent is 2 spaces
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +13,4 @@
android:id="@+id/speed_view_container"
android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_weight="1.2"
biodranik (Migrated from github.com) commented 2022-11-07 09:24:23 +00:00

These changes look unnecessary.

These changes look unnecessary.
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +47,4 @@
<!-- Time -->
<RelativeLayout
android:layout_width="0dp"
android:layout_height="match_parent"
biodranik (Migrated from github.com) commented 2022-11-07 09:24:54 +00:00

Please don't commit unnecessary changes.

Please don't commit unnecessary changes.
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +112,4 @@
android:textAppearance="@style/MwmTextAppearance.NavMenu.Number.Dimension"
tools:text="99:99 AM" />
</RelativeLayout>
biodranik (Migrated from github.com) commented 2022-11-07 09:25:59 +00:00
  1. Unnecessary formatting
  2. Unnecessary swap of paddingStart and paddingEnd
1. Unnecessary formatting 2. Unnecessary swap of paddingStart and paddingEnd
biodranik (Migrated from github.com) commented 2022-11-07 09:26:11 +00:00

The same.

The same.
alensiljak (Migrated from github.com) reviewed 2022-11-07 09:31:37 +00:00
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +13,4 @@
android:id="@+id/speed_view_container"
android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_weight="1.2"
alensiljak (Migrated from github.com) commented 2022-11-07 09:31:36 +00:00

I didn't make those changes. I was editing the file in the visual editor. For everything else you can blame Google and JetBrains. The formatting is applied using the project formatter for Android Studio you mentioned above.

I didn't make those changes. I was editing the file in the visual editor. For everything else you can blame Google and JetBrains. The formatting is applied using the project formatter for Android Studio you mentioned above.
pm4rcin (Migrated from github.com) reviewed 2022-11-07 09:32:04 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +206,4 @@
if (last == null)
return;
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
pm4rcin (Migrated from github.com) commented 2022-11-07 09:32:04 +00:00

A bit off-topic but I'm wondering why do people often use 18/5 when converting to kph when it's just 3.6? Is it easier to remember or what?

A bit off-topic but I'm wondering why do people often use 18/5 when converting to kph when it's just 3.6? Is it easier to remember or what?
alensiljak (Migrated from github.com) reviewed 2022-11-07 09:34:31 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +206,4 @@
if (last == null)
return;
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
alensiljak (Migrated from github.com) commented 2022-11-07 09:34:31 +00:00

It is easier to copy/paste from a search result for "m/s to mph" ;)

It is easier to copy/paste from a search result for "m/s to mph" ;)
biodranik (Migrated from github.com) reviewed 2022-11-07 09:55:06 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +206,4 @@
if (last == null)
return;
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
biodranik (Migrated from github.com) commented 2022-11-07 09:55:06 +00:00

3600 / 1000 is way better and more obvious than 18/5

`3600 / 1000` is way better and more obvious than 18/5
biodranik (Migrated from github.com) reviewed 2022-11-07 09:56:52 +00:00
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +13,4 @@
android:id="@+id/speed_view_container"
android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_weight="1.2"
biodranik (Migrated from github.com) commented 2022-11-07 09:56:51 +00:00

It doesn't matter who made these changes. Our repository doesn't need them. There are zero benefits in merging them. They pollute the project git history. Please undo them.

We generally don't apply automatic code formatters, but stick to the defined code style.

It doesn't matter who made these changes. Our repository doesn't need them. There are zero benefits in merging them. They pollute the project git history. Please undo them. We generally don't apply automatic code formatters, but stick to the defined code style.
pm4rcin (Migrated from github.com) reviewed 2022-11-07 10:10:23 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +206,4 @@
if (last == null)
return;
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
pm4rcin (Migrated from github.com) commented 2022-11-07 10:10:22 +00:00

@alensiljak also if you change it in one place change in every other to keep it consistent.

@alensiljak also if you change it in one place change in every other to keep it consistent.
alensiljak (Migrated from github.com) reviewed 2022-11-07 10:12:15 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +206,4 @@
if (last == null)
return;
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
alensiljak (Migrated from github.com) commented 2022-11-07 10:12:15 +00:00

Seems that you guys have more time to work on this than I do. :D

Seems that you guys have more time to work on this than I do. :D
pm4rcin commented 2022-11-07 14:31:07 +00:00 (Migrated from github.com)
Copy link

@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);
@alensiljak I have cleaned up the patch ```diff 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); ```
❤️ 2
alensiljak commented 2022-11-07 14:35:02 +00:00 (Migrated from github.com)
Copy link

@alensiljak I have cleaned up the patch

Thank you so much! That saves a ton of time. What are the next steps?

> @alensiljak I have cleaned up the patch Thank you so much! That saves a ton of time. What are the next steps?
alensiljak (Migrated from github.com) reviewed 2022-11-07 14:37:46 +00:00
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +13,4 @@
android:id="@+id/speed_view_container"
android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_weight="1.2"
alensiljak (Migrated from github.com) commented 2022-11-07 14:37:46 +00:00

I understand and agree with your comments. But, unfortunately, I really don't have time at the moment to fiddle around with XML that was modified by the default tool for the job, using default or project settings as advised.

I understand and agree with your comments. But, unfortunately, I really don't have time at the moment to fiddle around with XML that was modified by the default tool for the job, using default or project settings as advised.
RedAuburn reviewed 2022-11-07 14:53:35 +00:00
android/res/layout/layout_nav_bottom_numbers.xml
@ -0,0 +13,4 @@
android:id="@+id/speed_view_container"
android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_weight="1.2"
RedAuburn commented 2022-11-07 14:53:35 +00:00
Member

I started looking into showing speed limit too, try reset the XML, then remove android:paddingStart & android:paddingEndlines from the speed number element

I started looking into showing speed limit too, try reset the XML, then remove `android:paddingStart` & `android:paddingEnd`lines from the speed number element
pm4rcin commented 2022-11-07 14:59:47 +00:00 (Migrated from github.com)
Copy link

You can reset your local branch and save the diff to the file and then git apply filename. Then commit and force push. WARNING: Force pushing will destroy your commit history but I don't think that we need more than 1 commit for such simple PR.

You can reset your local branch and save the diff to the file and then `git apply filename`. Then commit and force push. WARNING: Force pushing will destroy your commit history but I don't think that we need more than 1 commit for such simple PR.
alensiljak commented 2022-11-07 15:28:07 +00:00 (Migrated from github.com)
Copy link

I'm getting

error: patch failed: android/res/layout/layout_nav_bottom_numbers.xml:13
error: android/res/layout/layout_nav_bottom_numbers.xml: patch does not apply
I'm getting ``` error: patch failed: android/res/layout/layout_nav_bottom_numbers.xml:13 error: android/res/layout/layout_nav_bottom_numbers.xml: patch does not apply ```
alensiljak commented 2022-11-07 15:28:44 +00:00 (Migrated from github.com)
Copy link

The PR seems to have closed automatically when the branch was reset.

The PR seems to have closed automatically when the branch was reset.
biodranik commented 2022-11-07 15:37:02 +00:00 (Migrated from github.com)
Copy link

Did you delete your fork's branch? Can you push it again into your fork?

Did you delete your fork's branch? Can you push it again into your fork?
alensiljak commented 2022-11-07 15:39:10 +00:00 (Migrated from github.com)
Copy link

Did you delete your fork's branch? Can you push it again into your fork?

See above.
I reset it before trying to apply the patch but the patch does not apply. Might need to redo the changes from the patch manually.

> Did you delete your fork's branch? Can you push it again into your fork? See above. I reset it before trying to apply the patch but the patch does not apply. Might need to redo the changes from the patch manually.
alensiljak commented 2022-11-07 16:11:49 +00:00 (Migrated from github.com)
Copy link

OK, I've done it manually. The project builds; app tested in the emulator.

OK, I've done it manually. The project builds; app tested in the emulator.
vng (Migrated from github.com) reviewed 2022-11-08 19:43:19 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +250,4 @@
}
return result;
}
vng (Migrated from github.com) commented 2022-11-08 19:43:17 +00:00

Why not to use existing approach? Like:
Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());

Why not to use _existing_ approach? Like: Pair<String, String> speedAndUnits = StringUtils.nativeFormatSpeedAndUnits(last.getSpeed());
alensiljak (Migrated from github.com) reviewed 2022-11-23 10:52:50 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +250,4 @@
}
return result;
}
alensiljak (Migrated from github.com) commented 2022-11-23 10:52:50 +00:00

In this case, the unit is already known to the caller and that would somewhat obfuscate the point of the function, I think.

In this case, the unit is already known to the caller and that would somewhat obfuscate the point of the function, I think.
jordane-quincy (Migrated from github.com) reviewed 2023-01-22 11:25:33 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +225,4 @@
{
double speedLimit = convertMpsIntoUnits(info.speedLimit, speedAndUnits.second);
speedIndicator += String.format("/%.0f", speedLimit);
}
jordane-quincy (Migrated from github.com) commented 2023-01-22 11:25:32 +00:00

[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);
[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](https://git.omaps.dev/organicmaps/organicmaps/pulls/2963/files#diff-689d1838cea59440095256c64d22da973383adbb1143f45a1c545d32621b1de4R160) use space around the slash: ```suggestion speedIndicator += String.format(" / %.0f", speedLimit); ```
alensiljak (Migrated from github.com) reviewed 2023-01-22 12:07:52 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +225,4 @@
{
double speedLimit = convertMpsIntoUnits(info.speedLimit, speedAndUnits.second);
speedIndicator += String.format("/%.0f", speedLimit);
}
alensiljak (Migrated from github.com) commented 2023-01-22 12:07:51 +00:00

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.

> 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.
jordane-quincy (Migrated from github.com) reviewed 2023-01-22 13:15:29 +00:00
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +225,4 @@
{
double speedLimit = convertMpsIntoUnits(info.speedLimit, speedAndUnits.second);
speedIndicator += String.format("/%.0f", speedLimit);
}
jordane-quincy (Migrated from github.com) commented 2023-01-22 13:15:29 +00:00

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

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
alensiljak (Migrated from github.com) reviewed 2023-01-22 13:34:56 +00:00
android/res/layout/layout_nav_bottom_numbers.xml
alensiljak (Migrated from github.com) commented 2023-01-22 13:34:56 +00:00

I believe this is resolved by the second attempt, applying only the changes without reformatting.

I believe this is resolved by the second attempt, applying only the changes without reformatting.
biodranik (Migrated from github.com) reviewed 2023-01-27 22:26:54 +00:00
biodranik (Migrated from github.com) left a comment

What does it look like in a vertical orientation on different devices, including lower-end ones (e.g. Android 5-6), with speed 110 and speed limit 120, and with a larger/largest font selected in the device's options?

What does it look like in a vertical orientation on different devices, including lower-end ones (e.g. Android 5-6), with speed 110 and speed limit 120, and with a larger/largest font selected in the device's options?
android/src/app/organicmaps/widget/menu/NavMenu.java
@ -0,0 +250,4 @@
}
return result;
}
biodranik (Migrated from github.com) commented 2023-01-27 22:25:09 +00:00

Is this duplication really necessary?

Is this duplication really necessary?
biodranik (Migrated from github.com) requested changes 2023-03-06 23:38:25 +00:00
biodranik (Migrated from github.com) left a comment

organicmaps/organicmaps#4189 is a blocker for this PR and should be fixed first.

https://git.omaps.dev/organicmaps/organicmaps/issues/4189 is a blocker for this PR and should be fixed first.
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
2 participants
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#3817
No description provided.
Delete branch "speed-limit-android"

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?