mapbox-navigation-android icon indicating copy to clipboard operation
mapbox-navigation-android copied to clipboard

[Android 13] - Bundle#get is deprecated

Open dzinad opened this issue 3 years ago • 5 comments

We use this API to create FixLocation#extras. The logic is simple: there are multiple predefined types, we use bundle.get and check if the returned value is instance of one of the predefined types. If yes, create map entry with the following type, if no, ignore the entry. Code:

        val key = iterator.next()
        when (val value = this.get(key)) {
            is Boolean -> map[key] = Value(value)
            is Byte -> map[key] = Value(value.toLong())
            is Char -> map[key] = Value(value.toString())
            is Double -> map[key] = Value(value)
            is Float -> map[key] = Value(value.toDouble())
            is Int -> map[key] = Value(value.toLong())
            is Long -> map[key] = Value(value)
            is Short -> map[key] = Value(value.toLong())
            is String -> map[key] = Value(value)
            else -> {
                logW(
                    "Unsupported type in location extras",
                    LOG_CATEGORY
                )
            }
        }

Now Bundle#get is deprecated and there is no other way to determine which type has the value by a concrete key. For now we can leave it as-is because deprecation is not removal, but it's very probable that in the future we'll have to face this problem anyway. Questions:

  1. Where is FixLocation#extras used? We use it only to convert back to Location#extras. The whole object is passed to Navigator#updateLocation. Are the extras somehow used there? I see that iOS does not have any extras.
  2. Do we need all extras there or just some predefined ones? This way we can use type-safe methods like Bundle#getString if we know the key (and which type the value has).

cc @mapbox/navnative

dzinad avatar Aug 08 '22 16:08 dzinad

As I remember we used the extras for some experiments. For instance, somewhen we had R&Ds which based on assumptions about amount of satellites. This values are still used to high-level analysis of the signal quality. Not sure about other data types, I guess the method had been made as an universal way to provide any additional data into native part

0v3rt1r3d avatar Aug 10 '22 12:08 0v3rt1r3d

What if we reserve a special key for our extras (like "com.mapbox.navigation.extras") and store all of our arbitrary data there as an encoded json? For example, instead of a Bundle like this: Bundle().put("key1", "value1").put("key2", 22) we'd get a bundle like this: Bundle().put("com.mapbox.navigation.extras", "{\"key1\": \"value1\", \"key2\": 22}")

This way we don't depend on the types that Android SDK allows, we just need the string. But it would only work if:

  1. No keys are currently read from the extras on the native side;
  2. No system extras (or just a predefined set of them) is used;
  3. Whoever puts these values except for the system supports the changes on their side. BTW, who does that?

So the Nav SDK would only parse and proxy the following data:

  1. getString("com.mapbox.navigation.extras")
  2. maybe some other invocations that are predefined and of which we know the type. For example, getInt("satellites") docs.

@mapbox/navigation-android wdyt?

dzinad avatar Aug 18 '22 10:08 dzinad

key for our extras (like "com.mapbox.navigation.extras") and store all of our arbitrary data there as an encoded json?

Actually, we're not passing anything to NN via FixLocation, one of the reason to add extras was number of satellites that might be provided via Bundle#getExtras. Also, customer request (see navigation-sdks#1236) where custom data were provided via extra. I would ask @mapbox/navnative if this is still relevant and we need it at all. So these values are provided outside by vendors avoiding Nav SDK business logic.

RingerJK avatar Aug 18 '22 13:08 RingerJK

The extras we get are still relevant. This is a generic mechanism for external users to pass additional information with the locations (https://developer.android.com/reference/android/location/Location#getExtras()). The reason we use a generic bundle is that this is the interface in the Location. We don't work with the Bundle in the native code - we get a map of string -> generic value from the SDK, and we can make it more specific (like string -> string) and parse the values based on the key (assume, everything is a string).

mskurydin avatar Sep 13 '22 08:09 mskurydin

we get a map of string -> generic value from the SDK, and we can make it more specific (like string -> string) and parse the values based on the key (assume, everything is a string).

Could you please provide an example? I'm not sure what you mean. I was suggesting a single predefined key for an arbitrary dictionary (in json format): https://github.com/mapbox/mapbox-navigation-android/issues/6133#issuecomment-1219353067. We could parse it on the SDK side or pass to you as-is. The second option looks more flexible. Also, where do we have the docs on how to pass custom data via extras? It should be updated.

dzinad avatar Sep 13 '22 08:09 dzinad