implement de- and encoding of environment data descriptions #321
Conversation
this requires to keep track of the de-/encoded parameters because the environment data that ought to be used by a given environment data description depends on the value used for the preceeding parameter. Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Florian Jost <florian.jost@mbition.io>
| #: List of parameters that have been decoded so far. The journal | ||
| #: is used by some types of parameters which depend on the values of | ||
| #: other parameters; i.e., environment data description parameters | ||
| journal: List[Tuple["Parameter", Optional[ParameterValue]]] = field(default_factory=list) |
There was a problem hiding this comment.
a dict would be faster for lookup
There was a problem hiding this comment.
we need the sequence in which the parameters were processed, i.e. that would require at least OrderedDict. That said, I'm not sure how such a lookup would be realized and also parameters frequently appear more than once, e.g., for fields (which are repeated structure objects)...
There was a problem hiding this comment.
You are using the journal with reversed, meaning only the last value is actually needed
There was a problem hiding this comment.
yes, but then a different name than .journal is also required IMO. (I think that conceptually, the concept of a "change log" for en-/decoding is quite nice.) Performance-wise I don't think that it will matter much because responses containing environment data descriptions are probable quite rare (e.g., I don't have any files which use them and so far we have not gotten any bug reports about missing en-/decoding support.)
odxtools/dtcdop.py
Outdated
| if isinstance(physical_value, DiagnosticTroubleCode): | ||
| trouble_code = physical_value.trouble_code | ||
| elif isinstance(physical_value, int): | ||
| def numerical_trouble_code(self, dtc_value: ParameterValue) -> int: |
There was a problem hiding this comment.
function names usually start with a verb
There was a problem hiding this comment.
do you prefer get_numerical_trouble_code(), convert_to_numerical_trouble_code() or something else?
There was a problem hiding this comment.
okay, I renamed it to convert_to_numerical_trouble_code()
| odxassert(len(dtcs) == 1) | ||
| trouble_code = dtcs[0].trouble_code | ||
| dtcs = [dtc for dtc in self.dtcs if dtc.short_name == dtc_value] | ||
| if len(dtcs) != 1: |
There was a problem hiding this comment.
Could it happen that length > 1 ?
There was a problem hiding this comment.
I don't think so: in this case, it would be impossible to decide which of the environment datas that apply for the DTC ought to be used. (I suppose that this constitutes an incorrect PDX file?)
There was a problem hiding this comment.
Does the specs say anything about that?
There was a problem hiding this comment.
I'm not aware that it explicitly mentions multiple "normal" environment datas matching the same DTC, but it says that there can only be a single ALL-DATA environment data. Since multiple ALL-DATA environment datas is exactly the same problem as multiple "normal" ones for a given DTC, I infer that this is forbidden. (in which order shall these environment datas be processed anyway?)
There was a problem hiding this comment.
ok, sorry: I was in the wrong mental context for my previous two comments (interestingly they are still applicable to some extend): more than one DTC with the same short name cannot happen in valid ODX datasets because short names are required to be unique in their respective context. (if they were not, SNREFs would not work...)
the unit tests deal provide this themselves... Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Florian Jost <florian.jost@mbition.io>
…ouble_code()' as [at]kayoub5 rightfully noted, method names should start with a verb... Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Florian Jost <florian.jost@mbition.io>
|
@kayoub5: do you see any further issues here? |
Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Florian Jost <florian.jost@mbition.io>
This PR implements en- and decoding of environment data descriptions, the fourth multiplexer mechanism specified by the ODX standard besides tables, NRC-CONSTs and multiplexers. This requires to keep track of the de-/encoded parameters because the environment data that ought to be used by a given environment data description depends on the value used for one of the preceding parameters.
Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH.
Provider Information