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

Error encode/decode frame with list value signals #119

Closed
nuts4coffee opened this issue Jul 31, 2023 · 5 comments · Fixed by nuts4coffee/ldfparser#5 or #120
Closed

Error encode/decode frame with list value signals #119

nuts4coffee opened this issue Jul 31, 2023 · 5 comments · Fixed by nuts4coffee/ldfparser#5 or #120
Labels
bug Something isn't working

Comments

@nuts4coffee
Copy link
Contributor

Describe the bug
Unable to encode/decode frame with signal of list type.

Signal in LDF file:

  BSBatt2Current: 24, {0, 0, 2}, BMS2, GWM ;

To Reproduce
Steps to reproduce the behavior:

The issue can be reproduced from running the following 2 unit tests.

def test_encode_array():
    signal = LinSignal('BattCurr', 24, [0, 0, 2])
    encoding_type = LinSignalEncodingType(
        "BattCurrCoding",
        [PhysicalValue(0, 182272, 0.00390625, -512, "A")]
    )

    signal.encoding_type = encoding_type
    frame = LinUnconditionalFrame(0x20, "LinStatus", 3, {0: signal})

    encoded = frame.encode({"BattCurr": [-511.99609375, -511.99609375, -511.99609375]})


def test_decode_array():
    signal = LinSignal('BattCurr', 24, [0, 0, 2])
    encoding_type = LinSignalEncodingType(
        "BattCurrCoding",
        [PhysicalValue(0, 182272, 0.00390625, -512, "A")]
    )

    signal.encoding_type = encoding_type
    frame = LinUnconditionalFrame(0x20, "LinStatus", 3, {0: signal})

    decoded = frame.decode(frame.encode_raw({"BattCurr": [1, 1, 1]}))

Expected behavior
Encode/decode should work without errors.

Stacktrace/Code
If applicable, add stacktrace or code segments to help explain your problem.

ldfparser\frame.py:216: in decode
    converted[signal_name] = signal.encoding_type.decode(value, signal, keep_unit)
ldfparser\encoding.py:180: in decode
    return decoder.decode(value, signal, keep_unit)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <ldfparser.encoding.PhysicalValue object at 0x0000017FF8A9B848>, value = [1, 1, 1]
signal = <ldfparser.signal.LinSignal object at 0x0000017FF8A9B188>, keep_unit = False

    def decode(self, value: int, signal: 'LinSignal', keep_unit: bool = False) -> float:
>       if value < self.phy_min or value > self.phy_max:
E       TypeError: '<' not supported between instances of 'list' and 'int'

ldfparser\encoding.py:65: TypeError

Environment:

  • OS: (e.g. Linux) Win
  • Python version: (e.g. 3.8.5) 3.7.9
  • ldfparser version: (e.g. 0.2.1) 0.20.0
Optionally include the output of 'pipdeptree --warn silence -p ldfparser'

Additional context
Add any other context about the problem here.

@c4deszes
Copy link
Owner

In your examples you're providing scalar (float) values when encoding array signals, in the standard the initial value provided to array type signals must be an integer array and all the examples there point to this limitation.

Do you have a productive example of scalars encoded/decoded into array signals? Is this something that's allowed in ISO 17987 or SAE J2602?

@c4deszes c4deszes reopened this Jul 31, 2023
@nuts4coffee
Copy link
Contributor Author

nuts4coffee commented Aug 1, 2023

In the example, the initial value is an integer array: [0, 0, 2]. The scalar values are the physical value.
I think this one can better demo the issue. If I encode a frame with default values, I cannot decode it back.

def test_encode_array():
    signal = LinSignal('BattCurr', 24, [0, 0, 2])
    encoding_type = LinSignalEncodingType(
        "BattCurrCoding",
        [PhysicalValue(0, 182272, 0.00390625, -512, "A")]
    )

    signal.encoding_type = encoding_type
    frame = LinUnconditionalFrame(0x20, "LinStatus", 3, {0: signal})

    encoded = frame.encode({})    # encoded = bytearray(b'\x00\x00\x02')
    frame.decode(encoded)           # Error

@c4deszes
Copy link
Owner

c4deszes commented Aug 2, 2023

If you do provide a value in the encode step you would also encounter errors because the PhysicalValue encoder cannot encode list types. So even a line like this encoded = frame.encode({"BattCurr": [0, 1, 2]}) will lead to an exception.

I'd be hesitant to merge #120 since we don't have an example LDF for it, the standard doesn't mention anything about arrays being encoded into scalars, only ASCII and BCD converter examples are available. There's a slight hint at it in the NCF example.

I'll check the ISO standard tomorrow to see if there's anything in there that would confirm this use case. If it does I would prefer the approach where support for array signals is done in the value converters, rather than the frame encoding algorithm.

@c4deszes
Copy link
Owner

The ISO standard that I read didn't mention anything new regarding array signals. If this is something you know exists in your LDF files then we can add support but as previously mentioned it should done in the converter that way we won't introduce any breaking changes.

@nuts4coffee
Copy link
Contributor Author

Thank you for pushing back. I realize my original implementation is quite wrong. I've proposed a new solution, please review and let me know if this one is ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants