mercedes-benz / odxtools

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

some fixes for files using ODX 2.0 #302

Closed andlaus closed 4 months ago

andlaus commented 4 months ago

In ODX 2.0, ComparamSpec does not exist, so ComparamSubset seems to be used as target for COMPARAM-SPEC-REF instead. This commit takes care of that and thus hopefully fixes #301.

Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH. Provider Information

andlaus commented 4 months ago

@zariiii9003, @kayoub5: would be great if you could test this on some of your files based on ODX 2.0...

zariiii9003 commented 4 months ago

COMPARAM-SPEC does exist in my pdx file, i think ComparamSubset is exclusive to newer versions.

Anyhow, I get a different error now, so i guess your fix works 😄

  File "scratch.py", line 3, in <module>
    db = odxtools.load_pdx_file(
  File "D:\PythonProjekte\odxtools\odxtools\load_pdx_file.py", line 8, in load_pdx_file
    return Database(pdx_zip=ZipFile(pdx_file))
  File "D:\PythonProjekte\odxtools\odxtools\database.py", line 91, in __init__
    self.refresh()
  File "D:\PythonProjekte\odxtools\odxtools\database.py", line 128, in refresh
    dlc._finalize_init(self._odxlinks)
  File "D:\PythonProjekte\odxtools\odxtools\diaglayercontainer.py", line 135, in _finalize_init
    base_variant._finalize_init(odxlinks)
  File "D:\PythonProjekte\odxtools\odxtools\diaglayer.py", line 148, in _finalize_init
    diag_comms = self._compute_available_diag_comms(odxlinks)
  File "D:\PythonProjekte\odxtools\odxtools\diaglayer.py", line 578, in _compute_available_diag_comms
    return self._compute_available_objects(get_local_objects_fn, not_inherited_fn)
  File "D:\PythonProjekte\odxtools\odxtools\diaglayer.py", line 523, in _compute_available_objects
    odxraise(f"Diagnostic layer {self.short_name} cannot inherit object "
  File "D:\PythonProjekte\odxtools\odxtools\exceptions.py", line 41, in odxraise
    raise error_type(message)
odxtools.exceptions.OdxError: Diagnostic layer A cannot inherit object B due to an unresolveable inheritance conflict between parent layers C and D

Maybe i'll check later, what the problem is.

andlaus commented 4 months ago

Maybe i'll check later, what the problem is.

this is probably due to an incorrect ODX file (or bug/corner case in odxtools): it seems like there is a service (or single-ECU-job) which is gets defined by both C and D (the parents of diag layer A) and C and D exhibit the same inheritance priority...

andlaus commented 4 months ago

BTW: there's a decent chance that your file will load in non-strict mode. just add

odxtools.exceptions.strict_mode = False

before you load the file in question. (Be aware that the resulting database is undefined in this case, so your mileage may vary.)

zariiii9003 commented 4 months ago

odxtools.exceptions.strict_mode = False works with a few warnings. Thank you for your help 👍

andlaus commented 4 months ago

odxtools.exceptions.strict_mode = False works with a few warnings. Thank you for your help

Nice :). I'd be grateful if you would investigate a bit if these warnings are caused by incomplete ODX 2.0 support in odxtools (as mentioned above, I do not have any ODX 2.0 based files) or due to incorrect input files. (the exception about the inheritance conflict from above seems to be the latter.)

andlaus commented 4 months ago

ok, let's merge this. I'm not really happy about it since I do not really understand ODX 2.0 enough to judge if this fix is correct, but since it seems to work it should be good enough...

zariiii9003 commented 4 months ago

@andlaus i tried to search for the root cause, but i do not know anything about the odx file format.

But this is what happens: I have a BaseVariant BV, which has parent-refs to functional groups FG_A and FG_B. FG_A and FG_B have a parent-ref to the same Protocol (UDS). The Protocol defines a DiagService, which is used in FG_A, but not in FG_B. When the BaseVariant is loaded, there is is a conflict, because both FG_A and FG_B (inherited) provide this DiagService.

I don't know, if this is this pdx is incorrectly defined, but CANape does not complain when i load it.

andlaus commented 4 months ago

thanks for your analysis :+1:. it seems very plausible and this is a perfectly valid scenario that should work, so I guess this is a bug in odxtools then: The spec says that if conflicting objects are identical, there is no conflict. The strange thing is that such objects are compared by value in the inheritance code, though: https://github.com/mercedes-benz/odxtools/blob/main/odxtools/diaglayer.py#L515-L521

can you find out why that if statement does not trigger? (DiagService is a dataclass, so __equals__() is automatically generated.)