Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the code computing value inheritance #268

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

andlaus
Copy link
Collaborator

@andlaus andlaus commented Feb 19, 2024

I recently noticed that the ODX specification for value inheritance slightly differed from the version which was implemented by odxtools:

  • objects directly inherited from ECU-SHARED-DATA parents did not override objects inherited from diagnostic layers of different type
  • the "not inherited" objects were not considered while populating the list, causing the function to produce undefined results if inheritance conflicts were solved using the NOT-INHERITED-* mechanism.
  • inheritance conflicts where generally not handled explicitly.

This PR fixes these issues and we now hopefully fully adhere to the specification. (I doubt that it did matter for many datasets, though. For the ones I have access to, it didn't.)

Andreas Lauser <[email protected]>, on behalf of MBition GmbH.
Provider Information

the new version hopefully fully adheres to the specification, the
previous version had some issues with the priority of ECU shared data
parents, and conflict handling of inherited objects did not fully match
the specification:

 - the "not inherited" objects where not considered while populating
   the list, causing the function to produce undefined results if
   inheritance conflicts were solved using this mechanism.
 - inheritance conflicts where generally not detected.

Signed-off-by: Andreas Lauser <[email protected]>
Signed-off-by: Gerrit Ecke <[email protected]>
@andlaus
Copy link
Collaborator Author

andlaus commented Feb 20, 2024

@kayoub5: can you have a brief look at this / check that your files continue to work?

@andlaus
Copy link
Collaborator Author

andlaus commented Feb 22, 2024

Just a heads up: barring vetos, I'll merge this early next week.

@andlaus
Copy link
Collaborator Author

andlaus commented Feb 27, 2024

@kayoub5: do you see any issues with this one? if not, I'd like to merge it tomorrow...

@kayoub5
Copy link
Collaborator

kayoub5 commented Feb 27, 2024

@kayoub5: do you see any issues with this one? if not, I'd like to merge it tomorrow...

I don't see issues, but I don't have time to test it with the PDX files I have

@andlaus
Copy link
Collaborator Author

andlaus commented Feb 28, 2024

I don't see issues, but I don't have time to test it with the PDX files I have

okay. I think the risks of this are quite small, so let's merge and fix the fallout later (if any).

@andlaus andlaus merged commit abdf3c8 into mercedes-benz:main Feb 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants