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

TableStructParameter: allow the table key to be specified via SNREF #288

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

andlaus
Copy link
Collaborator

@andlaus andlaus commented Apr 8, 2024

This requires to use a custom _resolve_snrefs() function because the parameter object itself cannot know the context from where it is called...

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

this requires to use a custom `_resolve_snrefs()` function because the
parameter object itself cannot know the context from where it is
called...

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 April 8, 2024 15:17
@andlaus
Copy link
Collaborator Author

andlaus commented Apr 8, 2024

@QWander: can you check if this fixes your issue (#283)? If it doesn't it would be great if you provided a backtrace of the failure...

@QWander
Copy link
Contributor

QWander commented Apr 9, 2024

issue not solved, specific information:
File "odxtools\load_pdx_file.py", line 8, in load_pdx_file
return Database(pdx_zip=ZipFile(pdx_file))
File "odxtools\database.py", line 91, in init
self.refresh()
File "odxtools\database.py", line 128, in refresh
dlc._finalize_init(self._odxlinks)
File "odxtools\diaglayercontainer.py", line 129, in _finalize_init
ecu_shared_data._finalize_init(odxlinks)
File "odxtools\diaglayer.py", line 228, in _finalize_init
self.diag_layer_raw._resolve_snrefs(self)
File "odxtools\diaglayerraw.py", line 291, in _resolve_snrefs
self.diag_data_dictionary_spec._resolve_snrefs(diag_layer)
File "odxtools\diagdatadictionaryspec.py", line 232, in _resolve_snrefs
structure._resolve_snrefs(diag_layer)
File "odxtools\basicstructure.py", line 323, in _resolve_snrefs
param._resolve_snrefs(diag_layer)
File "odxtools\parameters\valueparameter.py", line 54, in _resolve_snrefs
super()._resolve_snrefs(diag_layer)
File "odxtools\parameters\parameterwithdop.py", line 69, in _resolv File "odxtools\parameters\parameterwithdop.py", line 69, in _resolve_snrefs
self._dop = odxrequire(ddds.all_data_object_properties.get(self.dop_snref))
File "odxtools\exceptions.py", line 76, in odxrequire
odxraise(message)
File "odxtools\exceptions.py", line 39, in odxraise
raise error_type()
odxtools.exceptions.OdxError

@andlaus
Copy link
Collaborator Author

andlaus commented Apr 9, 2024

okay, it seems like at least you now get a different exception: instead of complaining that table-keys cannot be referenced via SNREF in table-struct parameters, it cannot resolve the DOP associated with the parameter in parameterwithdop.py.

can you find out what the type of the referenced DOP is? (change line 69 of ´parameterwithdop.py` to

if ddds.all_data_object_properties.get(self.dop_snref) is None:
  print(f"Cannot resolve SNREF to DOP '{self.dop_snref}'")
self._dop = odxrequire(ddds.all_data_object_properties.get(self.dop_snref))

after you know the short name of the DOP that cannot be resolved, you can hunt it down in your PDX file:

unzip -o $YOUR_PDX_FILE.pdx
grep -B5 "<SHORT-NAME>$NAME_OF_DOP</SHORT-NAME>" *.odx-*

I suspect that it is a DYNAMIC-ENMARKER-FIELD, which -- as far as I know -- is currently the only type of DOP which is not yet implemented...

@andlaus
Copy link
Collaborator Author

andlaus commented Apr 16, 2024

@kayoub5: do you see any issues with this PR? If no, I'd like to push the Green Button...
@QWander: will you determine the type of the DOP which cannot be resolved?

@@ -61,15 +60,31 @@ def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None:
self._table_key = odxlinks.resolve(self.table_key_ref, TableKeyParameter)

@override
@final
def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should consider in the future making _resolve_snrefs take a "context" or "lookup" object, where diag_layer is a property.

We are getting too many "_resolve_snrefs" special cases

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed. this is a major refactoring, though. I'll open an issue...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, done: #290

@andlaus andlaus merged commit 88f5fd7 into mercedes-benz:main Apr 16, 2024
7 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.

None yet

3 participants