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

clean up the ParentRef class #136

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

andlaus
Copy link
Collaborator

@andlaus andlaus commented Jun 5, 2023

this cleans up the internals of the ParentRef class to make it consistent with the information provided by the XML, use (IMO) slightly more descriptive names, changes the inheritance priority of ECU-SHARED-DATA from the highest to lowest (i.e., everbody can derive from it instead of nobody) and converts the class to be a dataclass to make it clearer which attributes are supposed to be available and slightly reduce the amount of boiler-plate code.

In this context, note that the inheritance scheme which we implement is not 100% conformant with the specification: We allow a more specific (e.g., ECU-VARIANT) layer to inherit from any more generic layer (e.g., PROTOCOL), but the spec only allows to inherit from more generic layers that are once removed (i.e., BASE-VARIANT for ECU-VARIANT) plus from ECU-SHARED-DATA. Since we allow all inheritance possibilities of the spec (plus some more), I doubt that this will be a problem anywere except -- maybe -- for ODX editors build on top of odxtools...

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

this cleans up the internals of the `ParentRef` class to make it
consistent with the information provided by the XML, use (IMO)
slightly more descriptive names, changes the inheritance priority of
ECU-SHARED-DATA from the highest to lowest (i.e., everbody can derive
from it instead of nobody) and converts the class to be a dataclass to
make it clearer which attributes are supposed to be available and
slightly reduce the amount of boiler-plate code.

In this context, note that the inheritance scheme which we implement
is not 100% conformant with the specification: We allow a more
specific (e.g., ECU-VARIANT) layer to inherit from any more generic
layer (e.g., PROTOCOL), but the spec only allows to inherit from more
generic layers that are once removed (i.e., BASE-VARIANT for
ECU-VARIANT) plus from ECU-SHARED-DATA. Since we allow all inheritance
possibilities of the spec (plus some more), I doubt that this will be
a problem anywere except -- maybe -- for ODX editors build on top of
odxtools...

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
@andlaus andlaus requested a review from kayoub5 June 5, 2023 09:35
odxtools/parentref.py Outdated Show resolved Hide resolved
odxtools/parentref.py Outdated Show resolved Hide resolved
examples/somersaultecu.py Outdated Show resolved Hide resolved
odxtools/parentref.py Outdated Show resolved Hide resolved
it was basically only used for an assertation. This check has now been
moved to `DiagLayer`, and the inheritance priority of a diag layer is
now defined by `DiagLayerType`.

thanks to [at]kayoub5 for pointing this out!

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
…ind of parent layer

this basically means the specialized PARENT-REFs (i.e.,
BASE-VARIANT-REF, PROTOCOL-REF, etc.) are kind of useless in the spec.

thanks to [at]kayoub5 for wondering about this!

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
again, thanks to [at]kayoub5!

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
@andlaus
Copy link
Collaborator Author

andlaus commented Jun 6, 2023

ok, let's get over this hump. thanks @kayoub5 !

@andlaus andlaus merged commit 652837c into mercedes-benz:main Jun 6, 2023
6 checks passed
@andlaus andlaus deleted the untangle_diaglayer_3 branch December 7, 2023 13:30
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.

None yet

2 participants