VRInstances missing OL, OV, SV, and UV value representation (VR) types
The VRInstances used by createByTypeString cause logging of 'Invalid vr type - using UN' when missing.
static createByTypeString(type) {
var vr = VRinstances[type];
if (vr === undefined) {
if (type == "ox") {
// TODO: determine VR based on context (could be 1 byte pixel data)
// https://github.com/dgobbi/vtk-dicom/issues/38
validationLog.error("Invalid vr type", type, "- using OW");
vr = VRinstances["OW"];
} else if (type == "xs") {
validationLog.error("Invalid vr type", type, "- using US");
vr = VRinstances["US"];
} else {
validationLog.error("Invalid vr type", type, "- using UN");
vr = VRinstances["UN"];
}
}
return vr;
}
During write this vr value is used to write bytes, but I think the VR value is preserved in writing and the writing of the raw bytes seems harmless for these types.
However, either the VRInstances should support writing OL, OV, SV, and UV with their own writers or if they map to using the UN write function then createByTypeString should not log a validation error for those VR strings.
Nitpick: Please include deep links to the code instead of copying the code inline. E.g. here's the code in the repo: https://github.com/dcmjs-org/dcmjs/blob/167de51c745ed1f697783abc053ac5b3e33726f4/src/ValueRepresentation.js#L345-L362
Definitely OX and XS handling should be fixed. These are VRs where the type is only known in the context of the rest of the file. Rather than throwing a validation error we should determine what they really are. (See this issue: https://github.com/dcmjs-org/dcmjs/issues/368).
Regarding OL, OV, SV, and UV, these should really just be added to the code explicitly. It should just follow the existing conventions so probably not a lot of work if someone wants to take it on (and add tests and testing data).