Issue 15603: normative PIM is expressed as a MagicDraw file
Issue 15604: The Normative References section should be in part 2
Issue 15605: Compliance Section issue
Issue 15606: There is a Model file for the Java PSM but not the C++ PSM as claimed in the Manifest.
Issue 15607: The machine readable files include redundant and/or useless files
Issue 15792: GeoPosition vs. GeodeticPosition
Issue 15793: Dealing with error at PIM level
Issue 15794: Conformance Criteria
Issue 15795: Ref. to 6.3.2
Issue 15796: Stanard - editorial
Issue 15797: addSelectionListener
Issue 15798: SelectionType
Issue 15799: Selection by track number
Issue 15800: Setting the categories of selectable items
Issue 15801: getEntityTypes
Issue 15802: Consistent centring with a moving entity
Issue 15803: Sample C++ header files
Issue 17000: SelectionMethodology not implemented
Issue 17001: Issue: SelectionEvent takes a single Entity parameter
Issue 17002: SelectionManager selection methods take a single Entity parameter
Issue 17003: Viewport scaleToPoints takes a single GeodeticPosition parameter
Issue 17004: Add an Angle Class to the Standard
Issue 17005: Add a Distance Class to the Standard
Issue 17006: Add an EntityRepository Concept / Class to the Standard
Issue 17007: Modify the comments in ContainQuery
Issue 17008: Add method to Entity interface to “getReferencePoint
Issue 17009: Remove “plural” designation of method call in EntityTypeQuery
Issue 17010: Fix Comment in IntersectionQuery
Issue 17011: Add comments to QueryManager
Issue 17012: Revise comments in code/model for clarity in TacsitController.getEntityTypes
Issue 17013: Revise comments in code/model for clarity in TacsitController
Issue 17014: Add comments to ViewEye
Issue 17015: Add clarification / comment to ViewportManager.getViewports method
Issue 17050: SelectionMethodology not implemented
Issue 17051: SelectionEvent takes a single Entity parameter
Issue 17052: SelectionManager selection methods take a single Entity parameter
Issue 17053: Add a Distance Class to the Standard
Issue 17054: Add method to Entity interface to “getReferencePoint”
Issue 17055: Modify the comments in ContainQuery
Issue 17056: Remove “plural” designation of method call in EntityTypeQuery
Issue 17057: Fix Comment in IntersectionQuery
Issue 17058: Add comments to QueryManager
Issue 17059: Revise comments in code/model for clarity in TacsitController.getEntityTypes
Issue 17060: Revise comments in code/model for clarity in TacsitController
Issue 17061: Add comments to ViewEye
Issue 17062: Add clarification / comment to ViewportManager.getViewports method
Issue 17240: The GeodeticPosition is an interface, it would be appropriate if wildcards were used.
Issue 17454: Section 7.1.1.1.3 "isPointEntity": the table is written twice in the text
Issue 15603: normative PIM is expressed as a MagicDraw file (tacsit-ftf)
Click here for this issue's archive.
Source: Microsoft (Mr. Steve Cook, stcook(at)microsoft.com)
Nature: Uncategorized Issue
Severity:
Summary:
In the AB today, when the TACSIT submission was presented, I noted that the normative PIM is expressed as a MagicDraw file, not a pure OMG-compliant XMI file. The normative PIM should be an OMG-compliant file and the MagicDraw file should be a convenience artifact.
The Normative References section should be in part 2. However the current reference is not Normative and so the section should be blank
The Compliance section should state how software complies e.g. it might say that for the Java PSM that software will implement all the normative APIs and exhibit the behavior for each as defined in the specification.
There is a Model file for the Java PSM but not the C++ PSM as claimed in the Manifest.
The machine readable files include redundant and/or useless files – the .mdr and mdr.bak files are useless and the mdzip is not needed as well as the .xml file since one is a compressed version of the other.
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 ------------------------------- Chapter/Section: all Title: GeoPosition vs. GeodeticPosition Nature: Revision Severity: Significant Two classes seems to speak of the same stuff: GeoPosition (see Section 2.3.1.3) and GeodeticPosition. My guess is that GeoPosition should be renamed ad GeodeticPosition. Please fix or explain.
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 Chapter/Section: all Title: Dealing with error at PIM level Nature: Revision Severity: Significant Error should be dealt with at the PIM level for each method.
Chapter/Section: 2 Title: Conformance Criteria Nature: Revision Severity: Significant Please add "completely" between "match" and "one or more of".
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 Page: 7 Title: Ref. to 6.3.2 Nature: Clarification Severity: Minor Broken reference to section 6.3.2.
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 ------------------------------- Page: 7 Title: Stanard Nature: Clarification Severity: Minor "stanard" instead of "standard".
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 ------------------------------- Chapter/Section: 7.1 Title: addSelectionListener Nature: Enhancement Severity: Significant This API seems to me very poor. Wouldn't it be possible to add some convenience APIs allowing a listener on the primary selection, the secondary selection, on setting or unsetting on of these selection... This would lower the number of server-client exchanges for nothing but test that this not something of interest.
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 Chapter/Section: 7.1 Title: SelectionType Nature: Clarification Severity: Minor It is not clear what "OTHER" is for.
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 Chapter/Section: 7.1 Title: Selection by track number Nature: Enhancement Severity: Significant The ways to select by track number as well as to select within geographic geometry seem to me inefficient. Indeed, in that way there are 2 accesses to the TACSIT server(s): one to query on one to effectively select when an interface proposing to select from a an interface with a query as parameter would be more efficient since it implied just one access call to the TACSIT server.
TACSIT Controller Interface for CMS Systems Version 1.0 - Beta 1 Date: October 2010 dtc/10-10-11 Chapter/Section: 7.1 Title: Setting the categories of selectable items Nature: Enhancement Severity: Significant I didn't find any interface allowing to set the categories of selectable items.
"getEntityTypes" is misnamed because it doesn't get all the entity types, hence a name such as "getSelectableEntityTypes" would be more explicit.
The proposed way to do the consistent centring with a moving entity seems inefficient and will imply unsmooth multiple movements of the viewport. A specific interface allowing such a slaved centring with an entity would be, in my point of view, by far better than this.
Chapter/Section: 9 Title: Sample C++ header files Nature: Enhancement Severity: Significant Please replace the "sample" before "C++ header files" with "normative".
This may be a translation error from the platform specific model, but this must be implemented. It would best be implemented as an enumeration.
A translation error occurred from the java PSM to the specification source files. This should be a Collection or List of Entity objects, rather than a single Entity. The affected methods are: • Constructor • getEntities To ensure the integrity of the event object, the internally stored Collection or List should be unmodifiable. This can be easily achieved by copying the List and invoking Collections.unmodifiableList(source).
A translation error occurred from the java PSM to the specification source files. This should be a Collection or List of Entity objects, rather than a single entity. The affected methods are: • setSelection • addToSelection • removeFromSelection
A translation error occurred from the java PSM to the specification source files. scaleToPoints takes only a single GeodeticPosition, whereas it should take a List of GeodeticPositions. In addition, since GeodeticPosition is an interface, it would be appropriate if wildcards were used. The suggested fix is as follows: void scaleToPoints(List<? Extends GeodeticPosition> points, double margin);
Having an Angle class would be very useful. This would free you from potential errors about worrying whether a double represents degrees or radians. This worry is particularly troublesome, since interface components present angles as degrees, whereas processing is most frequently done in radians. When dealing with unstructured primitive values, it’s an easy mistake to make. An Angle class would allow both to operate in whatever unit was most convenient by providing getters for each unit type. In addition to unit safety checking, it also makes property editors (for example, for tables and forms) far simpler to construct. The simplest way to specify an editor or renderer is to set the default cell editor or renderer. This facilitates minimal coupling between UI implementations and the data being presented. If the data is stored and presented as doubles, this approach is not possible because an editor that works for a unit of distance is probably not appropriate to also work for a unit of angle. Since GeodeticPosition contains both, this problem is likely to come up very frequently. The problem can be worked around by explicitly setting a particular column’s editor or renderer, but this strongly couples the display capability to the ordering of the displayed TableModel. A sample implementation of Angle is provided in the reference implementation. If Angle is added, the following properties should be changed to an Angle: • Rectangle o Orientation property • Geodetic Position o Latitude property o Longitude property • ViewEyeProperties o Orientation property AGREED
Using this as an object rather than a plain double value is a cheap way to avoid costly errors. NASA lost a $125 million Mars orbiter because one team was using imperial units instead of metric. Representing distance as an object makes it glaringly obvious in the code which calculations are using which units. Consider the following method of Rectangle: double getHeight(); Embedded in the comments of the interface contract is that “getHeight()” returns a double in meters. But that’s not at all obvious in client code that uses getHeight(), especially if it’s 2 or 3 levels removed from the place it is called. Instead, if it were changed to: Distance getHeight(); Distance could have methods “double getMeters()”, and “double getFeet()”, that make it immediately obvious what the returned value represents. If adopted, the following changes should be made: • GeodeticPosition o Change altitude property from double to Distance • Rectangle o Change getHeight() to return Distance o Change getWidth() to return Distance • Circle o Change getRadius() to return Distance • ViewEyeProperties o Change get/setRangeScale to use Distance • Viewport o Change scaleToPoints(points, margin) margin parameter to Distance AGREED
The definition of how Entity objects are related to and move around through a TacsitController, ViewportManager, and Viewport seems to be vague by design. Presumably, vendors have different implementations that make creating a cohesive abstraction difficult. It is a worthy goal, however – if it can be achieved. Separating the subsystems into more clearly defined subcomponents (Track providing, and track viewing) promotes encapsulation and division of responsibility. It also allows simplified compliance to the Tacsit specification for implementations which really provide only a single subsystem.
Consider NASA’s Worldwind. It is a general geographic map package. It provides little facility for getting track information from remote sources. The entire entity store concept is foreign to Worldwind; the reference implementation is purely incidental. Worldwind could work with any entity store, provided it had a well defined interface.
The reference implementation contains a sample of what an EntityRepository would look like. The definition is given here (sans comments):
public interface EntityRepository<E extends Entity> extends QueryManager
{
public Iterator<E> getEntities();
public void addRepositoryListener(RepositoryListener listener);
public void removeRepositoryListener(RepositoryListener listener);
}
Repository Listener would be defined as follows:
public interface RepositoryListener
{
public void entitiesAdded(RepositoryChangeEvent event);
public void entitiesRemoved(RepositoryChangeEvent event);
public void entitiesUpdated(RepositoryChangeEvent event);
public void entitiesCleared(RepositoryChangeEvent event);
}
This interface description should be general enough to allow for abstraction of existing implementations.
An area of common concern for this type of implementation is update speed. If data is being imported at a very fast speed, the user interface may not be able to handle the volume of updates. For instance, consider the common scenario that each update received is an array of structs containing track data that contains all the values for the track in the system. If a repository were to fire an “entitiesUpdated” event every time a property was copied into the entity, that would potentially be many new objects created:
O = (number of objects created to fire the event) *(number of properties set) * (number of tracks)
Each track will probably have 10 properties, 2 objects will be needed, and there may be roughly 2000 tracks in the system. That’s 40,000 objects every time a new track block comes in – excluding anything that happens once it gets to the UI.
However, this problem can be alleviated by deviating slightly from the standard Listener implementation. Instead of broadcasting to a RepositoryListener any time a track is changed, the EntityRepository can poll its Entity objects for changes based on an update rate. It would then fire a summary of those changes to its associated listeners, effectively coalescing all the redundant events into a single one. This drastically cuts down on the number of objects created to:
O = (number of objects created to fire the event)
Each entity will need to be “polled” every update, but the polling consists simply of checking a time value against a known value. In the example above, 2000 lastModified values would have to be compared.
This allows the update interval to be easily scaled to the fastest possible rate that still gives the desired responsiveness in the user interface. The key attribute is this: from the perspective of the client of the entity repository interface, it behaves exactly as per a “normal” Listening pattern. The complexity is masked by the interface and doesn’t creep into client code.
An example of how such an implementation would work is included in the reference implementation, PolledEntityRepository.
FUTURE
The comments of this class reference state this: Containment is determined by using the Entity's Geometry as an argument to the contains() method on the Geometry specified by this Query, i.e., this.satisfies( entity ) = this.getGeometry().contains( entity.getGeometry() ) This is not possible, however. The entity class has no “Geometry” in its interface. If it were present, the Geometry interface does not provide enough information to evaluate such a query. Suggested Resolution: Change the comment to a more general description of the problem space. For example: Containment is determined by checking whether or not one entity is wholly contained within another entity. AGREED
There is no way to get even a general understanding about where an Entity exists at a general interface level. Suggested Resolution: Define a new method in the entity interface: GeodeticPosition getReferencePoint(); This is logically consistent across any entity type. An entity could still consist of multiple points (e.g., isPointEntity() would return false), and still return a center point. This keeps the conceptual integrity of the entity interface, but expands its utility. One such concrete example of the utility: Creating a user interface component for scaling a Viewport to a set of points. The most common use case is for those points to be entity positions. The QueryManager can be used to retrieve a list of entities, but from that point on you have to dive into implementation specific details to convert the Entities to a position to choose from. AGREED
Interface specification is pluralized. This implies that multiple EntityTypes are returned, however, the return value is singular. EntityType getEntityTypes( ); Suggested Resolution: Singularize getEntityTypes() to getEntityType(). AGREED
The intersection query has a comment that states: Intersection is determined by using the Entity's Geometry as an argument to the intersects() method on the Geometry specified by this Query, i.e., this.satisfies( entity ) = this.getGeometry().intersects( entity.getGeometry() ) This is not possible, however. The entity class has no “Geometry” in its interface. If it were present, the Geometry interface does not provide enough information to evaluate such a query. Suggested Resolution: Change the comment to a more general description of the problem space. For example: Intersection is determined by checking whether or not one entity partially overlaps another entity.
It is unclear whether or not a null query is permissible. Suggested Resolution: Explicitly state in the interface documentation that a null query should return all Entities in the QueryManager. AGREED
The comment for this method reads:
/**
* Return the Entity Types which are supported by the TACSIT. This will
* return a list of all Entity Types currently available for Selection and
* Query by this TACSITController
*
* @return
*/
The first sentence gives the impression that it will return anything that can possibly be in the TacsitController. The second sentence implies only entities that currently exist in the TacsitController.
Suggested resolution:
It seems likely that the first sentence is correct; the wording of the second sentence should be changed to “This will return a list of all Entity Types available for Selection and Query by this TACSITController”.
AGREED
It is not clear from the comments that the collections returned by getProjections() and getEntityTypes() cannot be changed. Suggested resolution: The comments should say the collections returned by getProjections and getEntityTYpes() are unmodifiable.
The comments of the member variables are very descriptive. However, these comments should appear in the Javadoc for the get and set methods for each property. This provides much better integration with IDEs – most especially important for remembering whether the values are in degrees or radians. Suggested Resolution: Copy the comments for member variables ViewEyeProperties to their respective get/set methods.
The method getViewports() returns a Collection of Viewports. A user of this class may think modifying the contents of the Collection would modify the state of the ViewportManager. Suggested Resolution: Explicitly state (and enforce) that the returned Collection is unmodifiable.
This may be a translation error from the platform specific model, but this must be implemented. It would best be implemented as an enumeration. Resolution: Nothing to resolve. Already addressed in specification. Discussion: This is already addressed in paragraph 7.1.1.16 with the Enumeration Class called SelectionMethodology. Its two valid values are “ViewportDependent” and “ViewportIndependent”
A translation error occurred from the java PSM to the specification source files. This should be a Collection or List of Entity objects, rather than a single Entity. The affected methods are: • Constructor • getEntities To ensure the integrity of the event object, the internally stored Collection or List should be unmodifiable. This can be easily achieved by copying the List and invoking Collections.unmodifiableList(source). Resolution: Update the Java PSM class methods to take/return a Collection<Entity> respectively. Revised Text: There is no modification necessary to the text of the specification. Java PSM files only.
A translation error occurred from the java PSM to the specification source files. This should be a Collection or List of Entity objects, rather than a single entity. The affected methods are: • setSelection • addToSelection • removeFromSelection Resolution: Update the Java PSM class methods to take a Collection<Entity>. Revised Text: There is no modification necessary to the text of the specification. Java PSM files only.
Using this as an object rather than a plain double value is a cheap way to avoid costly errors. NASA lost a $125 million Mars orbiter because one team was using imperial units instead of metric. Representing distance as an object makes it glaringly obvious in the code which calculations are using which units. Consider the following method of Rectangle: double getHeight(); Embedded in the comments of the interface contract is that “getHeight()” returns a double in meters. But that’s not at all obvious in client code that uses getHeight(), especially if it’s 2 or 3 levels removed from the place it is called. Instead, if it were changed to: Distance getHeight(); Distance could have methods “double getMeters()”, and “double getFeet()”, that make it immediately obvious what the returned value represents. If adopted, the following changes should be made: • GeodeticPosition o Change altitude property from double to Distance • Rectangle o Change getHeight() to return Distance o Change getWidth() to return Distance • Circle o Change getRadius() to return Distance • ViewEyeProperties o Change get/setRangeScale to use Distance • Viewport o Change scaleToPoints(points, margin) margin parameter to Distance Resolution: A Distance class has been added to the specification and referenced in the places suggested. Revised Text: STILL NEED TO IMPLEMENT THIS IN THE SPEC.
There is no way to get even a general understanding about where an Entity exists at a general interface level. Resolution: Define a new method in the entity interface: GeodeticPosition getReferencePoint(); This is logically consistent across any entity type. An entity could still consist of multiple points (e.g., isPointEntity() would return false), and still return a center point. This keeps the conceptual integrity of the entity interface, but expands its utility. One such concrete example of the utility: Creating a user interface component for scaling a Viewport to a set of points. The most common use case is for those points to be entity positions. The QueryManager can be used to retrieve a list of entities, but from that point on you have to dive into implementation specific details to convert the Entities to a position to choose from. Revised Text: Part of this resolution is the editorial fix to clean up the “extra” tables in 7.1.1.1.3. Update the model to reflect the new method in the Entity interface to add getReferencePoint() with a return type of GeodeticPosition. Add a new section to the specification and update the graphic in paragraph 7.1.1.1 on figure 7.11to reflect the new method. 7.1.1.1.5 getReferencePoint Returns the GeodeticPosition to be used as the main point of reference for this entity. Type GeodeticPosition Visibility public Is Abstract false Parameter
Severity: Support Text Summary: The comments of this class reference state this: Containment is determined by using the Entity's Geometry as an argument to the contains() method on the Geometry specified by this Query, i.e., this.satisfies( entity ) = this.getGeometry().contains( entity.getGeometry() ) This is not possible, however. The entity class has no “Geometry” in its interface. If it were present, the Geometry interface does not provide enough information to evaluate such a query. Resolution: The Geometry class “contains” method accepts a GeodeticPosition, not a Geometry. By adding the “getReferencePoint” method to entity (as proposed by issue XXXX) and modifying the comment on this method Change the comment to a more accurate description of how to use it. For example: Containment is determined by checking whether or not an entity’s reference point (GeodeticPosition) is contained within a specific Geometry, i.e. this.satisfies(entity) = this.getGeometry().contains(entity.getReferencePoint()). Revised Text: Section 7.2.1.1 changed the description / comment for the method to “Containment is determined by checking whether or not an entity’s reference point (GeodeticPosition) is contained within a specific Geometry, i.e. this.satisfies(entity) = this.getGeometry().contains(entity.getReferencePoint()).”
Summary: Interface specification is pluralized and the figure on 7.2.1.3 shows a return of multiple EntityType objects. This implies that multiple EntityTypes are returned, however, the return value specified in the table and the Java PSM is singular. EntityType getEntityTypes( ); Resolution / Discussion: Change the return type in the table to match the model and update the Java PSM. The Java PSM will return a Collection<EntityType> instead of a single EntityType object. Revised Text: In Paragraph 7.2.1.3.1, modify the table to: 7.2.1.3.1 getEntityTypes Returns the list of EntityType objects to compare with. Type EntityType [1..n] Visibility public IsAbstract false Parameter
Summary: The intersection query has a comment that states: Intersection is determined by using the Entity's Geometry as an argument to the intersects() method on the Geometry specified by this Query, i.e., this.satisfies( entity ) = this.getGeometry().intersects( entity.getGeometry() ) This is not possible, however. The entity class has no “Geometry” in its interface. If it were present, the Geometry interface does not provide enough information to evaluate such a query. Resolution / Discussion: Change the comment to a more general description of the problem space. For example: Intersection is determined by checking whether or not one entity partially overlaps another entity. Revised Text: Changed text in 7.2.1.5 to read: The IntersectionQuery is used to determine if an Entity intersects geometrically with a Geometry. Intersection is determined by checking whether or not one entity partially overlaps another entity
It is unclear whether or not a null query is permissible. Resolution / Discussion: Explicitly state in the interface documentation that a null query should return all Entities in the QueryManager. Revised Text: Add sentence to 7.2.1.6 “In the case where a null query is passed to the QueryManager, the QueryManager is to return all Entities in the QueryManager.”
The comment for this method reads:
/**
* Return the Entity Types which are supported by the TACSIT. This will
* return a list of all Entity Types currently available for Selection and
* Query by this TACSITController
*
* @return
*/
The first sentence gives the impression that it will return anything that can possibly be in the TacsitController. The second sentence implies only entities that currently exist in the TacsitController.
Resolution:
It seems likely that the first sentence is correct; the wording of the second sentence should be changed to “This will return a list of all Entity Types available for Selection and Query by this TACSITController”.
Revised Text:
Change the comments in paragraph 7.1.1.8.2 and the model to read:
Returns the Entity Types that are supported by the TAC SIT. This will return a list of all Entity Types available for Selection and Query by this TACSITController.
It is not clear from the comments that the collections returned by getProjections() and getEntityTypes() cannot be changed. Resolution / Discussion: The comments should say the collections returned by getProjections and getEntityTypes() are unmodifiable. Revised Text: Add text to paragraphs 7.1.1.8.1 and 7.1.1.8.8. Attempted changes to the collection returned by this method will have no effect on the behavior and functionality of the TacsitController.
The comments of the member variables are very descriptive. However, these comments should appear in the Javadoc for the get and set methods for each property. This provides much better integration with IDEs – most especially important for remembering whether the values are in degrees or radians. Resolution / Discussion: Copy the comments for member variables ViewEyeProperties to their respective get/set methods. Revised Text: Update the PIM model and Java PSM to reflect comments in section 7.1.1.9. There is no change needed to the specification document.
The method getViewports() returns a Collection of Viewports. A user of this class may think modifying the contents of the Collection would modify the state of the ViewportManager. Resolution / Discussion: Explicitly state (and enforce) that the returned Collection is unmodifiable. Revised Text: Add sentence to section 7.1.1.13.4 stating: The collection returned by this method is unmodifiable
The GeodeticPosition is an interface, it would be appropriate if wildcards were used. The suggested fix is as follows: void scaleToPoints(List<? Extends GeodeticPosition> points, double margin);
Section 7.1.1.1.3 "isPointEntity": the table is written twice in the text