Open ckwichael opened 1 year ago
It is really helpful to have your review!
Thanks again for taking the time to interpret thie document and point out errors and inconsistencies.
[ ] Make it an array of FrameTransformIndexPair s.
[ ] Make the referents of the indices explicit in the UML.
[ ] Scan of the whole document to unify usage of FrameListTransformPair vs FrameTransformIndexPair - similar other occurrences should use the FrameTransform... terminology.
ACTION: Document this and any other bugs found in an FAQ or the developers' guide (before summer release of corrigendum 1)
I apologize since this issue is somewhat pedantic, but I found some ambiguities in the
TransformList
property of theGraphSdu
.GraphSdu
the type of thetransformList
isFrameTransformIndexPair
, should this be an array ofFrameTransformIndexPair
? I believe that many of the graphs will generally have at least n-1 pairs where n is the number of nodes in the graph which would indicate that we need an array of pairs.FrameTransformIndexPair
datatype as having two members:outerFrameIndex
andinnerFrameIndex
. However in the json schema in section 9.2.5 thetransformList
is describes as having a typeFrameTransformPair
rather thanFrameTransformIndexPair
and this type seems to have an array oflink
objects that simply contain an array. I can intuit that the first value is likely theouterFrameIndex
and the second value is theinnerFrameIndex
but that is not explicitly stated in any requirement that I can find. Since this is a directed graph it seems that it is very important to be explicit in which index is which. My personal preference is the data type specified in the UML, that is, theFrameTransformIndexPair
which has two explicit members. It removes any ambiguity and, in my opinion, makes it easier to work with.transformList
asFrameListTransformPair
orFrameTransformIndexPair
. It is unclear whether the intent was that theFrameListTransformPair
was simply a list ofFrameTransformIndexPair
, a typo, or a remnant from and older version of the spec.I think that I'm able to intuit the spirit of the SDU, but someone else may interpret one of these requirements slightly differently or make a different assumption about the way the data should look which could damage interoperability between Geopose users. I think it would be beneficial to ensure that all of the requirements and datatypes are explicit and consistent.