Properly support decoding NRC-CONST parameters #314
Conversation
andlaus
force-pushed
the
nrc_const_coding
branch
from
0a9b340 to
943ce0d
Compare
if a negative response contains an NRC-CONST parameter, where the parameter's value is not in the list of expected parameters, the response is not applicable and decoding should be stopped at this point. (Thus, NRC-CONST parameters are a multiplexer mechanism which filters response objects based on the value of some other parameter.) This patch implements this by introducing a new `DecodeMismatch` exception which is raised whenever a NRC-CONST parameter which exhibits an unexpected value is encountered. The higher level decoding routines for decoding responses in `DiagLayer` then ignore these exceptions. (IMO, this solution is a bit sub-optimal because it uses exceptions for flow control in normal operation, but alternative implmentations would be far more complex.) Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Katja Köhler <katja.koehler@mbition.io>
andlaus
force-pushed
the
nrc_const_coding
branch
from
943ce0d to
d65660d
Compare
| DecodeError, | ||
| stacklevel=1, | ||
| ) | ||
| raise DecodeMismatch(f"NRC-CONST parameter {self.short_name} does not apply") |
There was a problem hiding this comment.
suggestion: change to warning, use a warning filter in odxtools/diagservice.py to force that warning as error
There was a problem hiding this comment.
this is not really possible: we want to abort the decoding process for the whole response even regardless of whether we are in strict mode or not. This is because if this happens, the response is not applicable. Since this is IMO normal processing and not really exceptional circumstances, I'm not really happy about using exceptions for flow control. That said, any other solution I can think of is at least one order of magnitude more invasive and error-prone...
There was a problem hiding this comment.
(I have to correct myself: actually, going the warnings route and making warnings raise would be possible, but it would be very convoluted and IMO it also would defeat the purpose. In particular, because these conditions can happen during normal operation, i.e. it is not a circumstance which one needs/wants be warned about.)
thanks to [at]kayoub5 for the catch! Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Katja Köhler <katja.koehler@mbition.io>
this is a drive-by fix, but it is pretty trivial... Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Katja Köhler <katja.koehler@mbition.io>
|
@kayoub5: do you consider this to be merge-ready with the latest modifications? |
|
@kayoub5: barring objections or approvals, I will merge this PR this Thursday or Friday... |
if a negative response contains an NRC-CONST parameter, where the parameter's value is not in the list of expected values, the response is not applicable and decoding should be stopped at this point. (Thus, NRC-CONST parameters are a multiplexer mechanism which filters responses based on the value of some other parameter.)
This patch implements this by introducing a new
DecodeMismatchexception which is raised whenever a NRC-CONST parameter which exhibits an unexpected value is encountered. The higher-level routines for decoding responses then ignore these exceptions when appropriate. IMO, this solution is a bit sub-optimal because it uses exceptions for flow control in normal operation, but alternative implementations would be far more complex.Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH.
Provider Information