fix: position not updated when fix is lost (#5565)

* fix: position not updated when fix is lost Signed-off-by: SimoneFiorani <[email protected]> * refactor: refactored conditions to parse data Signed-off-by: SimoneFiorani <[email protected]> * fix: small issues and test coverage Signed-off-by: SimoneFiorani <[email protected]> * fix: reduced method complexity Signed-off-by: SimoneFiorani <[email protected]> --------- Signed-off-by: SimoneFiorani <[email protected]>
eclipse-kura · Nov 15, 2024 · c685fdc · c685fdc
1 parent 9b894d6
commit c685fdc
Show file tree

Hide file tree

Showing 4 changed files with 198 additions and 58 deletions.
diff --git a/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/position/MMLocationParser.java b/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/position/MMLocationParser.java
@@ -16,6 +16,7 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -66,7 +67,11 @@ public class MMLocationParser {
     private Boolean isFix = false;
 
     public LocalDateTime getLocalDateTime() {
-        return Objects.requireNonNull(LocalDateTime.of(date, time));
+        if (!Objects.isNull(this.time) && !Objects.isNull(this.date)) {
+            return LocalDateTime.of(date, time);
+        }
+
+        return null;
     }
 
     /*
@@ -102,6 +107,25 @@ public String getNmeaDate() {
         return this.date.toString();
     }
 
+    /*
+     * Used by MMPositionProvider to invalidate position in case ModemManager doesn't provide any data, neither the nmea
+     * nor the raw. Usually, the Nmea data is always provided, while the Raw is provided only if Nmea has fix.
+     */
+
+    public void setInvalidFix() {
+        this.isFix = false;
+
+        this.latitudeDegrees = 0.0;
+        this.longitudeDegrees = 0.0;
+        this.altitudeMeters = 0.0;
+        this.speedMetersPerSecond = 0.0;
+        this.trackDegrees = 0.0;
+
+        LocalDateTime utcDateTime = LocalDateTime.now(ZoneOffset.UTC);
+        this.time = utcDateTime.toLocalTime();
+        this.date = utcDateTime.toLocalDate();
+    }
+
     public void parseRawLocation(Variant<?> rawLocationVariant) {
         Map<String, Variant<?>> locationData = (Map<String, Variant<?>>) rawLocationVariant.getValue();
         for (Map.Entry<String, Variant<?>> rawEntry : locationData.entrySet()) {
@@ -176,67 +200,83 @@ public void parseNmeaLocation(Variant<?> nmeaLocationVariant) {
     }
 
     private void parseGgaSentence(List<String> gsaTokens) {
-        if (!gsaTokens.get(7).isEmpty()) {
-            this.nrSatellites = Integer.parseInt(gsaTokens.get(7));
-        }
-        if (!gsaTokens.get(8).isEmpty()) {
-            this.mDOP = Double.parseDouble(gsaTokens.get(8));
+        if (gsaTokens.size() > 9) {
+            if (!gsaTokens.get(7).isEmpty()) {
+                this.nrSatellites = Integer.parseInt(gsaTokens.get(7));
+            }
+            if (!gsaTokens.get(8).isEmpty()) {
+                this.mDOP = Double.parseDouble(gsaTokens.get(8));
+            }
+        } else {
+            setInvalidFix();
         }
     }
 
     private void parseGsaSentence(List<String> gsaTokens) {
 
-        if (!gsaTokens.get(2).isEmpty()) {
+        if (gsaTokens.size() > 5) {
+            if (!gsaTokens.get(2).isEmpty()) {
 
-            int fixType = Integer.parseInt(gsaTokens.get(2));
-            this.fixQuality = fixType;
-            if (fixType == 2 || fixType == 3) {
-                this.isFix = true;
-            } else {
-                this.isFix = false;
+                int fixType = Integer.parseInt(gsaTokens.get(2));
+                this.fixQuality = fixType;
+                if (fixType == 2 || fixType == 3) {
+                    this.isFix = true;
+                } else {
+                    this.isFix = false;
+                }
             }
-        }
 
-        if (!gsaTokens.get(15).isEmpty()) {
-            this.mPDOP = Double.parseDouble(gsaTokens.get(15));
-        }
-        if (!gsaTokens.get(16).isEmpty()) {
-            this.mHDOP = Double.parseDouble(gsaTokens.get(16));
-        }
-        if (!gsaTokens.get(17).isEmpty()) {
-            this.mVDOP = Double.parseDouble(gsaTokens.get(17));
+            if (!gsaTokens.get(15).isEmpty()) {
+                this.mPDOP = Double.parseDouble(gsaTokens.get(15));
+            }
+            if (!gsaTokens.get(16).isEmpty()) {
+                this.mHDOP = Double.parseDouble(gsaTokens.get(16));
+            }
+            if (!gsaTokens.get(17).isEmpty()) {
+                this.mVDOP = Double.parseDouble(gsaTokens.get(17));
+            }
+        } else {
+            setInvalidFix();
         }
     }
 
     /*
      * Date is received in format dd-M-yy, so we convert it to yy-M-dd
      */
     private void parseRmcSentence(List<String> rmcTokens) {
-        if (!rmcTokens.get(9).isEmpty()) {
-            this.date = LocalDate.parse(rmcTokens.get(9), DateTimeFormatter.ofPattern("ddMyy"));
-        }
-        if (!rmcTokens.get(7).isEmpty()) {
-            this.speedMetersPerSecond = Double.parseDouble(rmcTokens.get(7)) * KNOTS_TO_MS;
-        }
-        if (!rmcTokens.get(8).isEmpty()) {
-            this.trackDegrees = Double.parseDouble(rmcTokens.get(8));
-        }
-        if (!rmcTokens.get(4).isEmpty()) {
-            this.latitudeHemisphere = rmcTokens.get(4).charAt(0);
-        }
-        if (!rmcTokens.get(6).isEmpty()) {
-            this.longitudeHemisphere = rmcTokens.get(6).charAt(0);
-        }
-        if (!rmcTokens.get(2).isEmpty()) { // check validity
-            this.validFix = rmcTokens.get(2).charAt(0);
-            if (!"A".equals(rmcTokens.get(2))) {
-                this.isFix = false;
+        if (rmcTokens.size() > 9) {
+            if (!rmcTokens.get(9).isEmpty()) {
+                this.date = LocalDate.parse(rmcTokens.get(9), DateTimeFormatter.ofPattern("ddMyy"));
+            }
+            if (!rmcTokens.get(7).isEmpty()) {
+                this.speedMetersPerSecond = Double.parseDouble(rmcTokens.get(7)) * KNOTS_TO_MS;
+            }
+            if (!rmcTokens.get(8).isEmpty()) {
+                this.trackDegrees = Double.parseDouble(rmcTokens.get(8));
+            }
+            if (!rmcTokens.get(4).isEmpty()) {
+                this.latitudeHemisphere = rmcTokens.get(4).charAt(0);
+            }
+            if (!rmcTokens.get(6).isEmpty()) {
+                this.longitudeHemisphere = rmcTokens.get(6).charAt(0);
+            }
+            if (!rmcTokens.get(2).isEmpty()) { // check validity
+                parseRmcFixValidity(rmcTokens.get(2));
             } else {
-                this.isFix = true;
+                this.validFix = 'V';
+                this.isFix = false;
             }
         } else {
-            this.validFix = 'V';
+            setInvalidFix();
+        }
+    }
+
+    private void parseRmcFixValidity(String validityToken) {
+        this.validFix = validityToken.charAt(0);
+        if (!"A".equals(validityToken)) {
             this.isFix = false;
+        } else {
+            this.isFix = true;
         }
     }
 
@@ -293,8 +333,8 @@ private GNSSType sentenceIdToGnssType(String type) {
 
     @Override
     public String toString() {
-        return "ModemManagerProvider [latitude=" + latitudeDegrees + ", longitude=" + longitudeDegrees + ", altitude="
-                + altitudeMeters + ", speed=" + speedMetersPerSecond + ", timestamp=" + time + ", date=" + date
-                + ", gnssType=" + gnssTypes + "]";
+        return "ModemManagerProvider [isFix=" + isFix + ", latitude=" + latitudeDegrees + ", longitude="
+                + longitudeDegrees + ", altitude=" + altitudeMeters + ", speed=" + speedMetersPerSecond + ", timestamp="
+                + time + ", date=" + date + ", gnssType=" + gnssTypes + "]";
     }
 }
diff --git a/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/position/MMPositionProvider.java b/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/position/MMPositionProvider.java
@@ -16,6 +16,7 @@
 import java.time.LocalDateTime;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
@@ -151,12 +152,20 @@ private void getModemManagerLocation() {
 
         for (Location location : availableLocations) {
             Map<UInt32, Variant<?>> locationMap = location.GetLocation();
-            if (locationMap.containsKey(NMEA_LOCATION_SOURCE) && locationMap.containsKey(RAW_LOCATION_SOURCE)) {
-                Variant<?> nmeaData = locationMap.get(NMEA_LOCATION_SOURCE);
-                this.mmLocationParser.parseNmeaLocation(nmeaData);
+            Optional<Variant<?>> nmeaData = locationMap.containsKey(NMEA_LOCATION_SOURCE)
+                    ? Optional.of(locationMap.get(NMEA_LOCATION_SOURCE))
+                    : Optional.empty();
 
-                Variant<?> rawData = locationMap.get(RAW_LOCATION_SOURCE);
-                this.mmLocationParser.parseRawLocation(rawData);
+            Optional<Variant<?>> rawData = locationMap.containsKey(RAW_LOCATION_SOURCE)
+                    ? Optional.of(locationMap.get(RAW_LOCATION_SOURCE))
+                    : Optional.empty();
+
+            if (!nmeaData.isPresent() && !rawData.isPresent()) {
+                this.mmLocationParser.setInvalidFix();
+            } else if (nmeaData.isPresent()) {
+                this.mmLocationParser.parseNmeaLocation(nmeaData.get());
+
+                rawData.ifPresent(this.mmLocationParser::parseRawLocation);
             }
         }
 

diff --git a/...lipse.kura.nm.test/src/test/java/org/eclipse/kura/nm/position/MMPositionProviderTest.java b/...lipse.kura.nm.test/src/test/java/org/eclipse/kura/nm/position/MMPositionProviderTest.java
@@ -14,6 +14,7 @@
 package org.eclipse.kura.nm.position;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -42,20 +43,49 @@ public class MMPositionProviderTest {
     NMDbusConnector mockNmDbusConnector;
     Position retrievedPosition;
     LocalDateTime retrieveDateTime;
+    Boolean isFix;
+
+    Exception unexpectedException;
 
     @Test
     public void shouldRetrieveCorrectPosition() throws InterruptedException {
 
         givenModemManagerFakeLocation(39.5, 9.7, 4.5);
         givenPositionProviderWithMockDbusConnector();
-        givenProviderInitAndStart();
+        givenProviderInitAndStartWithRefreshRate(1);
 
-        whenServiceAskForPosition();
+        whenServiceAsksForPosition();
 
+        thenNoExceptionThrown();
+        thenPositionIsFixed(true);
         thenPositionIsCorrect(39.5, 9.7, 4.5, 0, 10.2);
         thenDateTimeIsCorrect("2024-10-29T10:33:55");
     }
 
+    @Test
+    public void shouldNotReportPositionWithInvalidRefreshRate() throws InterruptedException {
+        givenModemManagerFakeLocation(39.5, 9.7, 4.5);
+        givenPositionProviderWithMockDbusConnector();
+        givenProviderInitAndStartWithRefreshRate(-1);
+
+        whenServiceAsksForFix();
+
+        thenNoExceptionThrown();
+        thenPositionIsFixed(false);
+    }
+
+    @Test
+    public void shouldNotReportPositionWithoutNmeaData() throws InterruptedException {
+        givenModemManagerWithNoFix();
+        givenPositionProviderWithMockDbusConnector();
+        givenProviderInitAndStartWithRefreshRate(1);
+
+        whenServiceAsksForFix();
+
+        thenNoExceptionThrown();
+        thenPositionIsFixed(false);
+    }
+
     /*
      * The return location is a gps point in Rome retrieved with {@link https://nmeagen.org/}
      */
@@ -88,23 +118,65 @@ private void givenModemManagerFakeLocation(double lat, double lon, double alt) {
         this.mockNmDbusConnector = dbusConnector;
     }
 
+    private void givenModemManagerWithNoFix() {
+
+        Scanner scanner = new Scanner(MMPositionProviderTest.class.getResourceAsStream("/noFixNmeaSentences.txt"),
+                "UTF-8");
+        CharSequence locationString = scanner.useDelimiter("\\A").next().replace(" ", "");
+        scanner.close();
+
+        Location mockLocation = mock(Location.class);
+        Map<UInt32, Variant<?>> variantMap = new HashMap<>();
+        variantMap.put(new UInt32(MMModemLocationSource.MM_MODEM_LOCATION_SOURCE_GPS_NMEA.getValue()),
+                new Variant<>(locationString));
+
+        when(mockLocation.GetLocation()).thenReturn(variantMap);
+
+        NMDbusConnector dbusConnector = mock(NMDbusConnector.class);
+        when(dbusConnector.getAvailableMMLocations()).thenReturn(Arrays.asList(mockLocation));
+
+        this.mockNmDbusConnector = dbusConnector;
+    }
+
     private void givenPositionProviderWithMockDbusConnector() {
         this.provider = new MMPositionProvider(this.mockNmDbusConnector);
     }
 
-    private void givenProviderInitAndStart() {
+    private void givenProviderInitAndStartWithRefreshRate(int refreshRate) {
         Map<String, Object> properties = new HashMap<>();
-        properties.put("modem.manager.refresh.rate.seconds", 1);
+        properties.put("modem.manager.refresh.rate.seconds", refreshRate);
         PositionServiceOptions options = new PositionServiceOptions(properties);
 
         this.provider.init(options, mock(LockStatusListener.class), mock(GpsDeviceAvailabilityListener.class));
         this.provider.start();
     }
 
-    private void whenServiceAskForPosition() throws InterruptedException {
+    private void whenServiceAsksForPosition() throws InterruptedException {
         Thread.sleep(3000);
-        this.retrievedPosition = this.provider.getPosition();
-        this.retrieveDateTime = this.provider.getDateTime();
+
+        try {
+            this.isFix = this.provider.isLocked();
+            this.retrievedPosition = this.provider.getPosition();
+            this.retrieveDateTime = this.provider.getDateTime();
+        } catch (Exception ex) {
+            this.unexpectedException = ex;
+        }
+
+    }
+
+    private void whenServiceAsksForFix() throws InterruptedException {
+        Thread.sleep(3000);
+
+        try {
+            this.isFix = this.provider.isLocked();
+        } catch (Exception ex) {
+            this.unexpectedException = ex;
+        }
+
+    }
+
+    private void thenNoExceptionThrown() {
+        assertNull(this.unexpectedException);
     }
 
     private void thenPositionIsCorrect(double lat, double lon, double alt, double speed, double track) {
@@ -119,4 +191,8 @@ private void thenDateTimeIsCorrect(String expectedDateTime) {
         assertEquals(expectedDateTime, this.retrieveDateTime.toString());
     }
 
+    private void thenPositionIsFixed(boolean fixValue) {
+        assertEquals(fixValue, this.isFix);
+    }
+
 }
diff --git a/kura/test/org.eclipse.kura.nm.test/src/test/resources/noFixNmeaSentences.txt b/kura/test/org.eclipse.kura.nm.test/src/test/resources/noFixNmeaSentences.txt
@@ -0,0 +1,15 @@
+$GPGSA,A,1,,,,,,,,,,,,,,,,*32
+
+$GPRMC,,V,,,,,,,,,,N,V*29
+
+$GPGSV,4,1,13,05,49,296,25,07,47,056,40,09,17,099,12,11,13,226,22,1*6A
+
+$GPGSV,4,2,13,13,33,285,40,14,29,157,29,15,01,291,26,20,59,233,23,1*69
+
+$GPGSV,4,3,13,30,83,075,41,08,02,064,,18,02,336,,22,12,171,,1*6F
+
+$GPGSV,4,4,13,27,00,035,,1*55
+
+$GPVTG,,T,,M,,N,,K,N*2C
+
+$GPGGA,,,,,,0,,,,,,,,*66