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.

Sign up for GitHub

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

resolve snrefs for child items #373

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

resolve snrefs for child items #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion odxtools/diagservice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,12 @@ def _resolve_snrefs(self, context: SnRefContext) -> None:
# here because ComparamInstance.short_name is only valid after
# reference resolution
self._comparams = NamedItemList(self.comparam_refs)

if self._request:
self._request._resolve_snrefs(context)
for response in self._positive_responses:
response._resolve_snrefs(context)
for response in self._negative_responses:
response._resolve_snrefs(context)
Copy link
Collaborator

@andlaus andlaus Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this might fix the specific problem which you've encountered, this is pretty much a coincidence: this causes the referenced objects to be retargeted to basically the last ECU variant encountered during the reference resolution procedure. If you're looking at another diagnostic layer, e.g. the base variant, its snrefs will have been incorrectly resolved...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, u are correct. So with this change, I need to call _resolve_snrefs again before I try to visit other variants. Since the child variant would use parent objects (References) directly, I cannot find other solutions for this problem.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess #374 might make your life considerably easier?!

context.diag_service = None

def decode_message(self, raw_message: bytes) -> Message:
Expand Down
4 changes: 4 additions & 0 deletions odxtools/field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,7 @@ def _resolve_snrefs(self, context: SnRefContext) -> None:
if self.env_data_desc_snref is not None:
self._env_data_desc = resolve_snref(self.env_data_desc_snref, ddds.env_data_descs,
EnvironmentDataDescription)
if self._structure:
self._structure._resolve_snrefs(context)
if self._env_data_desc:
self._env_data_desc._resolve_snrefs(context)
2 changes: 2 additions & 0 deletions odxtools/parameters/parameterwithdop.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ def _resolve_snrefs(self, context: SnRefContext) -> None:
if self.dop_snref:
ddds = odxrequire(context.diag_layer).diag_data_dictionary_spec
self._dop = resolve_snref(self.dop_snref, ddds.all_data_object_properties, DopBase)
if self._dop:
self._dop._resolve_snrefs(context)

@property
def dop(self) -> DopBase:
Expand Down
1 change: 1 addition & 0 deletions odxtools/parameters/tablekeyparameter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None:
self._table_row = odxlinks.resolve(self.table_row_ref, TableRow)
else:
self._table_row = odxlinks.resolve(self.table_row_ref)
self._table_row._resolve_odxlinks(odxlinks)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this line, the parser will raise exception on below line, said that the "table" of tablerow does not exist.

Copy link
Collaborator

@andlaus andlaus Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. That said, _resolve_odxlinks() must not be called for objects that are the result of a reference resolution lest there be cyclic reference resolution hell. as an alternative to this patch line 94 should better be replaced by

            self._table = odxlinks.resolve(self._table_row.table_ref, Table)

in this case, the .table property below should also be simplified to

    @property
    def table(self) -> "Table":
        return self._table

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want you change the PR to only fix this issue (and remove the SNREF changes) or do you prefer if I do it?

self._table = self._table_row.table

@override
Expand Down
Loading