stadiamaps / ferrostar

A FOSS navigation SDK built from the ground up for the future
https://stadiamaps.github.io/ferrostar/
Other
180 stars 25 forks source link

Convert MaxSpeed to consistent JSON format (a float of meters per second) #272

Closed Archdoog closed 4 weeks ago

Archdoog commented 2 months ago

The MaxSpeed json object has two formats:

  1. {"unknown": true} - The speed limit is unknown
  2. {"speed":30.2,"unit":"km/h"} - The speed is unknown and potentially the localized unit (depending on API).

This is an annoying object to deserialize on some platforms where json serialization requires typed references often requiring a custom decode handler. Because of this, it's probably best that we fix this once in rust to standardize the type definition. Suggestion is probably:

/// Speed limit in meters per second. None if unknown.
speed_limit: Option<f64>
// TODO: This is a bonus as the platforms will be able to convert to KPH, MPH, etc.
// Some apps may prefer localized units from a route engine that supports it, device locale,
// or even manual user preference.
local_speed_limit_unit: Option<String> 

This is a much easier thing to deserialize as it always has the same type definition on a platform.

ianthetechie commented 2 months ago

Why can we just do enum with two variants where one variant has two required fields (and the unit is an enum of course) and the other has none?

enum SpeedLimit {
    Unlimited,
    Unknown,
    Posted {
        speed: f32,
        unit: SpeedUnit
    }
}

And for platform code that wants it in a standard unit, we can have methods on the enum impl. We can't expose these in UniFFI directly but we can expose top level functions for convenience.

This should keep the data model lossless while letting us centralize conversion logic where it's needed for some purpose other than display.

Archdoog commented 2 months ago

We can absolutely just provide the custom deserializers on each platform required to handle the untouched MaxSpeed from the backend. I'm all for that, as long as developers can just include a MaxSpeed model as a child in any custom attributes model definition.

On Swift we can provide a custom MaxSpeed: Codable with its specialized Decoder and encode methods assuming https://www.swiftbysundell.com/articles/codable-synthesis-for-swift-enums/ doesn't provide any useful automatic solutions. Will have to research the same on Kotlin.

My main goal is avoiding friction when you define your slightly custom Attributes model on whatever platform. Enum with value patterns are not commonly very nice with serialization tools. Rust serde is the exception where it works perfectly out of the box.

ianthetechie commented 2 months ago

Yeah, understood. I think we can do that with the UniFFI magic in the toml file we were chatting about earlier. I'd be happy to put in some work next week to make that happen if you can work up some rough code that you'd like to see compilable. (i.e. draft of something that probably doesn't work but that you'd like to just magically work)

I think we're getting close on this :D

ianthetechie commented 2 months ago

I also see you had implemented some of the ideas already in your PR now :P Still working through that...

Archdoog commented 4 weeks ago

Think this can be dropped as a result of #287. Android may be a little harder, but the benefits of this system seem to be solid enough as demonstrated on the iOS ticket.