hardbyte / python-can

The can package provides controller area network support for Python developers
https://python-can.readthedocs.io
GNU Lesser General Public License v3.0
1.31k stars 604 forks source link

BLF PDU is not strictly padding to 4N. #459

Closed dutchhuang closed 5 years ago

dutchhuang commented 6 years ago

I was using the library to parse some CAN data files (in BLF format) that I recorded with CANoe 11.0.42 - 64bit.

And the library reports a same 'LOBJ' header check exception for most files, and then I traced the error a bit, and found out that the error was actually caused by a wrong offset calculation for next PDU (thus the header wasn't well recognized).

Here is the header info print out:

(b'LOBJ', 32, 1, 197, 65) (b'LOBJ', 32, 1, 205, 65) (b'LOBJ', 32, 1, 216, 65) (b'LOBJ', 32, 1, 538, 65) (b'LOBJ', 32, 1, 1043, 65) (b'LOBJ', 32, 1, 335, 65) (b'LOBJ', 32, 1, 104, 48) (b'LOBJ', 32, 1, 104, 48) (b'LOBJ', 32, 1, 104, 48) (b'LOBJ', 32, 1, 104, 48) (b'LOBJ', 32, 1, 104, 48) (b'LOBJ', 32, 1, 96, 49) (b'LOBJ', 32, 1, 96, 49) (b'LOBJ', 32, 1, 96, 49) (b'LOBJ', 32, 1, 96, 49) (b'LOBJ', 32, 1, 104, 48) (b'LOBJ', 32, 1, 96, 49) (b'LOBJ', 32, 1, 40, 31) (b'LOBJ', 32, 1, 40, 31) (b'LOBJ', 32, 1, 96, 49) (b'LOBJ', 32, 1, 116, 66) (b'LOBJ', 32, 1, 134, 66) (b'BJ \x00', 1, 134, 4325376, 131072) Traceback (most recent call last): ... File "C:\Python\Python36-32\lib\site-packages\can\io\blf.py", line 177, in iter raise BLFParseError() can.io.blf.BLFParseError

I checked the raw data, and found out that when the 'size' in the 'object header' is '134', the real data size is also '134', so the real PDU is not padded to 4N length, thus next_pos = pos + obj_size + (obj_size % 4) won't work for this case.

Does anyone know the reason behind? Any quick solution or suggestion that? I have a quick and dirty hack to that, which is to calculate 2 possible next_pos and retry when header check is failed, but I mean it doesn't answer the question...

christiansandberg commented 6 years ago

I just copied the implementation from the C++ library (https://bitbucket.org/tobylorenz/vector_blf) and the BUSMASTER implementation. And it seems to be needed for the other objects have a size that is not a multiple of 4. To me it is really weird though because it will not result in a correct padding (should be 4 - obj_size % 4) right?).

One way could be to search for the LOBJ signature instead of calculating where it is expected to be.

christiansandberg commented 6 years ago

I just found the specification for the BLF format in the installation! I'll take a look at it when I have time.

christiansandberg commented 6 years ago

It did not provide much information. Looking closer at the C++ library it seems different object types have different padding strategies. Number 66 (FlexRayVFrReceiveMsgEx) for example is different.

    uint8_t padding = (dataCount + 4) % 8;
    if (padding != 0) {
        is.seekg((8 - padding), std::ios_base::cur);
    }
dutchhuang commented 6 years ago

I just copied the implementation from the C++ library (https://bitbucket.org/tobylorenz/vector_blf) and the BUSMASTER implementation. And it seems to be needed for the other objects have a size that is not a multiple of 4. To me it is really weird though because it will not result in a correct padding (should be 4 - obj_size % 4) right?).

One way could be to search for the LOBJ signature instead of calculating where it is expected to be.

4 - obj_size %4 doesn't look like a correct formula as well, because I found one PDU with header (b'LOBJ', 32, 1, 1043, 65), and this PDU was padded with 3 00 bytes, which makes the original formula in the code correct version.

dutchhuang commented 6 years ago

It did not provide much information. Looking closer at the C++ library it seems different object types have different padding strategies. Number 66 (FlexRayVFrReceiveMsgEx) for example is different.

    uint8_t padding = (dataCount + 4) % 8;
    if (padding != 0) {
        is.seekg((8 - padding), std::ios_base::cur);
    }

Ahh.. You are absolutely right..

I just checked the object type, and all error pos are generated when obj_type is 66, which is a FR_RCVMESSAGE_EX, which then reminds me that I recorded CAN and Flexray in the same bag for most of the time.

Thanks for the hint! And sorry for the confusion :)

felixdivo commented 5 years ago

Is someone going to have a look at this?