ebroecker / canmatrix

Converting Can (Controller Area Network) Database Formats .arxml .dbc .dbf .kcd ...
BSD 2-Clause "Simplified" License
907 stars 400 forks source link

startbit of motorola coded signal may be inconsistent #19

Closed ebroecker closed 8 years ago

ebroecker commented 8 years ago

The startbit of motorola coded signals are coded a bit strange in 'dbc'. Canmatrix internally uses this (dbc-like) startbit. Sym: Im/exportsym converts this correct. semms that kcd and dbf currently is converted wrong:

But I'm not sure about other arxml. In excel it would be much better to use a 'better readable' startbit.

==> Conclusion: Strange dbc startbits should be handled in dbc-modules and not in sym, dbf, kcd; internal representation should be human readable and not dbc-like.

EDIT: kcd does need converted startbits: https://github.com/julietkilo/CANBabel/blob/master/src/main/java/com/github/canbabel/canio/dbc/DbcReader.java: /* * Calculates the least significant bit offset for big endian byte order * @param msb Most significant bit offset * @param length signal length in bit * @return lsb Least significant bit offset / public static int bigEndianLeastSignificantBitOffset(int msb, int length){ int lsb; int pos; int cpos; int bytes; pos = 7 - (msb % 8) + (length - 1); if (pos < 8){ / msb pass a byte order / lsb = msb - length + 1; } else { cpos = 7 - (pos % 8); bytes = pos / 8; lsb = cpos + (bytes * 8) + (msb/8) * 8; } return lsb; }

EDIT2: dbf does also some conversion (https://github.com/rbei-etas/busmaster/blob/master/Sources/Format%20Converter/DBC2DBFConverterLibrary/DBFSignal.cpp):

if(SIG_DF_MOTOROLA == m_ucDataFormat) { //The following is done because if signal type is of MOTOROLA //THe MSB is stored and will be in Big Endian format. //Get the LSB /In CANoe if we view the bits as a 8x8 Matrix, for Motorala signals the numbering will be in the reverse order with in the each row. So to get the proper MSB we need to again swap the bit position with in the row/ //i.e here we need to get the left half of the bits to the right and right //to the left with in the same row int ntemp; if(uiStartBit == 0) { ntemp = 0; } else { ntemp = uiStartBit % 4; //get the bit position with its the row. } //swap left bits to the right and right bits to the left. if(uiStartBit % 8 <= 3) { uiStartBit = uiStartBit + 7 - ntemp * 2; } else { uiStartBit = uiStartBit - 1 - ntemp * 2; } uiStartBit = uiStartBit + m_ucLength-1; //Invert the bit position (Converting to little endian format) uiStartBitX = (CDBFConverter::ucMsg_DLC*8)-1 - uiStartBit; }

ebroecker commented 8 years ago

should be fixed with the merge of "byteorder"-branch