Refactor reference frame module #135
Conversation
| Transformation { | ||
| from: ReferenceFrame::ITRF2014, | ||
| to: ReferenceFrame::ETRF2014, | ||
| params: TimeDependentHelmertParams { | ||
| t: Vector3::new(0.0, 0.0, 0.0), | ||
| t_dot: Vector3::new(0.0, 0.0, 0.0), | ||
| s: 0.0, | ||
| s_dot: 0.0, | ||
| r: Vector3::new(0.0, 0.0, 0.0), | ||
| r_dot: Vector3::new(0.085, 0.531, -0.770), | ||
| epoch: 1989.0, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
This was a duplicate, and only caught it when storing everything in a hashmap
| #[must_use] | ||
| pub const fn zeros() -> TimeDependentHelmertParams { | ||
| TimeDependentHelmertParams { | ||
| t: Vector3::new(0.0, 0.0, 0.0), | ||
| t_dot: Vector3::new(0.0, 0.0, 0.0), | ||
| s: 0.0, | ||
| s_dot: 0.0, | ||
| r: Vector3::new(0.0, 0.0, 0.0), | ||
| r_dot: Vector3::new(0.0, 0.0, 0.0), | ||
| epoch: 0.0, | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there any reason to not just make this the default? This seems like a reasonable default but I am not entirely sure that it makes sense to be either. If you agree it might be good to do a default impl on this struct
There was a problem hiding this comment.
Two reasons why this function exists.
- Having a
constfunction to initialize some of the transformations in the built-in list helps reduce duplication. - An
epochof 0.0 is almost certainly an incorrect value, which doesn't seem like a good thing for a Default value.
I do implement Default on this type where I initialize the epoch to something more reasonable in the context of geodetic reference frames. I guess we could set epoch to something other than zero here, but then the name zeros seems misleading. Not really sure what option satisfies the principle of least surprise here. I could just remove the pub from this function to avoid confusion with users of the crate?
One last refactor for usability. This PR contains two primary changes, along with a scattering of clean ups and documentation re-writes.
Support for handling new reference frames
Previously the
ReferenceFrameenum contained a set list of known reference frames, and we made sure to maintain an internal list of transformations for that set list of reference frames. This approach is obviously pretty inflexible, so I've added a new variant toReferenceFramecalledOtherto enable defining new reference frames at run-time. AStringis used to represent the unknown reference frame types. This slots in easily with our use of thestrummacros for converting the reference frame enums into or from strings, but this does make theReferenceFrametype no longer triviably copy-able.I've attempted to reduce the number of clones of the new
ReferenceFrame, but they can't be completely avoided without a significantly different approach of representing new reference frame types at run-time. I'd be happy to hear suggestions on alternative approaches if anyone has any.Simplify managing reference frame transformations
Previously it was expected that the user of the crate would get the list of hard-coded transformations with the
get_transformationfunction. If you needed a single transformation that would suffice, but if we didn't have a direct transformation hard-coded for you then you'd need to perform multiple transformations. TheTransformationGraphstruct was available to help to figure out the shortest path of transformations from one reference frame to another, but theTransformationGraphobject didn't actually handle the transformations for you. In short it was a bit of a pain to use the crate for simple transformations and a major pain to do more complicated transformations.To improve the situation I've replaced the old
TransformationGraphtype with a newTransformationRepositorytype which both maintains the graph of transformations and also applies multiple transformations for you in sequence when needed. You're able to easily get one pre-populated with the hard-coded transformations available, or an empty one, then add any additional transformations defined at run-time to the graph.