mercedes-benz / odxtools

odxtools is a collection of utilities to interact with the diagnostic functionality of automotive electronic control units using python
MIT License
172 stars 70 forks source link

Address hardcoded DOCTYPE attribute for PARENT-REF #261

Closed willzhang05 closed 7 months ago

willzhang05 commented 7 months ago

Addresses https://github.com/mercedes-benz/odxtools/issues/260

I'm not quite sure of the correct way to best handle this, I'm not 100% sure on the spec for PARENT-REF, but looking at the ISO 22901-1 document, I don't see any instances where the DOCTYPE attribute is used with PARENT-REF. Is there somewhere in the spec that lists DOCTYPE as a valid attribute for PARENT-REF?

andlaus commented 7 months ago

hm, I don't know about DOCTYPE, but removing the DOCREF will probably make it fail if the referenced parent is in a different DIAG-LAYER-CONTAINER than the variant that uses it (?)

That said, I thought that all variants are contained by DIAG-LAYER-CONTAINERs, but maybe this is not the case. what is the nesting of the tags for your protocol? for me, protocols look like

<ODX>
 <DIAG-LAYER-CONTAINER>
  <PROTOCOLS>
   <PROTOCOL>
    <SHORT-NAME>CAN</SHORT-NAME>
   </PROTOCOL>
  </PROTOCOLS>
 </DIAG-LAYER-CONTAINER>
</ODX>
willzhang05 commented 7 months ago

yes, that's how the tags are nested for me as well. I tried both removing just the DOCREF or just removing the DOCTYPE, but the only one that seemed to be valid for ODXStudio was without both. I'm not sure if this is a Vector quirk, but I'm just having a hard time finding any information about this part of the spec.

andlaus commented 7 months ago

in the ODX model, not specifying the DOCREF implies that the referenced object is in the same document fragment (read: .odx-* file) as the referencing object. as far as I understand this matter, the DOCREF can be specified even if this is the case, it is just redundant in this case; but maybe I'm wrong here, the wording is a bit ambiguous:

Generally, odxlinks are resolved in two steps:
a) Find the document fragment that contains the link target
b) Retrieve the link target from the document fragment
The first step uses the attributes DOCTYPE and DOCREF:
These two attributes may be defined only jointly. The DOCREF attribute refers to the
SHORT-NAME of the ODX-CATEGORY or the DIAG-LAYER. If DOCREF and DOCTYPE
are not set, the link target exists in the local ODX-CATEGORY where also the link source
exists (internal odxlink).

note that this wording neither explicitly allows nor forbits to specify the document fragment for "internal" odxlinks. thus, if candela does not support this, it might or might not be considered a bug. we could work around this by checking if the referenced diag layer is in the same container as the referencing one and only add these attributes if this is not the case. IMO this would be quite ugly, though...

andlaus commented 7 months ago

okay, I played a bit with this code. I noticed that the DOCTYPE attribute is not a container, but a layer. does odxstudio accept the file if you change DOCTYPE="CONTAINER" to DOCTYPE="LAYER"?

willzhang05 commented 7 months ago

closed in lieu of the solution in https://github.com/mercedes-benz/odxtools/pull/262