slightly refactor the length key code #161
Conversation
now, instead of using hacks with ODXLINK IDs, proper references to `LengthKeyParameter` are used by `ParamLengthInfoType`. This necessitates to include the diag coded types in the reference resolution machinery because `ParamLengthInfoType` is a diag coded type and it needs to resolve the reference to the length key parameter. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
| @@ -478,11 +493,28 @@ def __init__( | |||
| base_type_encoding=base_type_encoding, | |||
| is_highlow_byte_order_raw=is_highlow_byte_order_raw, | |||
| ) | |||
| self.length_key_id = length_key_id | |||
| self.length_key_ref = length_key_ref | |||
There was a problem hiding this comment.
small breaking change, bump major again?
There was a problem hiding this comment.
in principle its a breaking change, but I'm willing to bet a box of Champaign that nobody outside of this repository ever used this attribute...
odxtools/diagcodedtypes.py
Outdated
| @@ -519,14 +551,14 @@ def convert_bytes_to_internal(self, decode_state: DecodeState, bit_position: int | |||
| # if isinstance(param_value.parameter, LengthKeyParameter) would be prettier, | |||
| # but leads to cyclic import... | |||
| if (parameter.parameter_type == "LENGTH-KEY" and | |||
| parameter.odx_id == self.length_key_id # type: ignore | |||
| parameter.odx_id == self.length_key.odx_id # type: ignore | |||
There was a problem hiding this comment.
type ignore still needed?
There was a problem hiding this comment.
unfortunately it is still required:
> mypy --ignore-missing-imports odxtools examples/ tests/
odxtools/diagcodedtypes.py:554: error: "Parameter" has no attribute "odx_id" [attr-defined]
Found 1 error in 1 file (checked 110 source files)
this is due to the fact that the base class for parameters does not exhibit an ID. (only some do, amongst them LengthKeyParameter.)
We should only ignore the attr-defined error here though. Fixed, thanks!
| ): | ||
| # The bit length of the parameter to be extracted is given by the length key. | ||
| assert isinstance(value, int) | ||
| bit_length = value | ||
| break | ||
|
|
||
| assert bit_length is not None, f"Did not find any length key with ID {self.length_key_id}" | ||
| assert bit_length is not None, f"Did not find any length key with ID {self.length_key.odx_id}" |
There was a problem hiding this comment.
raise odx exception instead of assert
There was a problem hiding this comment.
I'd prefer not to open this can of worm in this PR. (I guess I should do the assert -> odxassert() conversion soon.)
The praise goes to [at]kayoub5, all remaining bugs are my own fault! Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
now, instead of using hacks with ODXLINK IDs, proper references to
LengthKeyParameterare used byParamLengthInfoType. This necessitates to include the diag coded types in the reference resolution machinery becauseParamLengthInfoTypeis a diag coded type and it needs to resolve the reference to the length key parameter.Andreas Lauser <[email protected]>, on behalf of MBition GmbH.
Provider Information