c-coderdbc
c-coderdbc copied to clipboard
Fix multiplex signal Pack functions
Hello,
The generation of the Pack functions does not correctly pack CAN messages that contain mutliplex signals and instead packs all the signals in the CAN message regardless of wether they are multiplex values and what the multiplex master signals value is.
This creates a bug where when a user tries to call the generated Pack_ function with a set of signal values set corresponding to a particular master multiplexor signal set then other multiplex signals that where not part of the set will overwrite these values.
Let me show an example:
Below is a .dbc defining an example CAN message with multiplex signals and a multiplex signal master
BO_ 1096 MUX_SIG_TEST1: 8 MCU
SG_ mux8_sig1 m8 : 8|8@1+ (1,0) [1|3] "" VMU
SG_ mux8_sig2 m8 : 16|15@1+ (0.125,0) [0|4090] "V" VMU
SG_ mux7_sig1 m7 : 8|8@1+ (1,0) [1|3] "" VMU
SG_ mux7_sig2 m7 : 16|15@1+ (0.125,0) [0|4090] "V" VMU
SG_ mux6_sig1 m6 : 8|48@1+ (1,0) [0|0] "" VMU
SG_ mux0_sig1 m0 : 32|8@1+ (1,0) [0|0] "" VMU
SG_ mux0_sig2 m0 : 24|8@1+ (1,0) [0|0] "" VMU
SG_ mux5_sig1 m5 : 8|48@1+ (1,0) [0|0] "" VMU
SG_ mux4_sig4 m4 : 8|32@1+ (1,0) [0|0] "" VMU
SG_ mux4_sig3 m3 : 24|16@1+ (1,0) [0|0] "" VMU
SG_ mux0_sig3 m0 : 16|8@1+ (1,0) [0|0] "" VMU
SG_ mux0_sig4 m0 : 8|8@1+ (1,0) [0|0] "" VMU
SG_ mux2_sig1 m2 : 8|16@1+ (1,-32767) [0|32760] "RPM" VMU
SG_ mux1_sig1 m1 : 8|15@1+ (0.125,0) [0|4090] "Nm" VMU
SG_ mux3_sig1 m3 : 16|8@1+ (1,0) [0|0] "" VMU
SG_ mux3_sig2 m3 : 8|8@1+ (1,0) [0|0] "" VMU
SG_ mux_master M : 0|7@1+ (1,0) [0|0] "" VMU
SG_ signal1 : 7|1@1+ (1,0) [0|0] "" VMU
Calling the coderdbc tool generates the following Pack function for this CAN message in the output testdb.c file
./build/coderdbc -dbc test/testdb2.dbc -out test2/gencode -drvname testdb -nodeutils -rw
generates this in the output file gencode/lib/testdb.c:
uint32_t Pack_MUX_SIG_TEST1_testdb(MUX_SIG_TEST1_t* _m, uint8_t* _d, uint8_t* _len, uint8_t* _ide)
{
uint8_t i; for (i = 0u; i < TESTDB_VALIDATE_DLC(MUX_SIG_TEST1_DLC); _d[i++] = TESTDB_INITIAL_BYTE_VALUE);
#ifdef TESTDB_USE_SIGFLOAT
_m->mux1_sig1_ro = (uint16_t) TESTDB_mux1_sig1_ro_toS(_m->mux1_sig1_phys);
_m->mux2_sig1_ro = (uint16_t) TESTDB_mux2_sig1_ro_toS(_m->mux2_sig1_phys);
_m->mux8_sig2_ro = (uint16_t) TESTDB_mux8_sig2_ro_toS(_m->mux8_sig2_phys);
_m->mux7_sig2_ro = (uint16_t) TESTDB_mux7_sig2_ro_toS(_m->mux7_sig2_phys);
#endif // TESTDB_USE_SIGFLOAT
_d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
_d[1] |= (uint8_t) ( (_m->mux8_sig1 & (0xFFU)) | (_m->mux7_sig1 & (0xFFU)) | (_m->mux6_sig1 & (0xFFU)) | (_m->mux3_sig2 & (0xFFU)) | (_m->mux5_sig1 & (0xFFU)) | (_m->mux4_sig4 & (0xFFU)) | (_m->mux1_sig1_ro & (0xFFU)) | (_m->mux0_sig4 & (0xFFU)) | (_m->mux2_sig1_ro & (0xFFU)) );
_d[2] |= (uint8_t) ( ((_m->mux6_sig1 >> 8U) & (0xFFU)) | ((_m->mux5_sig1 >> 8U) & (0xFFU)) | ((_m->mux4_sig4 >> 8U) & (0xFFU)) | ((_m->mux1_sig1_ro >> 8U) & (0x7FU)) | ((_m->mux2_sig1_ro >> 8U) & (0xFFU)) | (_m->mux8_sig2_ro & (0xFFU)) | (_m->mux3_sig1 & (0xFFU)) | (_m->mux0_sig3 & (0xFFU)) | (_m->mux7_sig2_ro & (0xFFU)) );
_d[3] |= (uint8_t) ( ((_m->mux6_sig1 >> 16U) & (0xFFU)) | ((_m->mux5_sig1 >> 16U) & (0xFFU)) | ((_m->mux4_sig4 >> 16U) & (0xFFU)) | ((_m->mux8_sig2_ro >> 8U) & (0x7FU)) | ((_m->mux7_sig2_ro >> 8U) & (0x7FU)) | (_m->mux4_sig3 & (0xFFU)) | (_m->mux0_sig2 & (0xFFU)) );
_d[4] |= (uint8_t) ( ((_m->mux6_sig1 >> 24U) & (0xFFU)) | ((_m->mux5_sig1 >> 24U) & (0xFFU)) | ((_m->mux4_sig4 >> 24U) & (0xFFU)) | ((_m->mux4_sig3 >> 8U) & (0xFFU)) | (_m->mux0_sig1 & (0xFFU)) );
_d[5] |= (uint8_t) ( ((_m->mux6_sig1 >> 32U) & (0xFFU)) | ((_m->mux5_sig1 >> 32U) & (0xFFU)) );
_d[6] |= (uint8_t) ( ((_m->mux6_sig1 >> 40U) & (0xFFU)) | ((_m->mux5_sig1 >> 40U) & (0xFFU)) );
*_len = (uint8_t) MUX_SIG_TEST1_DLC;
*_ide = (uint8_t) MUX_SIG_TEST1_IDE;
return MUX_SIG_TEST1_CANID;
}
The above code is packing all the multiplex signals for each byte regardless of the master signal mux_master value! This problem becomes more apparent when you also enable the TESTDB_USE_SIGFLOAT section in the above code and it runs for example the
_m->mux2_sig1_ro = (uint16_t) TESTDB_mux2_sig1_ro_toS(_m->mux2_sig1_phys);
in the above code. This signal due to its -32767 offset will now overwrite all the other signals in the _d[2] (byte 2) of the CAN message regardless of what the multiplex master signal value is. If I want to only send the signals associated with the mux master value of 1 well I can't as the above mux2_sig1_ro value will overwrite the contents of byte2.