[Essentials] Use mean sea level altitude on Android API 34+#35097
[Essentials] Use mean sea level altitude on Android API 34+#35097KitKeen wants to merge 5 commits intodotnet:mainfrom
Conversation
Android API level 34 introduced Location.getMslAltitudeMeters(), which provides altitude measured against the geoid (mean sea level). The existing implementation always reads the WGS84 ellipsoidal altitude, which is reported inconsistently with iOS/Windows where altitude is typically expressed against the geoid. Prefer the MSL altitude when the device exposes it (API 34+ and HasMslAltitude is true) and report AltitudeReferenceSystem.Geoid in that case. Fall back to the existing WGS84 path on older devices. Also reports AltitudeReferenceSystem.Unspecified when no altitude is available, instead of hard-coding Ellipsoid for a null altitude. Fixes dotnet#27554
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35097Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35097" |
🟠 .NET MAUI Code Review — Discussion Needed
🟠 Review Session — Discussion Needed —
|
Address review feedback on dotnet#35097. Previously VerticalAccuracy was always read from Location.VerticalAccuracyMeters (ellipsoidal accuracy), regardless of whether Altitude came from the MSL/geoid or WGS84/ellipsoid path. On Android API 34+ with MSL altitude, this produced an altitude and a vertical accuracy in different reference systems. Select altitude and its accuracy together in GetAltitude so they can never diverge: - MSL path (API 34+ && HasMslAltitude): altitude from MslAltitudeMeters, accuracy from MslAltitudeAccuracyMeters when HasMslAltitudeAccuracy is true, else null. - Ellipsoid path (HasAltitude): altitude from Altitude, accuracy from VerticalAccuracyMeters when API 26+ and HasVerticalAccuracy. - No altitude: (null, Unspecified, null). Add device tests covering all three GetAltitude branches and the MSL-without-MSL-accuracy case.
The existing TestLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem test happened to exercise the fallback because MslAltitudeMeters was never set, but that was implicit. This adds a dedicated test that pins the invariant: on API 34+ where HasMslAltitude is false (e.g. the device did not report a reference-corrected altitude), ToLocation falls back to the ellipsoidal altitude and tags it Ellipsoid — not Geoid, not Unspecified. This is the second half of the IsAndroidVersionAtLeast(34) && HasMslAltitude guard added in the previous commit; without an explicit test, a refactor that dropped the HasMslAltitude check would still pass the suite.
Each test now runs at least one assertion on any API level before the API 34+ specific branch, so pre-34 runs exercise the fallback path (Ellipsoid or Unspecified) instead of returning early without asserting. This keeps the gate test meaningful on devices below API 34 and catches regressions in either code path.
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #4 automatically generated candidates and selected try-fix-4 as the strongest fix.
Why: try-fix-4 wins because it combines the PR's correct GetAltitude helper with two critical additions the PR omits: (1) a one-line fix in the Location(Location point) copy constructor that was missing AltitudeReferenceSystem = point.AltitudeReferenceSystem — without this, Geocoding.GetPlacemarksAsync silently resets the reference system on Android 34+; (2) a regression test (LocationCopyConstructor_CopiesAltitudeReferenceSystem) that protects against that bug recurring. It also removes the byte-for-byte duplicate test.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-4`)
diff --git a/src/Essentials/src/Types/Location.shared.cs b/src/Essentials/src/Types/Location.shared.cs
index 892cf582d4..cf5bb4caec 100644
--- a/src/Essentials/src/Types/Location.shared.cs
+++ b/src/Essentials/src/Types/Location.shared.cs
@@ -109,6 +109,7 @@ namespace Microsoft.Maui.Devices.Sensors
Speed = point.Speed;
Course = point.Course;
IsFromMockProvider = point.IsFromMockProvider;
+ AltitudeReferenceSystem = point.AltitudeReferenceSystem;
}
/// <summary>
diff --git a/src/Essentials/src/Types/LocationExtensions.android.cs b/src/Essentials/src/Types/LocationExtensions.android.cs
index 7c3d664b94..ceb72cc9a9 100644
--- a/src/Essentials/src/Types/LocationExtensions.android.cs
+++ b/src/Essentials/src/Types/LocationExtensions.android.cs
@@ -19,18 +19,18 @@ namespace Microsoft.Maui.Devices.Sensors
internal static IEnumerable<Location> ToLocations(this IEnumerable<AndroidAddress> addresses) =>
addresses?.Select(a => a.ToLocation());
- internal static Location ToLocation(this AndroidLocation location) =>
- new Location
+ internal static Location ToLocation(this AndroidLocation location)
+ {
+ var (altitude, altitudeReference, verticalAccuracy) = GetAltitude(location);
+
+ return new Location
{
Latitude = location.Latitude,
Longitude = location.Longitude,
- Altitude = location.HasAltitude ? location.Altitude : default(double?),
+ Altitude = altitude,
Timestamp = location.GetTimestamp().ToUniversalTime(),
Accuracy = location.HasAccuracy ? location.Accuracy : default(float?),
- VerticalAccuracy =
- OperatingSystem.IsAndroidVersionAtLeast(26) && location.HasVerticalAccuracy
- ? location.VerticalAccuracyMeters
- : null,
+ VerticalAccuracy = verticalAccuracy,
ReducedAccuracy = false,
Course = location.HasBearing ? location.Bearing : default(double?),
Speed = location.HasSpeed ? location.Speed : default(double?),
@@ -40,8 +40,35 @@ namespace Microsoft.Maui.Devices.Sensors
#pragma warning disable CS0618 // Type or member is obsolete
: location.IsFromMockProvider,
#pragma warning restore CS0618 // Type or member is obsolete
- AltitudeReferenceSystem = AltitudeReferenceSystem.Ellipsoid
+ AltitudeReferenceSystem = altitudeReference
};
+ }
+
+ // Prefer mean sea level altitude (Android API 34+) when available so altitude
+ // values are consistent across platforms without manual geoid correction.
+ // Altitude and its accuracy are selected together so they always come from
+ // the same reference system (MSL/geoid vs WGS84/ellipsoid).
+ static (double? Altitude, AltitudeReferenceSystem Reference, double? VerticalAccuracy) GetAltitude(AndroidLocation location)
+ {
+ if (OperatingSystem.IsAndroidVersionAtLeast(34) && location.HasMslAltitude)
+ {
+ double? mslAccuracy = location.HasMslAltitudeAccuracy
+ ? location.MslAltitudeAccuracyMeters
+ : null;
+ return (location.MslAltitudeMeters, AltitudeReferenceSystem.Geoid, mslAccuracy);
+ }
+
+ if (location.HasAltitude)
+ {
+ double? ellipsoidAccuracy =
+ OperatingSystem.IsAndroidVersionAtLeast(26) && location.HasVerticalAccuracy
+ ? location.VerticalAccuracyMeters
+ : null;
+ return (location.Altitude, AltitudeReferenceSystem.Ellipsoid, ellipsoidAccuracy);
+ }
+
+ return (null, AltitudeReferenceSystem.Unspecified, null);
+ }
static readonly DateTime epoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
diff --git a/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs b/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs
index 9742e83aad..f07e37b7e8 100644
--- a/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs
+++ b/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs
@@ -42,33 +42,6 @@ namespace Microsoft.Maui.Essentials.DeviceTests.Shared
Assert.Null(location.VerticalAccuracy);
}
- [Fact]
- public void ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid()
- {
- // On every API level, a location that reports an ellipsoidal altitude but no
- // MSL altitude must resolve to Ellipsoid. On pre-34 devices that is the only
- // code path; on API 34+ devices this exercises the HasMslAltitude == false
- // fallback branch. Either way this assertion runs, so the test cannot silently
- // pass if the fallback regresses.
- var androidLocation = new AndroidLocation("test")
- {
- Altitude = 123.45,
- };
-
- if (OperatingSystem.IsAndroidVersionAtLeast(26))
- androidLocation.VerticalAccuracyMeters = 5.0f;
-
- var location = androidLocation.ToLocation();
-
- Assert.Equal(123.45, location.Altitude);
- Assert.Equal(AltitudeReferenceSystem.Ellipsoid, location.AltitudeReferenceSystem);
-
- if (OperatingSystem.IsAndroidVersionAtLeast(26))
- Assert.Equal(5.0, location.VerticalAccuracy);
- else
- Assert.Null(location.VerticalAccuracy);
- }
-
[Fact]
public void ToLocation_MslAltitude_UsesGeoidReferenceSystem()
{
@@ -130,5 +103,30 @@ namespace Microsoft.Maui.Essentials.DeviceTests.Shared
Assert.Equal(AltitudeReferenceSystem.Geoid, location.AltitudeReferenceSystem);
Assert.Null(location.VerticalAccuracy);
}
+
+ [Fact]
+ public void LocationCopyConstructor_CopiesAltitudeReferenceSystem()
+ {
+ var originalLocation = new Location
+ {
+ Latitude = 37.7749,
+ Longitude = -122.4194,
+ Altitude = 100.0,
+ AltitudeReferenceSystem = AltitudeReferenceSystem.Geoid,
+ VerticalAccuracy = 2.5
+ };
+
+ var copiedLocation = new Location(originalLocation);
+
+ Assert.Equal(originalLocation.Latitude, copiedLocation.Latitude);
+ Assert.Equal(originalLocation.Longitude, copiedLocation.Longitude);
+ Assert.Equal(originalLocation.Altitude, copiedLocation.Altitude);
+ Assert.Equal(AltitudeReferenceSystem.Geoid, copiedLocation.AltitudeReferenceSystem);
+ Assert.Equal(originalLocation.VerticalAccuracy, copiedLocation.VerticalAccuracy);
+ }
}
}
kubaflo
left a comment
There was a problem hiding this comment.
Could you please try the ai's suggestions?
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #1 automatically generated candidates and selected try-fix-1 as the strongest fix.
Why: try-fix-1 (inline set-defaults-then-override) is functionally equivalent to the PR's GetAltitude() helper but inlined into ToLocation() with plain local variables. It is the simplest approach, passes all 5 device tests, and avoids any helper-method linker sensitivity that may have contributed to the gate failure on the PR's version.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-1`)
diff --git a/src/Essentials/src/Types/LocationExtensions.android.cs b/src/Essentials/src/Types/LocationExtensions.android.cs
index 7c3d664b94..370bf854c6 100644
--- a/src/Essentials/src/Types/LocationExtensions.android.cs
+++ b/src/Essentials/src/Types/LocationExtensions.android.cs
@@ -19,18 +19,40 @@ namespace Microsoft.Maui.Devices.Sensors
internal static IEnumerable<Location> ToLocations(this IEnumerable<AndroidAddress> addresses) =>
addresses?.Select(a => a.ToLocation());
- internal static Location ToLocation(this AndroidLocation location) =>
- new Location
+ internal static Location ToLocation(this AndroidLocation location)
+ {
+ // Default to no altitude with Unspecified reference; override below
+ // when the platform reports a usable value.
+ double? altitude = null;
+ AltitudeReferenceSystem altitudeRef = AltitudeReferenceSystem.Unspecified;
+ double? verticalAccuracy = null;
+
+ // Prefer mean-sea-level altitude (API 34+) for cross-platform consistency.
+ if (OperatingSystem.IsAndroidVersionAtLeast(34) && location.HasMslAltitude)
+ {
+ altitude = location.MslAltitudeMeters;
+ altitudeRef = AltitudeReferenceSystem.Geoid;
+ verticalAccuracy = location.HasMslAltitudeAccuracy
+ ? location.MslAltitudeAccuracyMeters
+ : null;
+ }
+ else if (location.HasAltitude)
+ {
+ altitude = location.Altitude;
+ altitudeRef = AltitudeReferenceSystem.Ellipsoid;
+ verticalAccuracy = OperatingSystem.IsAndroidVersionAtLeast(26) && location.HasVerticalAccuracy
+ ? location.VerticalAccuracyMeters
+ : null;
+ }
+
+ return new Location
{
Latitude = location.Latitude,
Longitude = location.Longitude,
- Altitude = location.HasAltitude ? location.Altitude : default(double?),
+ Altitude = altitude,
Timestamp = location.GetTimestamp().ToUniversalTime(),
Accuracy = location.HasAccuracy ? location.Accuracy : default(float?),
- VerticalAccuracy =
- OperatingSystem.IsAndroidVersionAtLeast(26) && location.HasVerticalAccuracy
- ? location.VerticalAccuracyMeters
- : null,
+ VerticalAccuracy = verticalAccuracy,
ReducedAccuracy = false,
Course = location.HasBearing ? location.Bearing : default(double?),
Speed = location.HasSpeed ? location.Speed : default(double?),
@@ -40,8 +62,9 @@ namespace Microsoft.Maui.Devices.Sensors
#pragma warning disable CS0618 // Type or member is obsolete
: location.IsFromMockProvider,
#pragma warning restore CS0618 // Type or member is obsolete
- AltitudeReferenceSystem = AltitudeReferenceSystem.Ellipsoid
+ AltitudeReferenceSystem = altitudeRef
};
+ }
static readonly DateTime epoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
Add missing AltitudeReferenceSystem assignment in the Location copy constructor and add a unit test to cover the regression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed the copy constructor issue- added AltitudeReferenceSystem to the copy constructor and added a unit test to cover the regression. |
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #4 automatically generated candidates and selected try-fix-4 as the strongest fix.
Why: try-fix-4 passes all 272 device tests and uses the same GetAltitude tuple-helper pattern as the PR. It replaces the byte-for-byte duplicate test with an explicit MSL-precedence regression test that directly validates the API 34+ behaviour when both altitude systems are present. The PR gate failure is an XHarness infrastructure issue (exit code 82), not a code correctness failure.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-4`)
diff --git a/src/Essentials/src/Types/LocationExtensions.android.cs b/src/Essentials/src/Types/LocationExtensions.android.cs
index 7c3d664b94..d342f057cb 100644
--- a/src/Essentials/src/Types/LocationExtensions.android.cs
+++ b/src/Essentials/src/Types/LocationExtensions.android.cs
@@ -19,18 +19,18 @@ namespace Microsoft.Maui.Devices.Sensors
internal static IEnumerable<Location> ToLocations(this IEnumerable<AndroidAddress> addresses) =>
addresses?.Select(a => a.ToLocation());
- internal static Location ToLocation(this AndroidLocation location) =>
- new Location
+ internal static Location ToLocation(this AndroidLocation location)
+ {
+ var (altitude, altitudeReference, verticalAccuracy) = GetAltitude(location);
+
+ return new Location
{
Latitude = location.Latitude,
Longitude = location.Longitude,
- Altitude = location.HasAltitude ? location.Altitude : default(double?),
+ Altitude = altitude,
Timestamp = location.GetTimestamp().ToUniversalTime(),
Accuracy = location.HasAccuracy ? location.Accuracy : default(float?),
- VerticalAccuracy =
- OperatingSystem.IsAndroidVersionAtLeast(26) && location.HasVerticalAccuracy
- ? location.VerticalAccuracyMeters
- : null,
+ VerticalAccuracy = verticalAccuracy,
ReducedAccuracy = false,
Course = location.HasBearing ? location.Bearing : default(double?),
Speed = location.HasSpeed ? location.Speed : default(double?),
@@ -40,8 +40,34 @@ namespace Microsoft.Maui.Devices.Sensors
#pragma warning disable CS0618 // Type or member is obsolete
: location.IsFromMockProvider,
#pragma warning restore CS0618 // Type or member is obsolete
- AltitudeReferenceSystem = AltitudeReferenceSystem.Ellipsoid
+ AltitudeReferenceSystem = altitudeReference
};
+ }
+
+ // Prefer mean sea level altitude (Android API 34+) when available so Android
+ // matches the geoid-based altitude reported by iOS and Windows. Altitude and
+ // accuracy are selected together so they always come from the same reference system.
+ static (double? Altitude, AltitudeReferenceSystem Reference, double? VerticalAccuracy) GetAltitude(AndroidLocation location)
+ {
+ if (OperatingSystem.IsAndroidVersionAtLeast(34) && location.HasMslAltitude)
+ {
+ double? mslAccuracy = location.HasMslAltitudeAccuracy
+ ? location.MslAltitudeAccuracyMeters
+ : null;
+ return (location.MslAltitudeMeters, AltitudeReferenceSystem.Geoid, mslAccuracy);
+ }
+
+ if (location.HasAltitude)
+ {
+ double? ellipsoidAccuracy =
+ OperatingSystem.IsAndroidVersionAtLeast(26) && location.HasVerticalAccuracy
+ ? location.VerticalAccuracyMeters
+ : null;
+ return (location.Altitude, AltitudeReferenceSystem.Ellipsoid, ellipsoidAccuracy);
+ }
+
+ return (null, AltitudeReferenceSystem.Unspecified, null);
+ }
static readonly DateTime epoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
diff --git a/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs b/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs
index 9742e83aad..2042d07b1c 100644
--- a/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs
+++ b/src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs
@@ -43,13 +43,8 @@ namespace Microsoft.Maui.Essentials.DeviceTests.Shared
}
[Fact]
- public void ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid()
+ public void ToLocation_MslAltitudeTakesPrecedenceOverEllipsoidalAltitude()
{
- // On every API level, a location that reports an ellipsoidal altitude but no
- // MSL altitude must resolve to Ellipsoid. On pre-34 devices that is the only
- // code path; on API 34+ devices this exercises the HasMslAltitude == false
- // fallback branch. Either way this assertion runs, so the test cannot silently
- // pass if the fallback regresses.
var androidLocation = new AndroidLocation("test")
{
Altitude = 123.45,
@@ -58,49 +53,59 @@ namespace Microsoft.Maui.Essentials.DeviceTests.Shared
if (OperatingSystem.IsAndroidVersionAtLeast(26))
androidLocation.VerticalAccuracyMeters = 5.0f;
- var location = androidLocation.ToLocation();
-
- Assert.Equal(123.45, location.Altitude);
- Assert.Equal(AltitudeReferenceSystem.Ellipsoid, location.AltitudeReferenceSystem);
-
+ var baseline = androidLocation.ToLocation();
+ Assert.Equal(123.45, baseline.Altitude);
+ Assert.Equal(AltitudeReferenceSystem.Ellipsoid, baseline.AltitudeReferenceSystem);
if (OperatingSystem.IsAndroidVersionAtLeast(26))
- Assert.Equal(5.0, location.VerticalAccuracy);
+ Assert.Equal(5.0, baseline.VerticalAccuracy);
else
- Assert.Null(location.VerticalAccuracy);
+ Assert.Null(baseline.VerticalAccuracy);
+
+ if (!OperatingSystem.IsAndroidVersionAtLeast(34))
+ return;
+
+ // When both altitude systems are present, API 34+ must prefer MSL/geoid so
+ // Android matches iOS and Windows without manual geoid correction.
+ androidLocation.MslAltitudeMeters = 100.0;
+ androidLocation.MslAltitudeAccuracyMeters = 2.5f;
+
+ var location = androidLocation.ToLocation();
+
+ Assert.Equal(100.0, location.Altitude);
+ Assert.Equal(AltitudeReferenceSystem.Geoid, location.AltitudeReferenceSystem);
+ Assert.Equal(2.5, location.VerticalAccuracy);
}
[Fact]
public void ToLocation_MslAltitude_UsesGeoidReferenceSystem()
{
- var androidLocation = new AndroidLocation("test")
+ var baselineLocation = new AndroidLocation("test")
{
Altitude = 123.45,
};
- // Baseline: without MSL altitude set, we must get Ellipsoid on any API level.
- // This guarantees an assertion runs even on pre-34 devices so the test cannot
- // silently pass if the API-34 branch is accidentally taken or the fallback
- // regresses.
- var baseline = androidLocation.ToLocation();
+ if (OperatingSystem.IsAndroidVersionAtLeast(26))
+ baselineLocation.VerticalAccuracyMeters = 5.0f;
+
+ var baseline = baselineLocation.ToLocation();
Assert.Equal(123.45, baseline.Altitude);
Assert.Equal(AltitudeReferenceSystem.Ellipsoid, baseline.AltitudeReferenceSystem);
+ if (OperatingSystem.IsAndroidVersionAtLeast(26))
+ Assert.Equal(5.0, baseline.VerticalAccuracy);
+ else
+ Assert.Null(baseline.VerticalAccuracy);
- // The remaining assertions exercise the API 34+ MSL path. MslAltitudeMeters is
- // only meaningful on API 34+, so below that we stop after validating the
- // fallback above.
if (!OperatingSystem.IsAndroidVersionAtLeast(34))
return;
+ var androidLocation = new AndroidLocation("test");
androidLocation.MslAltitudeMeters = 100.0;
androidLocation.MslAltitudeAccuracyMeters = 2.5f;
- androidLocation.VerticalAccuracyMeters = 5.0f; // ellipsoidal accuracy, must NOT be surfaced
var location = androidLocation.ToLocation();
Assert.Equal(100.0, location.Altitude);
Assert.Equal(AltitudeReferenceSystem.Geoid, location.AltitudeReferenceSystem);
- // VerticalAccuracy must be paired with the chosen altitude reference system,
- // so the MSL accuracy is used rather than the ellipsoidal one.
Assert.Equal(2.5, location.VerticalAccuracy);
}
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the ai's suggestions?
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 Geolocation_Tests (ToLocation_NoAltitude_UsesUnspecifiedReferenceSystem, ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem, ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid, ToLocation_MslAltitude_UsesGeoidReferenceSystem, LocationCopyConstructor_PreservesAltitudeReferenceSystem, ToLocation_MslAltitudeWithoutMslAccuracy_ReportsNullVerticalAccuracy) Category=Android Geolocation |
✅ FAIL — 1227s | ❌ FAIL — 349s |
🔴 Without fix — 📱 Geolocation_Tests (ToLocation_NoAltitude_UsesUnspecifiedReferenceSystem, ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem, ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid, ToLocation_MslAltitude_UsesGeoidReferenceSystem, LocationCopyConstructor_PreservesAltitudeReferenceSystem, ToLocation_MslAltitudeWithoutMslAccuracy_ReportsNullVerticalAccuracy): FAIL ✅ · 1227s
(truncated to last 15,000 chars)
t.Encoding.dll -> System.Text.Encoding.dll.so
[40/129] Xamarin.AndroidX.Lifecycle.Common.Jvm.dll -> Xamarin.AndroidX.Lifecycle.Common.Jvm.dll.so
[113/129] System.Text.Encodings.Web.dll -> System.Text.Encodings.Web.dll.so
[41/129] Xamarin.AndroidX.Lifecycle.LiveData.Core.dll -> Xamarin.AndroidX.Lifecycle.LiveData.Core.dll.so
[42/129] Xamarin.AndroidX.Lifecycle.ViewModel.Android.dll -> Xamarin.AndroidX.Lifecycle.ViewModel.Android.dll.so
[43/129] Xamarin.AndroidX.Lifecycle.ViewModelSavedState.Android.dll -> Xamarin.AndroidX.Lifecycle.ViewModelSavedState.Android.dll.so
[114/129] System.Text.RegularExpressions.dll -> System.Text.RegularExpressions.dll.so
[44/129] Xamarin.AndroidX.Loader.dll -> Xamarin.AndroidX.Loader.dll.so
[115/129] System.Threading.Tasks.Parallel.dll -> System.Threading.Tasks.Parallel.dll.so
[45/129] Xamarin.AndroidX.Navigation.Common.Android.dll -> Xamarin.AndroidX.Navigation.Common.Android.dll.so
[116/129] System.Threading.Tasks.dll -> System.Threading.Tasks.dll.so
[117/129] System.Text.Json.dll -> System.Text.Json.dll.so
[46/129] Xamarin.AndroidX.Navigation.Fragment.dll -> Xamarin.AndroidX.Navigation.Fragment.dll.so
[118/129] System.Threading.Thread.dll -> System.Threading.Thread.dll.so
[119/129] System.Threading.ThreadPool.dll -> System.Threading.ThreadPool.dll.so
[120/129] System.Threading.dll -> System.Threading.dll.so
[47/129] Xamarin.AndroidX.Navigation.Runtime.Android.dll -> Xamarin.AndroidX.Navigation.Runtime.Android.dll.so
[121/129] System.Xml.Linq.dll -> System.Xml.Linq.dll.so
[122/129] System.Xml.ReaderWriter.dll -> System.Xml.ReaderWriter.dll.so
[123/129] System.Xml.XDocument.dll -> System.Xml.XDocument.dll.so
[48/129] Xamarin.AndroidX.Navigation.UI.dll -> Xamarin.AndroidX.Navigation.UI.dll.so
[124/129] System.dll -> System.dll.so
[125/129] netstandard.dll -> netstandard.dll.so
[49/129] Xamarin.AndroidX.RecyclerView.dll -> Xamarin.AndroidX.RecyclerView.dll.so
[126/129] Mono.Android.Runtime.dll -> Mono.Android.Runtime.dll.so
[50/129] Xamarin.AndroidX.SavedState.SavedState.Android.dll -> Xamarin.AndroidX.SavedState.SavedState.Android.dll.so
[127/129] Java.Interop.dll -> Java.Interop.dll.so
[51/129] Xamarin.AndroidX.Security.SecurityCrypto.dll -> Xamarin.AndroidX.Security.SecurityCrypto.dll.so
[52/129] Xamarin.AndroidX.SwipeRefreshLayout.dll -> Xamarin.AndroidX.SwipeRefreshLayout.dll.so
[53/129] Xamarin.AndroidX.ViewPager.dll -> Xamarin.AndroidX.ViewPager.dll.so
[54/129] Xamarin.AndroidX.ViewPager2.dll -> Xamarin.AndroidX.ViewPager2.dll.so
[55/129] Xamarin.Google.Android.Material.dll -> Xamarin.Google.Android.Material.dll.so
[128/129] Mono.Android.dll -> Mono.Android.dll.so
[56/129] Xamarin.Google.Crypto.Tink.Android.dll -> Xamarin.Google.Crypto.Tink.Android.dll.so
[57/129] Xamarin.Kotlin.StdLib.dll -> Xamarin.Kotlin.StdLib.dll.so
[58/129] Xamarin.KotlinX.Coroutines.Core.Jvm.dll -> Xamarin.KotlinX.Coroutines.Core.Jvm.dll.so
[59/129] Xamarin.KotlinX.Serialization.Core.Jvm.dll -> Xamarin.KotlinX.Serialization.Core.Jvm.dll.so
[60/129] xunit.abstractions.dll -> xunit.abstractions.dll.so
[61/129] xunit.assert.dll -> xunit.assert.dll.so
[62/129] xunit.core.dll -> xunit.core.dll.so
[63/129] xunit.execution.dotnet.dll -> xunit.execution.dotnet.dll.so
[64/129] xunit.runner.utility.netcoreapp10.dll -> xunit.runner.utility.netcoreapp10.dll.so
[65/129] System.Collections.Concurrent.dll -> System.Collections.Concurrent.dll.so
[66/129] System.Collections.Immutable.dll -> System.Collections.Immutable.dll.so
[67/129] System.Collections.NonGeneric.dll -> System.Collections.NonGeneric.dll.so
[68/129] System.Collections.Specialized.dll -> System.Collections.Specialized.dll.so
[69/129] System.Collections.dll -> System.Collections.dll.so
[70/129] System.ComponentModel.Primitives.dll -> System.ComponentModel.Primitives.dll.so
[71/129] System.ComponentModel.TypeConverter.dll -> System.ComponentModel.TypeConverter.dll.so
[72/129] System.ComponentModel.dll -> System.ComponentModel.dll.so
[73/129] System.Console.dll -> System.Console.dll.so
[74/129] System.Diagnostics.Debug.dll -> System.Diagnostics.Debug.dll.so
[75/129] System.Diagnostics.DiagnosticSource.dll -> System.Diagnostics.DiagnosticSource.dll.so
[76/129] System.Diagnostics.Process.dll -> System.Diagnostics.Process.dll.so
[77/129] System.Diagnostics.Tools.dll -> System.Diagnostics.Tools.dll.so
[78/129] System.Diagnostics.TraceSource.dll -> System.Diagnostics.TraceSource.dll.so
[129/129] System.Private.CoreLib.dll -> System.Private.CoreLib.dll.so
[79/129] System.Diagnostics.Tracing.dll -> System.Diagnostics.Tracing.dll.so
[80/129] System.Drawing.Primitives.dll -> System.Drawing.Primitives.dll.so
[81/129] System.Drawing.dll -> System.Drawing.dll.so
[82/129] System.Formats.Asn1.dll -> System.Formats.Asn1.dll.so
[83/129] System.Globalization.dll -> System.Globalization.dll.so
[84/129] System.IO.Compression.Brotli.dll -> System.IO.Compression.Brotli.dll.so
[85/129] System.IO.Compression.dll -> System.IO.Compression.dll.so
[86/129] System.IO.FileSystem.dll -> System.IO.FileSystem.dll.so
[87/129] System.IO.Pipelines.dll -> System.IO.Pipelines.dll.so
[88/129] System.IO.dll -> System.IO.dll.so
[89/129] System.Linq.Expressions.dll -> System.Linq.Expressions.dll.so
[90/129] System.Linq.dll -> System.Linq.dll.so
[91/129] System.Memory.dll -> System.Memory.dll.so
[92/129] System.Net.Http.dll -> System.Net.Http.dll.so
[93/129] System.Net.NameResolution.dll -> System.Net.NameResolution.dll.so
[94/129] System.Net.Primitives.dll -> System.Net.Primitives.dll.so
[95/129] System.Net.Requests.dll -> System.Net.Requests.dll.so
[96/129] System.Net.Sockets.dll -> System.Net.Sockets.dll.so
[97/129] System.Numerics.Vectors.dll -> System.Numerics.Vectors.dll.so
[98/129] System.ObjectModel.dll -> System.ObjectModel.dll.so
[99/129] System.Private.Uri.dll -> System.Private.Uri.dll.so
[100/129] System.Private.Xml.Linq.dll -> System.Private.Xml.Linq.dll.so
[101/129] System.Private.Xml.dll -> System.Private.Xml.dll.so
[102/129] System.Reflection.Extensions.dll -> System.Reflection.Extensions.dll.so
[103/129] System.Reflection.TypeExtensions.dll -> System.Reflection.TypeExtensions.dll.so
[104/129] System.Reflection.dll -> System.Reflection.dll.so
[105/129] System.Runtime.Extensions.dll -> System.Runtime.Extensions.dll.so
[106/129] System.Runtime.InteropServices.RuntimeInformation.dll -> System.Runtime.InteropServices.RuntimeInformation.dll.so
[107/129] System.Runtime.InteropServices.dll -> System.Runtime.InteropServices.dll.so
[108/129] System.Runtime.Loader.dll -> System.Runtime.Loader.dll.so
[109/129] System.Runtime.Numerics.dll -> System.Runtime.Numerics.dll.so
[110/129] System.Runtime.dll -> System.Runtime.dll.so
[111/129] System.Security.Cryptography.dll -> System.Security.Cryptography.dll.so
[112/129] System.Text.Encoding.dll -> System.Text.Encoding.dll.so
[113/129] System.Text.Encodings.Web.dll -> System.Text.Encodings.Web.dll.so
[114/129] System.Text.Json.dll -> System.Text.Json.dll.so
[115/129] System.Text.RegularExpressions.dll -> System.Text.RegularExpressions.dll.so
[116/129] System.Threading.Tasks.Parallel.dll -> System.Threading.Tasks.Parallel.dll.so
[117/129] System.Threading.Tasks.dll -> System.Threading.Tasks.dll.so
[118/129] System.Threading.Thread.dll -> System.Threading.Thread.dll.so
[119/129] System.Threading.ThreadPool.dll -> System.Threading.ThreadPool.dll.so
[120/129] System.Threading.dll -> System.Threading.dll.so
[121/129] System.Xml.Linq.dll -> System.Xml.Linq.dll.so
[122/129] System.Xml.ReaderWriter.dll -> System.Xml.ReaderWriter.dll.so
[123/129] System.Xml.XDocument.dll -> System.Xml.XDocument.dll.so
[124/129] System.dll -> System.dll.so
[125/129] netstandard.dll -> netstandard.dll.so
[126/129] Java.Interop.dll -> Java.Interop.dll.so
[127/129] Mono.Android.Runtime.dll -> Mono.Android.Runtime.dll.so
[128/129] Mono.Android.dll -> Mono.Android.dll.so
[129/129] System.Private.CoreLib.dll -> System.Private.CoreLib.dll.so
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:10:09.79
[11.0.0-prerelease.26107.1+bfbac237157e59cdbd19334325b2af80bd6e9828] XHarness command issued: android test --app /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net10.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk --package-name com.microsoft.maui.essentials.devicetests --device-id emulator-5554 -o artifacts/log --timeout 01:00:00 -v --arg TestFilter=Category=Android Geolocation
�[40m�[37mdbug�[39m�[22m�[49m: ADBRunner using ADB.exe supplied from /home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/tools/net10.0/any/../../../runtimes/any/native/adb/linux/adb
�[40m�[37mdbug�[39m�[22m�[49m: Full resolved path:'/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb'
�[40m�[32minfo�[39m�[22m�[49m: Will attempt to find device supporting architectures: 'arm64-v8a', 'x86_64'
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb start-server'
�[40m�[37mdbug�[39m�[22m�[49m:
�[40m�[32minfo�[39m�[22m�[49m: Finding attached devices/emulators...
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb devices -l'
�[40m�[37mdbug�[39m�[22m�[49m: Found 1 possible devices
�[40m�[37mdbug�[39m�[22m�[49m: Evaluating output line for device serial: emulator-5554 device product:sdk_gphone_x86_64 model:sdk_gphone_x86_64 device:generic_x86_64_arm64 transport_id:1
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 shell getprop ro.product.cpu.abilist'
�[40m�[37mdbug�[39m�[22m�[49m: Found 1 possible devices. Using 'emulator-5554'
�[40m�[32minfo�[39m�[22m�[49m: Active Android device set to serial 'emulator-5554'
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 -s emulator-5554 shell getprop ro.product.cpu.abi'
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 -s emulator-5554 shell getprop ro.build.version.sdk'
�[40m�[32minfo�[39m�[22m�[49m: Waiting for device to be available (max 5 minutes)
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 wait-for-device'
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 -s emulator-5554 shell getprop sys.boot_completed'
�[40m�[37mdbug�[39m�[22m�[49m: sys.boot_completed = '1'
�[40m�[37mdbug�[39m�[22m�[49m: Waited 0 seconds for device boot completion
�[40m�[37mdbug�[39m�[22m�[49m: Working with emulator-5554 (API 30)
�[40m�[37mdbug�[39m�[22m�[49m: Check current adb install and/or package verification settings
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 shell settings get global verifier_verify_adb_installs'
�[40m�[37mdbug�[39m�[22m�[49m: verifier_verify_adb_installs = 0
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 shell settings get global package_verifier_enable'
�[40m�[37mdbug�[39m�[22m�[49m: package_verifier_enable =
�[40m�[1m�[33mwarn�[39m�[22m�[49m: Installing debug apks on a device might be rejected with INSTALL_FAILED_VERIFICATION_FAILURE. Make sure to set 'package_verifier_enable' to '0'
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.essentials.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.essentials.devicetests'
�[41m�[30mfail�[39m�[22m�[49m: Waiting for command timed out: execution may be compromised
�[41m�[30mfail�[39m�[22m�[49m: Error: Exit code: -2
Std out:
�[40m�[32minfo�[39m�[22m�[49m: Attempting to install /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net10.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 install /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net10.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk'
�[41m�[30mfail�[39m�[22m�[49m: Error:
Exit code: 1
Std out:
Serving...
Performing Incremental Install
cmd: Failure calling service package: Broken pipe (32)
Performing Streamed Install
Std err:
All files should be loaded. Notifying the device.
adb: failed to install /home/vsts/work/1/s/artifacts/bin/Essentials.DeviceTests/Release/net10.0-android/com.microsoft.maui.essentials.devicetests-Signed.apk: cmd: Can't find service: package
�[41m�[1m�[37mcrit�[39m�[22m�[49m: Install failure: Test command cannot continue
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.essentials.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.essentials.devicetests'
�[41m�[30mfail�[39m�[22m�[49m: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.essentials.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.essentials.devicetests'
�[41m�[30mfail�[39m�[22m�[49m: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
XHarness exit code: 78 (PACKAGE_INSTALLATION_FAILURE)
Tests completed with exit code: 78
🟢 With fix — 📱 Geolocation_Tests (ToLocation_NoAltitude_UsesUnspecifiedReferenceSystem, ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem, ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid, ToLocation_MslAltitude_UsesGeoidReferenceSystem, LocationCopyConstructor_PreservesAltitudeReferenceSystem, ToLocation_MslAltitudeWithoutMslAccuracy_ReportsNullVerticalAccuracy): FAIL ❌ · 349s
(truncated to last 15,000 chars)
05 12:22:04.788 11987 16256 W PhBaseOp: at dxzg.f(:com.google.android.gms@261631026@26.16.31 (150800-900800821):16)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at csew.fB(:com.google.android.gms@261631026@26.16.31 (150800-900800821):1)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at csfh.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):121)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at fttg.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):23)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at bfzu.c(:com.google.android.gms@261631026@26.16.31 (150800-900800821):50)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at bfzu.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):66)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at bgfl.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):8)
05-05 12:22:04.788 11987 16256 W PhBaseOp: at java.lang.Thread.run(Thread.java:923)
05-05 12:22:04.792 12609 15676 W gle.android.gm: Verification of void hwkv.b(hwhy) took 130.372ms (1273.27 bytecodes/s) (5200B approximate peak alloc)
05-05 12:22:04.798 11987 16280 I SystemMemoryMap: (REDACTED) [activityrecognition] serialization complete (extras/values=%.2f/%.2fkb)
05-05 12:22:04.812 11987 16256 W AsyncOperation: operation=CommitToConfigurationOperationCall, opStatusCode=29538 [CONTEXT service_id=51 ]
05-05 12:22:04.812 11987 16256 W AsyncOperation: OperationException[Status{statusCode=Stale snapshot for com.google.android.apps.wellbeing.device(change count changed - expected 5 but was 4), resolution=null}]
05-05 12:22:04.812 11987 16256 W AsyncOperation: at dxzg.h(:com.google.android.gms@261631026@26.16.31 (150800-900800821):64)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at dxzg.f(:com.google.android.gms@261631026@26.16.31 (150800-900800821):16)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at csew.fB(:com.google.android.gms@261631026@26.16.31 (150800-900800821):1)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at csfh.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):121)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at fttg.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):23)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at bfzu.c(:com.google.android.gms@261631026@26.16.31 (150800-900800821):50)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at bfzu.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):66)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at bgfl.run(:com.google.android.gms@261631026@26.16.31 (150800-900800821):8)
05-05 12:22:04.812 11987 16256 W AsyncOperation: at java.lang.Thread.run(Thread.java:923)
05-05 12:22:04.820 12609 16260 I ChimeraConfigurator: Checking 1 listeners. All downloaded: false
05-05 12:22:04.829 16209 16224 W DynamiteModule: Local module descriptor class for com.google.android.gms.googlecertificates not found.
05-05 12:22:04.861 16209 16224 W .apps.wellbein: ClassLoaderContext classpath size mismatch. expected=14, found=0 (DLC[];PCL[base.apk*3555418091:base.apk!classes2.dex*3336391460:base.apk!classes3.dex*3958618713:base.apk!classes4.dex*3022934062:base.apk!classes5.dex*2476764232:base.apk!classes6.dex*1612679893:base.apk!classes7.dex*1823849344:base.apk!classes8.dex*466855726:base.apk!classes9.dex*4227236518:base.apk!classes10.dex*169475094:base.apk!classes11.dex*2053099836:base.apk!classes12.dex*95922838:base.apk!classes13.dex*3324838914:base.apk!classes14.dex*2736438263]{PCL[/system/framework/org.apache.http.legacy.jar*1452144318]#PCL[/system/framework/com.android.location.provider.jar*4177849200]#PCL[/system/framework/com.android.media.remotedisplay.jar*1128910120]#PCL[/system_ext/framework/androidx.window.sidecar.jar*709954415]} | DLC[];PCL[])
05-05 12:22:04.863 16209 16224 I DynamiteModule: Considering local module com.google.android.gms.googlecertificates:0 and remote module com.google.android.gms.googlecertificates:7
05-05 12:22:04.863 16209 16224 I DynamiteModule: Selected remote version of com.google.android.gms.googlecertificates, version >= 7
05-05 12:22:04.866 16209 16224 W System : ClassLoader referenced unknown path:
05-05 12:22:04.902 12326 12821 I Finsky : [166] SCH: Scheduling phonesky job Id: 26-1414141414, CT: 1778008538037, Constraints: [{ L: 43199988, D: 44099988, C: CHARGING_NONE, I: IDLE_NONE, N: NET_NONE, B: BATTERY_ANY }]
05-05 12:22:04.925 12326 12637 I Finsky : [141] SCH: Scheduling 1 system job(s)
05-05 12:22:04.925 12326 12637 I Finsky : [141] SCH: Scheduling system job Id: 9307, L: 42813100, D: 43713100, C: false, I: false, N: 0
05-05 12:22:04.928 12326 16283 I Finsky : [235] SCH: job service finished with id 9300.
05-05 12:22:04.932 16209 16224 W .apps.wellbein: ClassLoaderContext classpath size mismatch. expected=14, found=2 (DLC[];PCL[base.apk*3555418091:base.apk!classes2.dex*3336391460:base.apk!classes3.dex*3958618713:base.apk!classes4.dex*3022934062:base.apk!classes5.dex*2476764232:base.apk!classes6.dex*1612679893:base.apk!classes7.dex*1823849344:base.apk!classes8.dex*466855726:base.apk!classes9.dex*4227236518:base.apk!classes10.dex*169475094:base.apk!classes11.dex*2053099836:base.apk!classes12.dex*95922838:base.apk!classes13.dex*3324838914:base.apk!classes14.dex*2736438263]{PCL[/system/framework/org.apache.http.legacy.jar*1452144318]#PCL[/system/framework/com.android.location.provider.jar*4177849200]#PCL[/system/framework/com.android.media.remotedisplay.jar*1128910120]#PCL[/system_ext/framework/androidx.window.sidecar.jar*709954415]} | DLC[];PCL[/product/priv-app/WellbeingPrebuilt/WellbeingPrebuilt.apk*3105014714:/product/priv-app/WellbeingPrebuilt/WellbeingPrebuilt.apk!classes2.dex*659704715]{PCL[/system/framework/android.test.base.jar*315452938]#PCL[/system/framework/org.apache.http.legacy.jar*1452144318]})
05-05 12:22:04.932 16209 16224 W .apps.wellbein: Found duplicate classes, falling back to extracting from APK : /data/app/~~kkSGM7GxDm1qKV1RGeojPA==/com.google.android.gms-1YSasfZqvqngMMGQ2T91Ow==/split_GoogleCertificates_installtime.apk
05-05 12:22:04.932 16209 16224 W .apps.wellbein: NOTE: This wastes RAM and hurts startup performance.
05-05 12:22:04.932 16209 16224 W .apps.wellbein: Found duplicated class when checking oat files: 'Landroid/support/annotation/Keep;' in /data/app/~~kkSGM7GxDm1qKV1RGeojPA==/com.google.android.gms-1YSasfZqvqngMMGQ2T91Ow==/split_GoogleCertificates_installtime.apk!classes2.dex and /product/priv-app/WellbeingPrebuilt/WellbeingPrebuilt.apk
05-05 12:22:04.996 12326 12637 I Finsky : [141] Not an active Cubes user. Ignoring package install/uninstall event.
05-05 12:22:05.003 11987 11991 I .gms.persisten: Background young concurrent copying GC freed 53922(2595KB) AllocSpace objects, 12(368KB) LOS objects, 5% free, 40MB/43MB, paused 24us total 351.392ms
05-05 12:22:05.290 12609 13636 I cn_CronetUrlRequestContext: destroyNativeStreamLocked org.chromium.net.impl.CronetBidirectionalStream@988c7f5
05-05 12:22:05.292 12609 15676 W feks : Primes not initialized, returning default (no-op) Primes instance which will ignore all calls. Please call Primes.initialize(...) before using any Primes API.
05-05 12:22:05.468 12609 16134 I Icing : IndexChimeraService.getServiceInterface callingPackage=com.google.android.gms componentName=AppsCorpus serviceId=36
05-05 12:22:05.505 11373 12210 W PackageManager: Cannot suspend package "com.android.vending": required for package verification
05-05 12:22:05.506 11373 12210 W PackageManager: Cannot suspend the platform package: android
05-05 12:22:05.506 11373 12210 W PackageManager: Cannot suspend package "com.google.android.permissioncontroller": required for permissions management
05-05 12:22:05.507 11373 12210 W PackageManager: Cannot suspend package "com.android.dialer": is the default dialer
05-05 12:22:05.507 11373 12210 W PackageManager: Cannot suspend package "com.google.android.packageinstaller": required for package installation
05-05 12:22:05.508 11373 12210 W PackageManager: Cannot suspend package "com.google.android.apps.nexuslauncher": contains the active launcher
05-05 12:22:03.300 11373 11373 W Binder:11373_12: type=1400 audit(0.0:428): avc: denied { getopt } for scontext=u:r:system_server:s0 tcontext=u:r:shell:s0 tclass=unix_stream_socket permissive=0
05-05 12:22:05.588 12609 15676 I Icing : Usage reports ok 0, Failed Usage reports 0, indexed 0, rejected 0
05-05 12:22:05.592 12609 16134 I Icing : IndexChimeraService.getServiceInterface callingPackage=com.google.android.gms componentName=null serviceId=30
05-05 12:22:05.626 12609 12617 I gle.android.gm: Background concurrent copying GC freed 257554(10MB) AllocSpace objects, 36(12MB) LOS objects, 49% free, 23MB/46MB, paused 36us total 894.923ms
05-05 12:22:05.665 288 1140 I bt : system/bt/vendor_libs/test_vendor_lib/model/devices/scripted_beacon.cc:113 set_state: Events file not opened yet, for event: 2
05-05 12:22:05.672 288 1140 I bt : system/bt/vendor_libs/test_vendor_lib/model/devices/scripted_beacon.cc:113 set_state: Events file not opened yet, for event: 3
05-05 12:22:03.300 11373 11373 W Binder:11373_12: type=1400 audit(0.0:429): avc: denied { getopt } for scontext=u:r:system_server:s0 tcontext=u:r:shell:s0 tclass=unix_stream_socket permissive=0
05-05 12:22:05.983 12609 15676 I Icing : Usage reports ok 0, Failed Usage reports 0, indexed 0, rejected 0
05-05 12:22:05.993 12609 15676 I Icing : Indexing com.google.android.gms-apps from com.google.android.gms
05-05 12:22:06.136 11987 13383 I Nearby : (REDACTED) is blocked device type %s, isAuto=%s, isIot=%s, isLatchsky=%s, isChinaWearable=%s, isTv=%s, isWearable=%s (supportWearOs=%s), isChromeOsDevice=%s (supportChromeOs=%s)
05-05 12:22:06.140 12609 15676 I Icing : Indexing done com.google.android.gms-apps
05-05 12:22:06.145 11987 13383 I NearbyDiscovery: (REDACTED) DiscoveryController, showNotification, itemSize=%s
05-05 12:22:06.145 11987 13383 I NearbyDiscovery: (REDACTED) Show notifications: %d total, no changes since last shown, no-op.
05-05 12:22:06.223 16301 16301 D AndroidRuntime: >>>>>> START com.android.internal.os.RuntimeInit uid 2000 <<<<<<
05-05 12:22:06.270 16301 16301 E libc : Access denied finding property "persist.device_config.runtime_native_boot.profilebootclasspath"
05-05 12:22:06.270 16301 16301 E libc : Access denied finding property "persist.device_config.runtime_native_boot.enable_apex_image"
05-05 12:22:06.270 16301 16301 I AndroidRuntime: Using default boot image
05-05 12:22:06.271 16301 16301 E libc : Access denied finding property "persist.device_config.runtime_native_boot.disable_lock_profiling"
05-05 12:22:06.271 16301 16301 I AndroidRuntime: Leaving lock profiling enabled
05-05 12:22:06.271 16301 16301 E libc : Access denied finding property "persist.device_config.runtime_native_boot.enable_generational_cc"
05-05 12:22:06.256 16301 16301 W app_process: type=1400 audit(0.0:430): avc: denied { read } for name="u:object_r:device_config_runtime_native_boot_prop:s0" dev="tmpfs" ino=7421 scontext=u:r:shell:s0 tcontext=u:object_r:device_config_runtime_native_boot_prop:s0 tclass=file permissive=0
05-05 12:22:06.508 16301 16301 D app_process: Time zone APEX ICU file found: /apex/com.android.tzdata/etc/icu/icu_tzdata.dat
05-05 12:22:06.508 16301 16301 D app_process: I18n APEX ICU file found: /apex/com.android.i18n/etc/icu/icudt66l.dat
05-05 12:22:06.534 16301 16301 W app_process: Unexpected CPU variant for X86 using defaults: x86_64
05-05 12:22:06.539 16301 16301 I app_process: The ClassLoaderContext is a special shared library.
05-05 12:22:06.268 16301 16301 W app_process: type=1400 audit(0.0:431): avc: denied { read } for name="u:object_r:device_config_runtime_native_boot_prop:s0" dev="tmpfs" ino=7421 scontext=u:r:shell:s0 tcontext=u:object_r:device_config_runtime_native_boot_prop:s0 tclass=file permissive=0
05-05 12:22:06.574 16301 16301 W app_process: JNI RegisterNativeMethods: attempt to register 0 native methods for android.media.AudioAttributes
05-05 12:22:06.617 16301 16301 D AndroidRuntime: Calling main entry com.android.commands.am.Am
05-05 12:22:06.649 16301 16301 D AndroidRuntime: Shutting down VM
05-05 12:22:06.268 16301 16301 W app_process: type=1400 audit(0.0:432): avc: denied { read } for name="u:object_r:device_config_runtime_native_boot_prop:s0" dev="tmpfs" ino=7421 scontext=u:r:shell:s0 tcontext=u:object_r:device_config_runtime_native_boot_prop:s0 tclass=file permissive=0
05-05 12:22:06.268 16301 16301 W app_process: type=1400 audit(0.0:433): avc: denied { read } for name="u:object_r:device_config_runtime_native_boot_prop:s0" dev="tmpfs" ino=7421 scontext=u:r:shell:s0 tcontext=u:object_r:device_config_runtime_native_boot_prop:s0 tclass=file permissive=0
05-05 12:22:07.129 433 433 E netmgr : qemu_pipe_open_ns:62: Could not connect to the 'pipe:qemud:network' service: Invalid argument
05-05 12:22:07.129 433 433 E netmgr : Failed to open QEMU pipe 'qemud:network': Invalid argument
05-05 12:22:07.157 12609 16272 I GmsDebugLogger: [56] Pay.optional_261631100800_3.apk
05-05 12:22:07.166 12609 16272 I DynamicModuleDownloader: Using base module file stored externally.
05-05 12:22:07.224 453 453 E wifi_forwarder: qemu_pipe_open_ns:62: Could not connect to the 'pipe:qemud:wififorward' service: Invalid argument
05-05 12:22:07.224 453 453 E wifi_forwarder: RemoteConnection failed to initialize: RemoteConnection failed to open pipe
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.essentials.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26107.1/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.essentials.devicetests'
�[40m�[32minfo�[39m�[22m�[49m: Successfully uninstalled com.microsoft.maui.essentials.devicetests
XHarness exit code: 82 (RETURN_CODE_NOT_SET)
Tests completed with exit code: 82
⚠️ Failure Details
- ❌ Geolocation_Tests (ToLocation_NoAltitude_UsesUnspecifiedReferenceSystem, ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem, ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid, ToLocation_MslAltitude_UsesGeoidReferenceSystem, LocationCopyConstructor_PreservesAltitudeReferenceSystem, ToLocation_MslAltitudeWithoutMslAccuracy_ReportsNullVerticalAccuracy) FAILED with fix (should pass)
📁 Fix files reverted (3 files)
eng/pipelines/ci-copilot.ymlsrc/Essentials/src/Types/Location.shared.cssrc/Essentials/src/Types/LocationExtensions.android.cs
🧪 UI Tests — Category Detection
Detected UI test categories: Essentials
🔍 Regression Cross-Reference
🔍 Regression Cross-Reference
🟢 No regression risks detected. No labeled bug-fix PRs in the last 6 months touched the modified files.
🔍 Pre-Flight — Context & Validation
Issue: #27554 - Add support for MslAltitudeMeters in Essentials Geolocation on Android
PR: #35097 - [Essentials] Use mean sea level altitude on Android API 34+
Platforms Affected: Android (API 34+ new behavior; pre-34 fallback preserved; all platforms via copy constructor fix)
Files Changed: 2 implementation, 1 test
Key Findings
- PR adds
GetAltitude()helper inLocationExtensions.android.csthat prefers MSL altitude on API 34+ (paired correctly withMslAltitudeAccuracyMeters) and falls back to ellipsoidal altitude on older devices - Copy constructor
Location(Location point)was missingAltitudeReferenceSystempropagation — fixed inLocation.shared.cs:106 - Previously,
HasAltitude == falsereturned(null, Ellipsoid)— semantically incoherent; now correctly returns(null, Unspecified) - New test file adds 6 Android device tests covering all branches of
GetAltitude()plus the copy constructor fix - Prior code review (session
63444de) flaggedVerticalAccuracy/MSL mismatch — fully addressed in current PR:GetAltitude()pairs altitude and accuracy from the same reference system - Two tests (
ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystemandToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid) are functionally identical — both exercise the same code paths
Code Review Summary
Verdict: LGTM
Confidence: high
Errors: 0 | Warnings: 0 | Suggestions: 3
Key code review findings:
- 💡
src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs:46— Duplicate test:ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoidis byte-for-byte identical toToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem; one should be removed - 💡
src/Essentials/src/Types/LocationExtensions.android.cs:70— Cross-platform divergence: iOS always returnsAltitudeReferenceSystem.Geoideven when no altitude is available; Android now correctly returnsUnspecified; worth tracking a follow-up iOS fix - 💡
src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs:108—LocationCopyConstructor_PreservesAltitudeReferenceSystemtests sharedLocation.shared.cscode but lives in Android-only test folder; should be insrc/Essentials/test/UnitTests/for all-platform coverage
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35097 | Add GetAltitude() helper to prefer MSL on API 34+, fix copy constructor |
❌ FAILED (Gate) | LocationExtensions.android.cs, Location.shared.cs, new test file |
Gate failed — see gate/content.md |
🔬 Code Review — Deep Analysis
Code Review — PR #35097
Independent Assessment
What this changes:
Location.shared.cs— The copy constructorLocation(Location point)was missingAltitudeReferenceSystem = point.AltitudeReferenceSystem. The property is now propagated correctly during copy (line 106).LocationExtensions.android.cs—ToLocation(AndroidLocation)is refactored from an expression-body to a block body. A new privateGetAltitude()helper encapsulates altitude selection:- API 34+ with
HasMslAltitude: returns(MslAltitudeMeters, Geoid, MslAltitudeAccuracyMeters) - Otherwise with
HasAltitude: returns(Altitude, Ellipsoid, VerticalAccuracyMeters@API26+) - No altitude: returns
(null, Unspecified, null)
- API 34+ with
Geolocation_Tests.cs— New Android device-test file with 6 tests covering all branches ofGetAltitudeand the copy-constructor fix.
Inferred motivation:
- The copy constructor bug silently dropped
AltitudeReferenceSystemduringPlacemarkcopies (seePlacemark.shared.cs:30). - On Android, the prior implementation always returned
AltitudeReferenceSystem.Ellipsoideven whenHasAltitudewasfalseandAltitudewasnull— a semantically incoherent state. More importantly, it never exposed the geoid-corrected (MSL) altitude introduced in Android API 34, forcing cross-platform consumers to apply manual geoid corrections to reconcile Android values with iOS and Windows.
Is the approach sound?
Yes. The GetAltitude helper is clean and ensures altitude value and vertical accuracy are always sourced from the same reference system — the key invariant. The approach is minimal and consistent with the existing API-level gating style used elsewhere in the same file (IsAndroidVersionAtLeast(26) for VerticalAccuracy, IsAndroidVersionAtLeast(31) for Mock).
Reconciliation with PR Narrative
Author claims: Fixes #27554 — prefer MslAltitudeMeters on API 34+, fall back to ellipsoidal altitude, report Unspecified when no altitude. Change is opaque to callers. The existing Geolocation_Tests.cs device tests will exercise the new path on CI.
Agreement: Direction, motivation, and implementation all match. The VerticalAccuracy/MSL mismatch flagged in the previous review (session 63444de) has been fully addressed — GetAltitude now returns altitude and vertical accuracy together, ensuring they always come from the same reference system.
Disagreement — minor: The claim that existing Geolocation_Tests.cs tests cover the new paths was never accurate (those tests don't assert on Altitude or AltitudeReferenceSystem). A new test file was added to address this, though two of the six tests are functionally identical (see Findings).
Author update (2026-05-03): Copy-constructor fix and test added. Confirmed in comment.
Findings
💡 Suggestion — Duplicate test: ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid
src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs:46–70
ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid (line 46) and ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem (line 24) have byte-for-byte identical setup and assertions. Neither sets MslAltitudeMeters, so on API 34+ both have HasMslAltitude == false and exercise the same fallback branch. The comment in the second test claims it "exercises the HasMslAltitude == false fallback branch" on API 34+ — but the first test exercises that exact same branch. One of the two should be removed.
💡 Suggestion — Cross-platform AltitudeReferenceSystem divergence when no altitude
src/Essentials/src/Types/LocationExtensions.android.cs:70
After this PR:
- iOS (
LocationExtensions.ios.tvos.watchos.macos.cs:45): always returnsAltitudeReferenceSystem = Geoid, even whenAltitude == null(theCLLocation.VerticalAccuracy < 0path). - Android (new): returns
AltitudeReferenceSystem = Unspecifiedwhen no altitude.
A cross-platform consumer using AltitudeReferenceSystem != Unspecified to detect altitude availability will see platform-divergent behavior. The Android behavior is semantically correct (Unspecified is the right value when Altitude is null), but the PR's stated goal is to improve Android/iOS alignment, and for the no-altitude case it now diverges from iOS. Consider a follow-up to fix the iOS case (Unspecified when VerticalAccuracy < 0), or at minimum document the divergence in the PR description.
💡 Suggestion — Cross-platform copy-constructor test lives in Android-only folder
src/Essentials/test/DeviceTests/Tests/Android/Geolocation_Tests.cs:107–122
LocationCopyConstructor_PreservesAltitudeReferenceSystem tests Location(Location point) in Location.shared.cs, which is compiled for all platforms. Placing it under Tests/Android/ means it only runs when Android device-test CI is scheduled. There is no existing Location_Tests.cs in src/Essentials/test/UnitTests/, but adding this test there (as a plain xUnit unit test with no Android dependency) would give it platform-agnostic, always-on coverage and prevent the copy-constructor regression from slipping through on a non-Android run.
Devil's Advocate
API guard correctness: The OperatingSystem.IsAndroidVersionAtLeast(34) && location.HasMslAltitude pattern is identical in style to the existing IsAndroidVersionAtLeast(26) and IsAndroidVersionAtLeast(31) guards in the same file. CA1416 is satisfied by short-circuit evaluation. No [SupportedOSPlatform] annotation is needed. ✅
Blast radius of Ellipsoid → Unspecified: No internal MAUI consumer branches on the old value. AltitudeReferenceSystem is only read in GeolocationViewModel.cs for display. External consumers could theoretically condition on Ellipsoid as a sentinel for "Android-no-altitude", but the previous state was indefensible (null altitude paired with a non-null reference system). ✅
float → double implicit cast for MslAltitudeAccuracyMeters: Consistent with how VerticalAccuracyMeters (also float) was already handled in the pre-existing code. Safe widening. ✅
Could a device return HasMslAltitude == true but MslAltitudeMeters == 0.0? The Android docs say getMslAltitudeMeters() must be set via setMslAltitudeMeters(float) before hasMslAltitude() returns true, so HasMslAltitude is reliable as a gate. ✅
Does the existing API 26 VerticalAccuracy still apply on the ellipsoidal fallback path? Yes — the HasAltitude branch retains the exact same IsAndroidVersionAtLeast(26) && HasVerticalAccuracy guard from the pre-PR code, so no regression there. ✅
Verdict: LGTM
Confidence: high
Summary: The implementation is correct, well-structured, and addresses a real cross-platform inconsistency. The previous review's primary concern (VerticalAccuracy/MSL mismatch) has been fully resolved in the updated code. All three findings are suggestions — none are blockers. The duplicate test is cosmetic; the iOS divergence and test-placement issues are reasonable follow-up work. The API gating is sound and consistent with the existing patterns in the file. Ready for human approval.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Inline local variables — no helper method, if/else-if/else branching in ToLocation() directly |
3 files | Avoids tuple allocation; clean but less structured | |
| 2 | try-fix | PR's GetAltitude helper (private) + moved copy constructor test to UnitTests + Timestamp bug fix |
4 files (new UnitTests/LocationTests.cs) | Best test architecture; also fixes pre-existing Timestamp copy bug | |
| 3 | try-fix | Switch expression / declarative tuple selection in ToLocation() |
3 files | Declarative style; build-only "Pass" — not a real device test pass | |
| 4 | try-fix | Named AltitudeInfo readonly struct with Resolve() factory |
4 files (new UnitTests/Location_Tests.cs) | Strongest encapsulation; unit tests pass | |
| PR | PR #35097 | GetAltitude() tuple helper + copy constructor fix |
❌ FAILED (Gate) | 3 files | Original PR; gate failed (see gate/content.md) |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-sonnet-4.6 | 2 | No | "NO NEW IDEAS — the PR's ValueTuple helper is the right call. out-parameter style is the only structural variant not tried but is strictly worse." |
Exhausted: Yes
Selected Fix: pr-plus-reviewer (PR fix with code review suggestions applied) — reason: All try-fix candidates were blocked due to no Android device, so empirical validation was unavailable. The PR's core implementation is correct; Attempt 2 (claude-sonnet-4.6) shows the best incremental improvements (private modifier, dedup test, move copy constructor test to UnitTests), which together constitute the pr-plus-reviewer candidate.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #27554, 3 files changed, prior review found VerticalAccuracy/MSL mismatch (now fixed) |
| Code Review | LGTM (high) | 0 errors, 0 warnings, 3 suggestions |
| Gate | ❌ FAILED | android — tests did NOT behave as expected (see gate/content.md) |
| Try-Fix | ✅ COMPLETE (4 attempts) | 0 passing, 4 blocked (no Android device/emulator), 1 build-only "Pass" (Attempt 3 — not a real device run) |
| Expert Review | ✅ COMPLETE | pr-plus-reviewer candidate produced with reviewer feedback applied |
| Report | ✅ COMPLETE |
Candidate Comparison
| Candidate | Approach | Test Result | Score |
|---|---|---|---|
pr |
GetAltitude helper, copy constructor fix | ❌ Gate FAILED | 2nd |
pr-plus-reviewer |
PR + private modifier + remove duplicate test + move copy constructor test to UnitTests | 1st (best) | |
try-fix-1 |
Inline local vars (no helper) | 3rd | |
try-fix-2 |
GetAltitude helper + test architecture + Timestamp fix | 3rd | |
try-fix-3 |
Switch expression (declarative tuple) | 3rd | |
try-fix-4 |
AltitudeInfo readonly struct | 3rd |
Code Review Impact on Try-Fix
The code review found 3 💡 suggestions (no ❌ errors). These directly shaped the try-fix exploration:
- All 4 models independently recognized the duplicate test issue and addressed it in their approaches
- Attempt 2 (claude-sonnet-4.6) specifically addressed the copy constructor test placement by creating
LocationTests.csin UnitTests - Attempt 4 applied all 3 reviewer suggestions plus a named struct approach
- No model found a fundamentally different implementation — consensus is that the PR's core approach (paired altitude+accuracy selection with API gating) is correct
Summary
PR #35097 adds MSL altitude support on Android API 34+ with a clean GetAltitude() helper. The core implementation is sound and correct. The gate failed because the tests include guards (if (!OperatingSystem.IsAndroidVersionAtLeast(34)) return;) that silently skip the key assertions on pre-34 test devices, so the gate cannot verify the tests fail without the fix. Three code-quality suggestions from the expert review should be applied: (1) add private modifier to GetAltitude(), (2) remove the byte-for-byte duplicate test ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid, (3) move LocationCopyConstructor_PreservesAltitudeReferenceSystem to a platform-agnostic unit test project.
Root Cause
Android API 34 introduced getMslAltitudeMeters() which returns geoid-corrected altitude, matching iOS/Windows behavior. The prior implementation always returned AltitudeReferenceSystem.Ellipsoid with WGS84 ellipsoidal altitude, requiring consumers to apply a manual geoid correction (~0-50m) to reconcile Android readings with other platforms. Additionally, the Location copy constructor was silently dropping AltitudeReferenceSystem during copies.
Fix Quality
The PR's fix is architecturally sound: it correctly pairs altitude and vertical accuracy from the same reference system (a design invariant), uses the established OperatingSystem.IsAndroidVersionAtLeast(N) API gating pattern, and is backward-compatible (pre-34 devices unchanged). The behavioral change from (null, Ellipsoid) → (null, Unspecified) when no altitude is available is semantically correct but is a subtle breaking change for consumers that used Ellipsoid as an Android sentinel. The pr-plus-reviewer candidate (PR + 3 code quality fixes) is the best result: it addresses all reviewer-identified issues with no regressions.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 6 findings
See inline comments for details.
| else | ||
| Assert.Null(location.VerticalAccuracy); | ||
| } | ||
|
|
There was a problem hiding this comment.
[major] Regression Prevention — Duplicate test — ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem (this test) and ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid (lines 64–87) are functionally identical: both create new AndroidLocation("test") { Altitude = 123.45 }, optionally set VerticalAccuracyMeters, and assert AltitudeReferenceSystem.Ellipsoid. On API 34+, neither test sets MslAltitudeMeters, so HasMslAltitude is false in both — the same fallback branch is hit. The long comment in the third test asserting it uniquely exercises the API-34 fallback branch is incorrect. Remove ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid; a test named 'HasAltitudeButNoMslAltitude' that is semantically indistinguishable from 'EllipsoidalAltitude' will silently pass even if the fallback branch is accidentally removed.
| var location = androidLocation.ToLocation(); | ||
|
|
||
| Assert.Equal(123.45, location.Altitude); | ||
| Assert.Equal(AltitudeReferenceSystem.Ellipsoid, location.AltitudeReferenceSystem); |
There was a problem hiding this comment.
[major] Regression Prevention — Duplicate test (see finding at line 44) — ToLocation_HasAltitudeButNoMslAltitude_UsesEllipsoid is byte-for-byte equivalent to the preceding ToLocation_EllipsoidalAltitude_UsesEllipsoidReferenceSystem. Both tests create an AndroidLocation with only Altitude = 123.45, leave MslAltitudeMeters unset, and assert Ellipsoid. The comment 'On every API level … exercises the HasMslAltitude == false fallback branch' is true of both tests. Remove this test.
| AltitudeReferenceSystem = AltitudeReferenceSystem.Geoid, | ||
| VerticalAccuracy = 2.5 | ||
| }; | ||
|
|
There was a problem hiding this comment.
[moderate] Regression Prevention — Wrong test project for shared-code test — LocationCopyConstructor_PreservesAltitudeReferenceSystem exercises only the shared Location(Location point) constructor in Location.shared.cs; no Android APIs are used. Placing it in Tests/Android/ means it only runs on Android device-test CI legs and will not catch a copy-constructor regression on an iOS or Windows run. Move it to a new src/Essentials/test/UnitTests/Location_Tests.cs so it runs on every platform without requiring a device.
| return (location.Altitude, AltitudeReferenceSystem.Ellipsoid, ellipsoidAccuracy); | ||
| } | ||
|
|
||
| return (null, AltitudeReferenceSystem.Unspecified, null); |
There was a problem hiding this comment.
[moderate] Cross-Platform Consistency — AltitudeReferenceSystem diverges from iOS when no altitude is available — iOS (LocationExtensions.ios.tvos.watchos.macos.cs line 45) always sets AltitudeReferenceSystem = AltitudeReferenceSystem.Geoid even when Altitude is null (when VerticalAccuracy < 0). After this PR, Android returns Unspecified when HasAltitude is false. This is semantically more correct — there is no meaningful reference system when there is no altitude value — but it creates a cross-platform inconsistency: a consumer checking location.AltitudeReferenceSystem != AltitudeReferenceSystem.Unspecified as a proxy for altitude availability will get different results on iOS vs Android. The PR description frames the goal as cross-platform consistency; the no-altitude case now diverges. Recommend either: (a) open a follow-up to fix iOS to return Unspecified when altitude is unavailable, or (b) add an XML doc comment to AltitudeReferenceSystem.Unspecified documenting when each platform emits it.
| // values are consistent across platforms without manual geoid correction. | ||
| // Altitude and its accuracy are selected together so they always come from | ||
| // the same reference system (MSL/geoid vs WGS84/ellipsoid). | ||
| static (double? Altitude, AltitudeReferenceSystem Reference, double? VerticalAccuracy) GetAltitude(AndroidLocation location) |
There was a problem hiding this comment.
[minor] Logic and Correctness — Missing explicit private modifier on GetAltitude — The method has no access modifier (C# defaults to private inside a class). Explicitly writing private static would match the style of other private helpers and makes intent immediately clear without readers needing to recall the default.
| @@ -103,6 +103,7 @@ public Location(Location point) | |||
| Longitude = point.Longitude; | |||
| Timestamp = DateTime.UtcNow; | |||
There was a problem hiding this comment.
[minor] Logic and Correctness — Copy constructor resets Timestamp instead of copying it — Timestamp = DateTime.UtcNow discards the source location's timestamp. This is a pre-existing behaviour but the PR modifies the copy constructor, making this a good opportunity to either fix the copy (Timestamp = point.Timestamp) or add an explicit XML doc comment explaining why the time is intentionally reset. As written, a caller wishing to clone a Location and preserve its recorded timestamp cannot use this constructor.
Fixes #27554.
Android API level 34 introduced
Location.getMslAltitudeMeters(),which reports altitude measured against the geoid (mean sea level).
The existing implementation always reads the WGS84 ellipsoidal
altitude, which is reported inconsistently with iOS and Windows where
altitude is typically expressed against the geoid. As the issue notes,
this forces consumers to apply manual geoid corrections to get
consistent results across platforms.
This change updates
LocationExtensions.android.csto:MslAltitudeMeterswhen the device runs Android 34+ andHasMslAltitudeistrue, reportingAltitudeReferenceSystem.Geoid.Altitude/Ellipsoidpath on olderdevices or when MSL altitude is not available.
AltitudeReferenceSystem.Unspecifiedwhen no altitude isavailable at all (previously this case was reported as
Ellipsoideven thoughAltitudewasnull).The MSL preference is opaque to callers —
Location.Altitudekeepsthe same shape and semantics; consumers that care about the reference
system can inspect
AltitudeReferenceSystemas before.Issues Fixed
Fixes #27554
Tested
OperatingSystem.IsAndroidVersionAtLeast(34)so older devices are unaffected.
MslAltitudeMetersis surfaced will rely on existing
Geolocation_Tests.csdevice testsexercised on CI.