sharedstreets / sharedstreets-js

SharedStreets (Node.js & Javascript)
https://sharedstreets.io
MIT License
78 stars 25 forks source link

Issues making the ReferenceID message #5

Closed DenisCarriere closed 6 years ago

DenisCarriere commented 6 years ago

@kpwebb Having a hard time defining the ReferenceID message.

Maybe I'm missing something obvious, here's the example that's failing for me.

Data example

https://github.com/sharedstreets/sharedstreets-pbf/blob/master/test/out/11-602-769.reference.json

{
  "id": "41d73e28819470745fa1f93dc46d82a9",
  "geometryId": "56e7b7dd3097e97d80b5eb15f662edbf",
  "formOfWay": "MultipleCarriageway",
  "locationReferences": [
    {
      "intersectionId": "69f13f881649cb21ee3b359730790bb9",
      "lon": -74.0048213,
      "lat": 40.7416415,
      "outboundBearing": 208,
      "distanceToNextRef": 9279
    },
    {
      "intersectionId": "f361178c33988ef9bfc8b51b7545c5fa",
      "lon": -74.0051265,
      "lat": 40.7408505,
      "inboundBearing": 188
    }
  ]
}

Java implementation

https://github.com/sharedstreets/sharedstreets-builder/blob/master/src/main/java/io/sharedstreets/data/SharedStreetsReference.java#L323-L340

    public static UniqueId generateId(SharedStreetsReference ssr) {
        String hashString = new String();

        hashString = "Reference " + ssr.formOfWay.value;

        for(SharedStreetsLocationReference lr : ssr.locationReferences) {
            hashString += String.format(" %.6f %.6f", lr.point.getX(), lr.point.getY());
            if(lr.outboundBearing != null) {
                hashString += String.format(" %d", Math.round(lr.outboundBearing));
                hashString += String.format(" %d", Math.round(lr.distanceToNextRef * 100)); // store in centimeter
            }
            if(lr.inboundBearing != null) {
                hashString += String.format(" %.1f", lr.inboundBearing);
            }
        }

        return UniqueId.generateHash(hashString);
    }

Javascript Implementation

export function referenceMessage (locationReferences: LocationReference[], formOfWay: FormOfWay): string {
  let message = `Reference ${formOfWay}`
  locationReferences.forEach(lr => {
    message += ` ${round(lr.lon)} ${round(lr.lat)}`
    if (lr.outboundBearing !== null && lr.outboundBearing !== undefined) {
      message += ` ${Math.floor(lr.outboundBearing)}`
      message += ` ${Math.floor(lr.distanceToNextRef * 100)}` // store in centimeter
    }
    if (lr.inboundBearing !== null && lr.inboundBearing !== undefined) {
      message += ` ${round(lr.inboundBearing, 1)}`
    }
  })
  return message
}

My Reference Message

Reference 2 -74.004821 40.741642 208 927900 -74.005127 40.740851 188.0

Message Breakdown

Message Part Description
Reference Start of Message
2 FormOfWay (this should be a number, correct?)
-74.004821 40.741642 208 Intersection 1 lonlats
208 outbound bearing
927900 distanceToNextRef (I don't think we should multiply x100, the input data should be in cm already)
-74.005127 40.740851 Intersection 2 lonlats
188.0 inboundBearing (why is outbound using " %d" and inbound is "%.1f"?)

Javascript Code using example

const locationReferenceOutbound = sharedstreets.locationReference([-74.0048213, 40.7416415], {outboundBearing: 208, distanceToNextRef: 9279})
const locationReferenceInbound = sharedstreets.locationReference([-74.0051265, 40.7408505], {inboundBearing: 188})
const formOfWay = 2; // => 'MultipleCarriageway'

sharedstreets.referenceId([locationReferenceOutbound, locationReferenceInbound], formOfWay)
// expected => '41d73e28819470745fa1f93dc46d82a9'
// actual => 'a3de812e1a291d5868ddabda0c2a17c4'

@kpwebb Could you let me know which part of the message is wrong & which parts are correct?

kpwebb commented 6 years ago

Ah, ok so just checked and here's the message string being used generate the hash:

Reference 2 -74.004821 40.741642 208 9279 -74.005127 40.740851 187.9

the problem was inboundBearing not being rounded to %d. Fixing java implementation

kpwebb commented 6 years ago

new java implementation -- let's drop the inbound reference entirely... i don't see how it adds value to the reference id and it isn't part of the original OpenLR spec.

       hashString = "Reference " + ssr.formOfWay.value;

        for(SharedStreetsLocationReference lr : ssr.locationReferences) {
            hashString += String.format(" %.6f %.6f", lr.point.getX(), lr.point.getY());
            if(lr.outboundBearing != null) {
                hashString += String.format(" %d", Math.round(lr.outboundBearing));
                hashString += String.format(" %d", Math.round(lr.distanceToNextRef * 100)); // store in centimeter
            }
        }
        UniqueId id = UniqueId.generateHash(hashString);
DenisCarriere commented 6 years ago

let's drop the inbound reference entirely

@kpwebb Should we also drop inboundBearing for the Protobuf schema? If we aren't ever planning on using it, is it worth storing that data?

kpwebb commented 6 years ago

Let's keep it in the protobuf -- folks are interested in this (was specifically requested by @migurski) and seems useful. I just don't think we should include it the ref id, as the ref should be unique based on other properties and this is just calculated as a convenience.

DenisCarriere commented 6 years ago

Ok! Sounds good to me!

Any chance you can update the *.reference.pbf sample data, that way I can make sure we are getting the same ReferenceID.

Anytime I build the Java Builder it throws some kind of error.. lol 😢

kpwebb commented 6 years ago

Done! New sample data is up and I'll push a new pre-built jar for Builder.

DenisCarriere commented 6 years ago

👍 Sweet!