comparaminstance.spec property is not available during object creation

[nada-ben-ali]

Although I tested the changes introduced in #386, after upgrading to the latest odxtools version 9.4.0, we got the issue:

I believe not encountering the issue during testing was due to a Git branching issue :( .

[andlaus]

can you provide a traceback? (does the exception happen in your own code or somewhere within odxtools.)

The point with this is that using .short_name before references have been resolved will almost certainly lead to incorrect results, and IMVHO it is better to get an issue explicitly flagged by an exception than it is to introduce subtle bugs in the code...

[kayoub5]

can you provide a traceback? (does the exception happen in your own code or somewhere within odxtools.)

The point with this is that using .short_name before references have been resolved will almost certainly lead to incorrect results, and IMVHO it is better to get an issue explicitly flagged by an exception than it is to introduce subtle bugs in the code...

The traceback is already included (screenshot)

This PR just reverts the change from https://github.com/mercedes-benz/odxtools/pull/386/files#r1952936788 to unblock the situation. you can do a follow up PR with a more proper fix.

[andlaus]

The traceback is already included (screenshot)

Yes, but I'm a bit confused: for me, loading a PDX file works fine using the current main version and the ecu variants in these files all specify quite a few comparams... (IOW, I would like to be able to replicate -- or at least understand -- the issue before reverting.)

[kayoub5]

@andlaus In

odxtools/odxtools/database.py

Line 154 in 1e5f030

dlc._finalize_init(self, self._odxlinks)

dlc._finalize_init(self, self._odxlinks) is called before dlc._resolve_snrefs(context), thus at that point of time spec has not been resolved yet, dlc._finalize_init creates the NamedItemList here

odxtools/odxtools/diaglayers/hierarchyelement.py

Line 224 in 1e5f030

self._comparam_refs = NamedItemList(self._compute_available_commmunication_parameters())

Creating the NamedItemList causes short_name getter to be called

[andlaus]

I understand. What I don't get is why this is a problem: the spec_ref of the comparam_refs is exclusively resolved in _resolve_odxlinks() and ._resolve_odxlinks() is called before .finalize_init(). Also, why does it work fine for me but obviously not for @nada-ben-ali?

[nada-ben-ali]

@andlaus the files that reported this issue have model version "2.0.1". For the files with model version "2.2.0", the issue is not reproducible.

[andlaus]

okay, thanks for the info. can you dig a deeper into what causes this?

I still cannot explain it because regardless of the model version, the odxlink references should be resolved when this NamedItemList gets created. Also, I suspect that the names of the items exposed by the ComparamInstance NamedItemList do not correspond to the short names of their comparam specs... (maybe ´._resolve_odxlinks()` is not called at all?)

[nada-ben-ali]

@andlaus the issue is that while resolving the odxlinks, the comparams were not resolved because:

doc_frag_db = self._db.get(ref_frag) was None, which is because the ref_frag object has doc_type=COMPARAM_SPEC.
However, the OdxDocFragment object with the same doc_name in self._db has doc_type=COMPARAM_SUBSET.

Database.comparam_specs is empty, while Database.subsets contains the required comparam specs, which were tagged as COMPARAM-SPEC in the PDX file.
In the PDX file, there is no COMPARAM-SUBSET tag, only the COMPARAM-SPEC tag exists.

[andlaus]

@nada-ben-ali: that sounds a lot like an unrelated issue with the dataset which was uncovered by that change? what do you think about changing the fallback code for ODXLINK resolution if the reference was not found in the specified document fragment to "use the first document fragment which has an object with the specified ID"?

[nada-ben-ali]

@kayoub @andlaus could you please review the lastest changes?

[andlaus]

cool, now I'm happy! thanks for your patience :)

[andlaus]

small question (now irrelevant because the PR is already merged): isn't it possible to simply look at et_element.tag, i.e.

doc_type = DocType.COMPARAM_SUBSET if et_element.tag == "COMPARAM_SUBSET" else DocType.COMPARAM_SPEC

[kayoub5]

possible

[nada-ben-ali]

yess, it works as well (the tag is "COMPARAM-SUBSET" instead of "COMPARAM_SUBSET").

[andlaus]

I supppose that would be less hacky, but I don't mind if you want to leave it as-is...