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

dop_snref Temporary plan #283

Closed
wants to merge 7 commits into from
Closed
12 changes: 11 additions & 1 deletion odxtools/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ def __init__(self,

def refresh(self) -> None:
# Create wrapper objects
self._ecu_shared_datas = NamedItemList(chain(*[dlc.ecu_shared_datas
for dlc in self.diag_layer_containers if dlc.ecu_shared_datas]))

self._diag_layers = NamedItemList(
chain(*[dlc.diag_layers for dlc in self.diag_layer_containers]))

Expand Down Expand Up @@ -125,13 +128,20 @@ def refresh(self) -> None:
# let the diaglayers sort out the inherited objects and the
# short name references
for dlc in self.diag_layer_containers:
dlc._finalize_init(self._odxlinks)
dlc._finalize_init(self._odxlinks, self._ecu_shared_datas)
Copy link
Member

Choose a reason for hiding this comment

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

if you need to add such an argument, make it a Database object, i.e., this line should become dlc._finalize_init(self._odxlinks, self). I somehow doubt that this is necessary, though...


@property
def odxlinks(self) -> OdxLinkDatabase:
"""A map from odx_id to object"""
return self._odxlinks

@property
def ecu_shared_datas(self) -> OdxLinkDatabase:
Copy link
Member

Choose a reason for hiding this comment

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

that should return NamedItemList[DiagLayer] like the .ecus and .protocols properties. (on line 95 you define self._ecu_shared_data this way...)

"""
All ecu_shared_datas defined by this database
"""
return self._ecu_shared_datas

@property
def protocols(self) -> NamedItemList[DiagLayer]:
"""
Expand Down
26 changes: 25 additions & 1 deletion odxtools/diaglayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None:

self.diag_layer_raw._resolve_odxlinks(odxlinks)

def _finalize_init(self, odxlinks: OdxLinkDatabase) -> None:
def _finalize_init(self, odxlinks: OdxLinkDatabase,
ecu_shared_datas: NamedItemList['DiagLayer']) -> None:
"""This method deals with everything inheritance related and
-- after the final set of objects covered by the diagnostic
layer is determined -- resolves any short name references in
Expand Down Expand Up @@ -210,6 +211,8 @@ def _finalize_init(self, odxlinks: OdxLinkDatabase) -> None:
sdgs=ddds_sdgs,
)

self._add_import_ref(ecu_shared_datas)

#####
# compute the communication parameters applicable to the
# diagnostic layer. Note that communication parameters do
Expand Down Expand Up @@ -591,6 +594,27 @@ def not_inherited_fn(parent_ref: ParentRef) -> List[str]:

return self._compute_available_objects(get_local_objects_fn, not_inherited_fn)

def _add_import_ref(self, ecu_shared_datas) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

I guess that after #285, the IMPORT-REF mechanism is implemented. What can/should be added to Database and DiagLayerContainer though, are wrapper attributes for the different kinds of DiagLayers (analogos to .ecus or .protocols)

if imp_refs := self.import_refs:
for imp in imp_refs:
doc_name = next(iter(imp.ref_docs)).doc_name
doc_type = next(iter(imp.ref_docs)).doc_type
# Delete the “DLC_” name prefix
if doc_type == 'CONTAINER':
doc_name = doc_name[4:]
Copy link
Member

Choose a reason for hiding this comment

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

the DLC_ prefix in short names of diag layer containers is a convention of the PDX files you are interested in. we cannot hard-code this here...

if doc_name in ecu_shared_datas:
share_ddds = (ecu_shared_datas[doc_name]
.diag_layer_raw.diag_data_dictionary_spec)
if hasattr(share_ddds, 'all_data_object_properties'):
share_ddds_all_properties = share_ddds.all_data_object_properties
# diag_data_dictionary_spec
if ddds_all_properties := (self.diag_layer_raw.diag_data_dictionary_spec
.all_data_object_properties):
ddds_all_properties.append_list(share_ddds_all_properties)
else:
odxraise(f"The imports-refs method used in {self.short_name} is error,"
f"file {doc_name} is undefined")

#####
# </value inheritance mechanism helpers>
#####
Expand Down
13 changes: 7 additions & 6 deletions odxtools/diaglayercontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,18 @@ def _resolve_odxlinks(self, odxlinks: OdxLinkDatabase) -> None:
for ecu_variant in self.ecu_variants:
ecu_variant._resolve_odxlinks(odxlinks)

def _finalize_init(self, odxlinks: OdxLinkDatabase) -> None:
def _finalize_init(self, odxlinks: OdxLinkDatabase,
ecu_shared_datas: NamedItemList[DiagLayer]) -> None:
for ecu_shared_data in self.ecu_shared_datas:
ecu_shared_data._finalize_init(odxlinks)
ecu_shared_data._finalize_init(odxlinks, ecu_shared_datas)
Copy link
Member

Choose a reason for hiding this comment

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

as mentioned above, it is better pass the whole Database object here (if this is really necessary.)

for protocol in self.protocols:
protocol._finalize_init(odxlinks)
protocol._finalize_init(odxlinks, ecu_shared_datas)
for functional_group in self.functional_groups:
functional_group._finalize_init(odxlinks)
functional_group._finalize_init(odxlinks, ecu_shared_datas)
for base_variant in self.base_variants:
base_variant._finalize_init(odxlinks)
base_variant._finalize_init(odxlinks, ecu_shared_datas)
for ecu_variant in self.ecu_variants:
ecu_variant._finalize_init(odxlinks)
ecu_variant._finalize_init(odxlinks, ecu_shared_datas)

@property
def diag_layers(self) -> NamedItemList[DiagLayer]:
Expand Down
16 changes: 16 additions & 0 deletions odxtools/nameditemlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ def append(self, item: T) -> None:

super().append(item)

def append_list(self, item_list) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

what's the difference of this and the .extend() method?! Also, there's no type annotation for item_list.

"""
An instance of a itemAttributeList class,add another instance

\return The name under which item is accessible
"""
if item_list:
for item in item_list:
item_name = self._get_item_key(item)
if item_name not in self._item_dict:
self._item_dict[item_name] = item
super().append(item)

def _add_attribute_item(self, item: T) -> None:
item_name = self._get_item_key(item)

Expand Down Expand Up @@ -152,6 +165,9 @@ def __getattr__(self, key: str) -> T:

return self._item_dict[key]

def __contains__(self, key) -> bool:
return key in self._item_dict
Copy link
Member

@andlaus andlaus Apr 8, 2024

Choose a reason for hiding this comment

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

this method should be unnecessary: ItemAttributeList is a list, so the x in x in my_named_item_list should be an item. maybe this function can be renamed to .contains_name()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if x in my_named_item_list: print("x in my_named_item_list") else: print("x not in my_named_item_list")
This method returns a Boolean value (True or False), indicating whether the key exists in the _item_dict dictionary, not the item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all code:return True if key in self._item_dict else False

Copy link
Member

Choose a reason for hiding this comment

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

sure. the point is that this class primarily is a list, not a dict, and lists check for the item values, not the keys like dictionaries...


def get(self, key: Union[int, str], default: Optional[T] = None) -> Optional[T]:
if isinstance(key, int):
if 0 <= key and key < len(self):
Expand Down
7 changes: 5 additions & 2 deletions odxtools/parameters/parameterwithdop.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from ..dopbase import DopBase
from ..dtcdop import DtcDop
from ..encodestate import EncodeState
from ..exceptions import odxassert, odxrequire
from ..exceptions import odxassert, odxrequire, odxraise
from ..odxlink import OdxDocFragment, OdxLinkDatabase, OdxLinkId, OdxLinkRef
from ..odxtypes import ParameterValue
from ..physicaltype import PhysicalType
Expand Down Expand Up @@ -66,7 +66,10 @@ def _resolve_snrefs(self, diag_layer: "DiagLayer") -> None:

if self.dop_snref:
ddds = diag_layer.diag_data_dictionary_spec
self._dop = odxrequire(ddds.all_data_object_properties.get(self.dop_snref))
if snref_shortname := ddds.all_data_object_properties.get(self.dop_snref):
Copy link
Member

Choose a reason for hiding this comment

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

That if statment is equivalent to simply

self._dop = ddds.all_data_object_properties.get(self.dop_snref)

that said, I think the odxrequire() should stay because the reference to the DOP must be resolved. If it isn't (most probably because the referenced DOP is not yet implemented, e.g., DYNAMIC-ENDMARKER-FIELD, one should use the non-strict mode for now. This is because quite a bit of code assumes that ParameterWithDop objects provide a DOP...)

self._dop = odxrequire(snref_shortname)
else:
odxraise(f"{self.dop_snref} in not found in {diag_layer.short_name}")

@property
def dop(self) -> DopBase:
Expand Down
Loading