-
Notifications
You must be signed in to change notification settings - Fork 15
Typescript generation #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| * of a and b for a different ellipsoid. Adapted from Military Handbook 600008 | ||
| * @param latLonAlt {lat: lon: alt:} in degrees and meters | ||
| * @return {x: y: z:} in meters | ||
| * @param {LatLonAlt} latLonAlt in degrees and meters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi again @B1Dobbs I was using your version of this project locally since this PR hasn't been merged and noticed there is a slight issue here with the conversion types. Seems as though the params types for the getXYZfromLatLonAltDegrees are for some reason expecting shorthanded { lat: number; lon: number; alt: number } instead of the fully written out words... weird! Also the returned value isn't an instance of Vector3Float its just raw values.
Same goes for the return value for convertDisToLatLongInDegrees it isn't an instance of the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jfelicianiats, we personally weren't using this supporting class so I could have made a mistake here. I'll take a look!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jfelicianiats I see now that LatLonAlt is used differently across the functions in this class. I hadn't noticed that previously!
convertDisToLatLongInDegreesreturns the variables spelled outgetXYZfromLatLonAltDegreesexpects a param with the shorted names
I'll have to create two separate types to handle this case unfortunately. The point of this PR is to create types for the existing code (warts and all) to ensure it's works exactly the same way with no breaking changes.
As for your second concern, the original code doesn't use the prototype function for Vector3Float and simply returns an object that looks like it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@B1Dobbs Awesome thanks! The reason I was mentioning the Vector3Float was because since it isn't a class instance, it fails to serialize properly if you put the result directly on a PDU for example. (Its missing some member functions since its not a class instance and instead just a plain object)
ie. encodeToBinary is not a function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend creating the class yourself with the results from the conversion and maybe open a separate issue to fix the original function.
Let me know if the updated typescript makes sense!
- Install tsd-jsdoc - Helper script for changing namespace to module - JSDoc for namespace and LatLonAlt - Types output
1c2c0e3 to
39e13ca
Compare
For issue #26.
src/disandsrc/disSupporting.types/types.d.ts.Let me know if there are any changes to be made! Unfortunately there is not an easy way to have types for the DIS 7 classes because it also uses the same
disnamespace.