Tracker Refactoring Comments
From IGSTK
Contents |
Comments TCON 11/29/2007
- Add a RequestSetReferenceTool(TrackerToolIdentifier) to Tracker API ( Andinet 15:15, 30 November 2007 (EST) Done )
- Add Request[Set/Get]CalibrationTransform(igstkTransform) to TrackerTool API ( Andinet 15:13, 30 November 2007 (EST) Done )
- Add RequestGetTransform'ToParent?'() to TrackerTool API ( Andinet 15:13, 30 November 2007 (EST) Done )
- When calibration transform is not set, this will return raw transform ( Andinet 15:13, 30 November 2007 (EST) Done )
- Add RequestGetTransofrmToReferenceTool() to TrackerTool API, this returns the transform w/r to another tracker tool
- All transforms are being composed inside Tracker class, it has internal map with pointer to attached TrackerTool class, to get the calibration transform ( Andinet 15:13, 30 November 2007 (EST) Done )
- Suggested using TrackerTool::Configure instead of ::Initialize ( Andinet 15:13, 30 November 2007 (EST) Done )
Base Class
Patrick's comments
- COM Port should should not be limited to COM1 ~ COM8, in case of using USB-To-Serial adapter, the COM port can go beyond that range. Andinet 11:54, 10 December 2007 (EST) I agree with you. We shouldn't be limited up to COM8. This can be fixed by reconfiguring the main CMake input file and modifying the SericalCommunication classes and related test programs.
- TrackerTool State Machine
- When calling RequestConfigure(), it will check an internal boolean flag m_TrackerToolConfigured. This flag is not being set properly. Andinet 09:55, 7 December 2007 (EST)This is a bug. I have added a fix. For 6DOF, once port number is specified, the tracker tool is marked to be "Configured" and hence it can beattached to the tracker. For 5DOF, channel number needs to be specified in addition to port number. For both tracker tool types, SROM file and tool ID can be specified. But these parameters/information are optional.
- SetInitialState and SetReadyToRun should be called after setting the transition table
- AuroraTrackerTool, TrackerTool classes [Fixed]
- There is no need for to have RequesetSetTransform, that is equivalent to RequestSetCalibrationTransform in TrackerTool class. Andinet 10:03, 7 December 2007 (EST) There are three transform values i.e raw transform, calibrated raw transform and calibrated raw transform with respect to a specified reference tracker tool. For this purpose, I have added Set/Get methods for RawTransform, CalibratedRawTransform, CalibratedRawTransformWithRespectToReferenceTrackerTool.
- RequestSetRawTransform should be called from Tracker friend class, and should be private Andinet 10:03, 7 December 2007 (EST) I agree. All the methods that set the different transforms are moved to the private section.
- Question? If Tracker is responsible for all transform composition, it then needs to return multiple transforms. or it will return a single calibrated reference frame transform? Andinet 10:11, 7 December 2007 (EST) The tracker returns the three transforms as mentioned above
- There is no RequestGetTransform and RequestGetTransformToReferenceTool methods. Andinet 15:28, 10 December 2007 (EST) I have added three transform accessing methods GetRawTransform(), GetCalibratedRawTransform(), and GetCalibratedRawTransformWithRespectToReferenceTool(). The GetToolTransform method has been removed from the tracker class.
- It is not clear how the raw transform is composed with calibration transform. Andinet 10:11, 7 December 2007 (EST) Take a look at igstkTrackerNew::UpdateStatusSuccesProcessing() method. Specifically, lines #785-797
- Testing program should fail when operation fails. Andinet 11:04, 10 December 2007 (EST) This can be done for tests with simulated data input. However, for real(manual) tests, user interaction can lead to operation failure. The information can be accessed by looking at the log file
- Tracker update --> data should be pushed into TrackerTool. Do we just need the Calibrated and Reference adjusted Transform? or do we need Raw or just calibrated Transform? Andinet 15:37, 10 December 2007 (EST) See explanation above.
- Use string for TrackerToolIdentifier is not optimal, and I don't see the constructor of this unique identifier. Do we leave it to the user to set this? coz there is a SetTrackerToolIdentifier method. Andinet 11:04, 10 December 2007 (EST) Tracker tool identifier is internally generated by the tracker tool class. Users are not required/permitted to specify an identifier. The way the identifiers are generated is tracker-specific. For example, marker names are used for MicronTrackers. Whereas string concatenation of port and channel number is used to identify 5DOF Aurora tracker tool
- Ideally, user only sees the TrackerTool and should be able to request for transform given a pointer to a TrackerTool. Andinet 11:08, 10 December 2007 (EST) This is now possible using GetRawTransform(), GetCalibratedRawTransform() and GetCalibratedRawTransformWRTReferenceTrackerTool() methods
- Why do we have a m_Tracker inside TrackerTool? Andinet 11:04, 10 December 2007 (EST) This is basically member data that contains pointer to the tracker to which the tracker tool is scheduled to be attached to. Adding this member variable is necessary to temporarily store the tracker pointer as request to attach to the tracker is processed by the state machine in the tracker tool class. I have renamed this member variable to "m_TrackerToAttachTo" to make it clearer.
- Detach and Attach of the Tool to Tracker... Andinet 11:04, 10 December 2007 (EST) There was a bug in the method that detaches tracker tool from the tracker. This has been fixed by Patrick
- No confirmation if attached physical tool is 5DOF or 6DOF. There might be a way to determine by querying the tracker hardware if it the attached tracker coil is 5DOF or 6DOF. I will need to look at NDI API documentation
- TrackerTool API
- RequestGetRawTransform
- RequestGetCalibratedRawTransform
- RequestGetTransformWithRespectToReference
Janne Beate's comments
- For those who don't know me: I work at Sintef with Frank and Torleif and I have been testing the tracker code. (Jbake)
- RequestAttachTool, RequestRemoveTool, SetToolTransform and GetToolTransform all take a tools unique identifier, but seeing these are public and users most likely already have a pointer to a trackertool available, it would be more intuitive to have them take a pointer to a trackertool like RequestSetReferenceTool instead. Andinet 08:02, 11 December 2007 (EST) Good point.. I will make the change...
Andinet 14:03, 11 December 2007 (EST) Fixed!
Frank's comments
- David and I looked at surgical coordinate definitions some time ago:
http://public.kitware.com/IGSTKWIKI/index.php/Surgical_coordinate_changes
- Using this as input and choosing an explicit naming convention the convenient TrackerTool API could look like this:
- RequestGetUncalibratedTransformWithRespectToWorldFrameCoordinates (alt. Global=World, WithRespectToTracker)
- RequestGetCalibratedTransformWithRespectToWorldFrameCoordinates
- RequestGetUncalibratedTransformWithRespectToReferenceFrameCoordinates
- RequestGetCalibratedTransformWithRespectToReferenceFrameCoordinates
- Or making them a bit shorter focusing on the differences:
- RequestGetUncalibratedTransformWRTWorldFC (alt. Global=World, WithRespectToTracker)
- RequestGetCalibratedTransformWRTWorldFC
- RequestGetUncalibratedTransformWRTReferenceFC
- RequestGetCalibratedTransformWRTReferenceFC
- If the last method uses the identity when a calibration transform is not set for the tool as well as for the reference tool transf. when a reference is not set it will BEHAVE like we anticipated (using a more general RequestGetTrasnform) in the unlikely real case that a calibration and/or reference is NOT used.
- But more impotently is the intended use of the Tracker/TrckerTool comp. together with other comp. in order to archive the most common IGS functionality: extracting a slice from a volume based on a tool POSE. I wrote some pseudo-code regarding this some time ago (before the refactoring, see the ImageResliceRep alg.):
http://public.kitware.com/IGSTKWIKI/index.php/TrackerToolExamples#Discussion
- Please comment, is it still relevant, what is the best way to archive this today?
Polaris
Ziv's additional Polaris refactoring proposal (5/2/08)
- Because the Polaris is not a single tracker but a family of trackers with a common API we need a finer class hierarchy. The polaris family is:
- Hybrid Polaris (old system) and Hybrid Spectra - wired and wireless tools, refresh rate of 60Hz.
- Passive Spectra - wireless tools, refresh rate of 60Hz.
- Vicra - wireless tools, refresh rate of 20Hz.
I propose to change to the following heirarchy:
All Polaris systems support wireless so we have an abstract base class PolarisWireless. The PolarisSpectra and PolarisVicra simply inherit from the base class and check for their respective refresh rates. The PolarisHybrid adds the support for wired tools.
We now also separate the wired and wireless tools, two simpler state machines than the current one.
Janne Beate's comments
- In VerifyTrackerToolInformation the port handle that is assigned through PHRQ is set to "1" (wireless) at line 227, but at this point you can be dealing with a wired tool with a SROMFile. Andinet 12:32, 6 February 2008 (EST) good catch. I have added if-else loop to take care of this. But it is not tested..I don't have a wired tracker tool with SROM File
- RemoveTrackerToolFromInternalDataContainers should take a pointer to the tool you want to remove instead of the trackerToolIdentifier. Getting a trackerToolIdentifier will require a trackertool anyway woun't it? Andinet 14:02, 11 December 2007 (EST) Fixed
- When calling reset from the TrackerState you will end up in CommunicationEstablishedState with default communication values, this means that you will have lost the values you set during the transition from IdleState to CommunicationEstablishedState. From a users point of view this is confusing, because one would expect to reset and still have the values you set with tracker->SetCommunication( serialComm ); (Same goes for Aurora) Andinet 20:27, 5 February 2008 (EST) Reset input is not designed to stop tracking but instead to reset the communication with the tracking hardware. This implies reseting communication parameters to its default values. The user shouldn't request reset input if she/he wants just to stop tracking.
Aurora
Janne Beate's comments
- Calling PDIS in AuroraTrackerNew::RemoveTrackerToolFromInternalDataContainers and then removing the port handle from all internal containers doesn't make sense. PDIS only stops the reporting of transforms, it never releases the porthandle (to do this one should call PHF instead). Because of this we cannot detach a tool and then attach it again, this will give a "Couldn't find uninitialized port"-error. It should be clear to the user if the tool is disabled (stopped reporting transforms) or if it is detached (porthandle released).
Micron
Janne Beate's comments
- We are using Micron tracker 2, model: BB-BW-S60 with MTC_R2.7 (MTC_R2.8 generates a bluescreen (bumblebe.sys -> DRIVER_LEFT_LOCKED_PAGES_IN_PROCESS) with the igstkMicronTrackerNewTest code). Andinet bluescreen? Can you give us more information about the problem? Have you run the demo program that comes with the toolkit? Jbake The demoprogram that comes with the toolkit works with the MTC_R2.8, but not the igstkMicronTrackerNewTest code. When I run the test it never ends properly, it just hangs after "[PASSED]". Ending it manually dosn't work either and after some time I get the bluescreen.
- In MicronTrackerNew::VerifyTrackerToolInformation idx=0, but the API for the micron tracker states that: Index1based, The index of the marker template. Must be in the range 1..count or the call would fail with a mtParameterOutOfRange error. (For Markers_TemplateItemGet(int ndex1based, mtHandle * p_outHandle) which is the function where idx eventually ends up.) Andinet 12:22, 27 December 2007 (EST) Fixed! Should be "for (unsigned int idx=1 ; idx < totalNumberOfTemplates+1 ; idx++ )"
- When calling: (in ie igstkMicronTrackerNewTest)
- 1. tracker->RequestStopTracking();
- 2. tracker->RequestStartTracking();
- ...the camera only returns "Error grabbing frame" (from the MicronTrackerNew::InternalThreadedUpdateStatus(...){ ... m_Cameras->grabFrame( m_SelectedCamera); ...}).
- My solution:
- By calling MicronTrackerNew::SetUpCamera() in MicronTrackerNew::InternalStartTracking, the problem is solved.
- Why is this?
- For some reason Cameras_AttachAvailableCameras need to be called when starting to track (not when opening communication?).
- Removing this->SetUpCameras() from MicronTrackerNew::InternalOpen (while calling it in MicronTrackerNew::InternalStartTracking), seem to work.
- Hard to pin point the excate reason for this because the MicronTracker API isn't very detailed when describing the reason for receiving the error "mtGrabFrameError" in m_Cameras->grabFrame(...).
- Suggestion:
- Maybe split up MicronTrackerNew::SetUpCameras() so that
- 1.MicronTrackerNew::InternalOpen does: this->m_Cameras->SetCameraCalibrationFilesDirectory( this->m_CalibrationFilesDirectory );
- 2.MicronTrackerNew::InternalStartTracking does: int success = this->m_Cameras->AttachAvailableCameras();
- Quote from MicronTracker API: Cameras_AttachAvailableCameras:
- "Connects to all the cameras available on the Firewire bus and starts their operation.
- This function provides a complete setup of the cameras interface, including optionally loading their calibration files, in a single call. The function detaches first (see
- Cameras_Detach()), so it can be repeatedly called, eg, following a temporary hardware failure such a as a cable disconnect, without the need to restart the client process."
Andinet 12:26, 27 December 2007 (EST) I can restart tracking without any problem in the igstkMicronTrackerTest program and also in the OneViewAndTrackingUsingMicronTracker example. Reattaching the camera every time we restart tracking will be an overkill
