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

Fix multiplex signal Pack functions #31

Open
calumroy opened this issue Aug 7, 2024 · 1 comment
Open

Fix multiplex signal Pack functions #31

calumroy opened this issue Aug 7, 2024 · 1 comment

Comments

@calumroy
Copy link

calumroy commented Aug 7, 2024

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.

@calumroy
Copy link
Author

calumroy commented Aug 7, 2024

See the above pull request for a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant