dbc paring, message._fmt with wrong padding about cantools HOT 14 CLOSED

from cantools.

Hi, first simple test looks good. I plan to confirm and provide a test case on Monday.

from cantools.

Hello,

this is the right place to file issues. I don't know all details about the DBC format myself, and I'm happy you pointed out that there is a problem in the package.

The 0 after the @ is the byte order of the signal. Looks like big endian byte order format is incorrect indeed, the number of bits in the format should be max 64.

The problem is probably not in _create_message_encode_decode_format(), but that the start bit in big endian signals should be converted somehow to the internal represenation of the frame data. I don't quite understand how to perform this conversion, and I don't have any good tools available as guidance, so it would be lovely if you can provide more information about the expected format string.

I made some changes on a branch that gives 64 bits in the format string (p16u15u1p16u15u1), but I'm not sure it is correct. B is mapped to bit 0, and A to bit 1, which does not match your Vector output. Anyway, the problem you highlight in this issue should be fixed, we just have to figure out how.

from cantools.

Hi,
not the fields length seems right.
But the interpretation of the bit-order of the signals A and C is wrong.

Attached is a view from the layout editor.
As said, vectors bit counting is unusual and so there is a second picture with bit index inverted which seems to be the natural counting for me:
-signal B is bit 0 of byte 0
-signal A is from bit 1 of byte 0 to bit 15 of byte 1
-concerning the bit order of A, bit 1 of byte0 is the MSB (even if it is called Motorola byte order).

Does that make sense?

from cantools.

I made another attempt on the same branch with the bit ordering reversed in big endian signals, but it's not clear to me if it is correct. I can imagine it's not as simple as to reverse the bit order, but probably depends on byte boundaries as well.

Can you provide the frame data on the CAN bus sent by the Vector tool, and its decoded signal values?

from cantools.

Hi,
unfortunately it appears to still not match the Vector format. I added a test case in a fork, but did not change the implementation.
Commit 5f14cff

I'm still new to github. This is the way to add test code from my side, right?

from cantools.

Yeah, a fork is usually how you share code.

I changed the code to make your test pass. The rest of the tests fails, but that's fine for now. Let me know if the fix solves your problem. The bit/byte ordering was pretty much a guess from my side from the beginning, so it wouldn't surprise me if all signals were mixed up in the currently release version. So any changes we make are probably to the better.

Will be very nice when the logic is correct for all configurations, just have to find reference DBC-files and CAN frame data, which I lack atm. =)

from cantools.

Hi,
now my signals, including signals of several other messages, are decoded correctly :-)

So which type of message signals are now expected to fail?
Most of my signals are of form @+0 and those appear to pass.

from cantools.

Nice, can you add another testcase, similar to what you already added, to make me even more confident I don't break anything in the future?

I'm not sure actually. I expect @1 to fail since I have no reference data of that kind, and usually it's hard to make a correct implementation by guessing and misinterpreting specifications. =)

I found https://se.mathworks.com/help/vnt/ug/canpack.html, maybe it can help me out. Looks neat, but it does not contain reference DBC file and CAN frames. =/

from cantools.

Did additional changes to make little endian byte order work (at least I think so). The big endian byte order still pass, but encoded frame data is padded with two zero bytes to a total of 8 bytes because it simplified the implementation. Cannot tell if padding is expected by the user, so I'll leave it like this until someone have a good argument for changing it.

https://github.com/eerimoq/cantools/commits/issue-5

If encoding and decoding works with this implementation I'll update the rest of the code to work as well (database dump to DBC for example).

from cantools.

I created a new release, 9.0.0, and uploaded it to PyPi.

Thanks for the help, and let me know if there are more problems. Pull requests are very welcome!

from cantools.

Reopen this issue or create a new one if the issue is still present. Thanks for the help!

from cantools.

Hi, issue appears to be solved. With 9.0.0 I did not see any problem in a dbc with various signals of different byte order. Thank you for fixing!

from cantools.

No problem, always satisfying improving the code quality and making users happy =)

from cantools.