BUG: Fix embedded transform errors in vtkPlusOpenIGTLinkVideoSource #874
Conversation
When receiving messages with embedded transforms from vtkPlusOpenIGTLinkVideoSource, several errors occurred.
- If the From and To coordinate frames of the embedded transform are the same, the Transform repository will log an error ("Setting a transform to itself is not allowed") when sending any images using vtkPlusOpenIGTLinkServer. Fixed by not unpacking the transform when From == To.
- If an incoming image message has no status set for the embedded transform, the transform is considered invalid. Fixed by marking incoming embedded transforms with no status as OK.
Re PlusToolkit#873
| @@ -74,17 +74,25 @@ PlusStatus vtkPlusOpenIGTLinkVideoSource::InternalUpdate() | |||
| igsioTrackedFrame trackedFrame; | |||
| igtl::MessageBase::Pointer bodyMsg = this->MessageFactory->CreateReceiveMessage(headerMsg); | |||
|
|
|||
| igsioTransformName embeddedTransformName = this->ImageMessageEmbeddedTransformName; | |||
| if (this->ImageMessageEmbeddedTransformName.From() == this->ImageMessageEmbeddedTransformName.To()) | |||
There was a problem hiding this comment.
I don't think we should do it. It goes against the rules that every transform is defined by the coordinate systems it transforms between. SomethingToSomethingTransform is always identity. Why the user cannot just invent any other name for To transform?
There was a problem hiding this comment.
Maybe the true error is that setting a transform to itself should be allowed, and it should be identity?
There was a problem hiding this comment.
I agree, it should be allowed (I think it is already allowed). If the matrix is anything else than identity then it should be rejected, because there is nowhere to store that in the transform repository (we need two different coordinate system names for that).
There was a problem hiding this comment.
Why the user cannot just invent any other name for To transform?
Currently if the device is connecting to a PlusServer, the embedded transform name must be specified, and must be between two valid coordinate systems, otherwise no data will be sent. This is another bug tracked in #873, but I think that using ImageToImage for the embedded transform name should be allowed in any case.
I agree, it should be allowed (I think it is already allowed).
If a tracked frame contains a transform of the form SomethingToSomething, then it will throw an error at this point when PlusOpenIGTLinkServer is copying transforms from the tracked frames to the transform repository:
https://github.com/IGSIO/IGSIO/blob/470b1553768981f3a86b9a79bf0f9016bebe74ce/Source/IGSIOCommon/vtkIGSIOTransformRepository.cxx#L151-L155
| std::string strStatus = trackedFrame.GetFrameField(transformStatusField); | ||
| if (strStatus.empty()) | ||
| { | ||
| // Transform status has not been set on the frame, and is assumed to be OK. |
There was a problem hiding this comment.
Is this a valid reason for a LOG_WARNING? (or LOG_WARNING once)
When receiving messages with embedded transforms from vtkPlusOpenIGTLinkVideoSource, several errors occurred.
Re #873