Harden style changes #568
Harden style changes #568
Conversation
LukasPaczos
added
the
location-layer-plugin
tobrun
left a comment
tobrun
left a comment
There was a problem hiding this comment.
is looking great! just some minor questions/remarks
| Assert.assertThat(plugin.locationEngine, nullValue()) | ||
| Assert.assertEquals(point.latitude(), location.latitude, 0.1) | ||
| Assert.assertEquals(point.longitude(), location.longitude, 0.1) | ||
| assertThat(plugin.locationEngine, nullValue()) |
There was a problem hiding this comment.
is this equivalent to assertNull(plugin.locationEgine)?
There was a problem hiding this comment.
Yes, and since that was the approach taken with this test suite before I took over, I think I'm going to stick with it. Going to do some cleanups of those assertions though.
| Assert.assertEquals(point.latitude(), location.latitude, 0.1) | ||
| Assert.assertEquals(point.longitude(), location.longitude, 0.1) | ||
| assertThat(plugin.locationEngine, nullValue()) | ||
| assertEquals(point.latitude(), location.latitude, 0.1) |
There was a problem hiding this comment.
The selected delta atm: The first decimal place is worth up to 11.1 km: it can distinguish the position of one large city from a neighboring large city. I feel this can be made more accurate:
For example: The third decimal place is worth up to 110 m: it can identify a large agricultural field or institutional campus. or
The fourth decimal place is worth up to 11 m: it can identify a parcel of land. It is comparable to the typical accuracy of an uncorrected GPS unit with no interference.
https://gis.stackexchange.com/questions/8650/measuring-accuracy-of-latitude-and-longitude
There was a problem hiding this comment.
My testing revealed, that when zoomed out to approximately 0 the precision can be around two decimal points. For the test not to be flaky, I'm just checking if the point is in a general position of the forced location, as they are mostly being moved from the infinity anyway, leaving the responsibility of precise rendering to the Maps SDK.
| plugin.isLocationLayerEnabled = false | ||
| uiController.loopMainThreadForAtLeast(MAP_RENDER_DELAY) | ||
| assertThat(mapboxMap.queryLocationSourceFeatures(LOCATION_SOURCE).isEmpty(), `is`(true)) | ||
| assertThat(mapboxMap.querySourceFeatures(LOCATION_SOURCE).isEmpty(), `is`(true)) |
There was a problem hiding this comment.
is this equivalent to assertTrue?
danesfeder
left a comment
danesfeder
left a comment
There was a problem hiding this comment.
This looks good to me after remaining comment from @tobrun are addressed.
Per chat with @LukasPaczos, this PR introduces booleans for both the Android lifecycle as well as the map lifecycle 👍
LukasPaczos
force-pushed
the
558-native-crash-on-style-reload
branch
from
4bad0d6 to
d11b44f
Compare
3 participants
Fixes #558.
This PR adds a bunch of stress tests that are trying to push as much data as possible while we are reloading the style. Those tests caught a bunch of issues, which are handled with additional state flags and by always using core references to sources and layers instead of local ones.
Let me know if the documentation around state flags is clear enough.
/cc @tobrun for eyes at espresso + map testing