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

comparaminstance.spec property is not available during object creation #392

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

Conversation

nada-ben-ali
Copy link
Collaborator

Although I tested the changes introduced in #386, after upgrading to the latest odxtools version 9.4.0, we got the issue:

image

image

I believe not encountering the issue during testing was due to a Git branching issue :( .

@andlaus
Copy link
Member

andlaus commented Feb 24, 2025

can you provide a traceback? (does the exception happen in your own code or somewhere within odxtools.)

The point with this is that using .short_name before references have been resolved will almost certainly lead to incorrect results, and IMVHO it is better to get an issue explicitly flagged by an exception than it is to introduce subtle bugs in the code...

@kayoub5
Copy link
Collaborator

kayoub5 commented Feb 24, 2025

can you provide a traceback? (does the exception happen in your own code or somewhere within odxtools.)

The point with this is that using .short_name before references have been resolved will almost certainly lead to incorrect results, and IMVHO it is better to get an issue explicitly flagged by an exception than it is to introduce subtle bugs in the code...

The traceback is already included (screenshot)

This PR just reverts the change from https://github.com/mercedes-benz/odxtools/pull/386/files#r1952936788 to unblock the situation. you can do a follow up PR with a more proper fix.

@andlaus
Copy link
Member

andlaus commented Feb 24, 2025

The traceback is already included (screenshot)

Yes, but I'm a bit confused: for me, loading a PDX file works fine using the current main version and the ecu variants in these files all specify quite a few comparams... (IOW, I would like to be able to replicate -- or at least understand -- the issue before reverting.)

@kayoub5
Copy link
Collaborator

kayoub5 commented Feb 24, 2025

@andlaus In

dlc._finalize_init(self, self._odxlinks)

dlc._finalize_init(self, self._odxlinks) is called before dlc._resolve_snrefs(context), thus at that point of time spec has not been resolved yet, dlc._finalize_init creates the NamedItemList here

self._comparam_refs = NamedItemList(self._compute_available_commmunication_parameters())

Creating the NamedItemList causes short_name getter to be called

@andlaus
Copy link
Member

andlaus commented Feb 24, 2025

I understand. What I don't get is why this is a problem: the spec_ref of the comparam_refs is exclusively resolved in _resolve_odxlinks() and ._resolve_odxlinks() is called before .finalize_init(). Also, why does it work fine for me but obviously not for @nada-ben-ali?

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.

3 participants