-
Notifications
You must be signed in to change notification settings - Fork 80
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
add support for special data groups (SDGs) #99
Conversation
Be aware that I might have missed a place or two where SDGs are allowed because the inheritance hierarchy defined by the ODX XSD is not identical to the one of the python code. Besides support for SDGs, this patch fixes various issues which I've stumbled over while testing it. (Side note: we *need* more comprehensive unittests!) Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Alexander Walz <alexander.walz@mbition.io>
Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Alexander Walz <alexander.walz@mbition.io>
odxtools/odxlink.py
Outdated
@@ -175,6 +176,9 @@ def resolve(self, ref: OdxLinkRef) -> Any: | |||
|
|||
obj = doc_frag_db.get(odx_id) | |||
if obj is not None: | |||
if odx_id.local_id == "stupid.sdg.caption": | |||
self._stupid_sdg_resolved = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops. that is an artifact of the instrumentation code which I used to develop this: I added an SDG which contained an SDG-caption with the ID "stupid.sdg.caption" to the XML tag I wanted to test and ensured that it was seen during the ID resolution. (using this method I found a few classes which had issues with ID resolution, BTW.)
p.resolve_references(parent_dl, odxlinks) | ||
else: | ||
p._resolve_references(odxlinks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be better to make this a common function in base class of parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a hack to be able to resolve SN-REFs: because to be able to do so in the limited way that mattered for us, we needed to access the parent diaglayer for those types of parameters where they are commonly used. I intend to resolve SN-REFs properly at some point (probably by extending the OdxLinkDatabase
class), but that is another major refactoring and thus left out of this PR for now. (in the mean time, naming the _resolve_references()
method differently for such classes would significantly reduce the confusion?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add resolve_references(parent_dl, odxlinks) to parameter class, default implementation does nothing, and override it in sub classes.
@andlaus I have limited access to my pc those days due to health issues, don't block merging waiting for my replies if I take too long |
as usual, the thanks go to [at]kayoub5! Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Oliver Kopp <oliver.kopp@mbition.io>
get well soon :) |
alright, it's time to get this merged... |
Special Data Groups (SDGs) are used to store data which is not yet standardized in a semi-structured manner. Be aware that I might have missed a place or two where SDGs are allowed because the inheritance hierarchy defined by the ODX XSD is not identical to the one of the python code.
Besides support for SDGs, this patch fixes various issues which I've stumbled over while testing it. (Side note: we need more comprehensive unittests!)
Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH.
Provider Information