Skip to content

Fix multiplex sig pack #32

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix multiplex sig pack #32

wants to merge 8 commits into from

Conversation

calumroy
Copy link

@calumroy calumroy commented Aug 7, 2024 •
edited
Loading

I fixed the signal packing for multiplex signals, issue #31

The new generated code takes into account the master multiplex value when generating the Pack functions.
I added an example dbc file test2.dbc that highlights the problem.
One of the messages in this dbc file is:

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

Here is a comparison of the generated code before this fix for the Pack function of the MUX_SIG_TEST1 CAN message from the test2.dbc file.

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;
}

And now below is the generated code after this fix.
CAN messages with no multiplex values will generate the original style Pack functions this change only affects messages that contain multiplex signals.
Note1: For _d[0] the signals mux_master and signal1 are not affected by the value of mux_master so they appear in each possible multiplex master if statement. The other bytes d[1] to d[6] contain only the mux signals that are associated with each mux_master value.

Note2: For the case where the mux_master has a value that doesn't equal any of the expected multiplex signal values (e.g the last else statement for each byte) just pack all the multiplex signals for each byte as none of them are valid anyway. We could have tried only packing the master multiplexor and non multiplexed signals for this case but I don't think this would matter since all multiplex signal values would not be valid anyway for this case.

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

  if (_m->mux_master == 8) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 7) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 6) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 3) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 5) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 4) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 1) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 0) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else if (_m->mux_master == 2) {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  else {
    _d[0] |= (uint8_t) ( (_m->mux_master & (0x7FU)) | ((_m->signal1 & (0x01U)) << 7U) );
  }
  if (_m->mux_master == 8) {
    _d[1] |= (uint8_t) ( (_m->mux8_sig1 & (0xFFU)) );
  }
  else if (_m->mux_master == 7) {
    _d[1] |= (uint8_t) ( (_m->mux7_sig1 & (0xFFU)) );
  }
  else if (_m->mux_master == 6) {
    _d[1] |= (uint8_t) ( (_m->mux6_sig1 & (0xFFU)) );
  }
  else if (_m->mux_master == 3) {
    _d[1] |= (uint8_t) ( (_m->mux3_sig2 & (0xFFU)) );
  }
  else if (_m->mux_master == 5) {
    _d[1] |= (uint8_t) ( (_m->mux5_sig1 & (0xFFU)) );
  }
  else if (_m->mux_master == 4) {
    _d[1] |= (uint8_t) ( (_m->mux4_sig4 & (0xFFU)) );
  }
  else if (_m->mux_master == 1) {
    _d[1] |= (uint8_t) ( (_m->mux1_sig1_ro & (0xFFU)) );
  }
  else if (_m->mux_master == 0) {
    _d[1] |= (uint8_t) ( (_m->mux0_sig4 & (0xFFU)) );
  }
  else if (_m->mux_master == 2) {
    _d[1] |= (uint8_t) ( (_m->mux2_sig1_ro & (0xFFU)) );
  }
  else {
    _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)) );
  }
  if (_m->mux_master == 8) {
    _d[2] |= (uint8_t) ( (_m->mux8_sig2_ro & (0xFFU)) );
  }
  else if (_m->mux_master == 7) {
    _d[2] |= (uint8_t) ( (_m->mux7_sig2_ro & (0xFFU)) );
  }
  else if (_m->mux_master == 6) {
    _d[2] |= (uint8_t) ( ((_m->mux6_sig1 >> 8U) & (0xFFU)) );
  }
  else if (_m->mux_master == 3) {
    _d[2] |= (uint8_t) ( (_m->mux3_sig1 & (0xFFU)) );
  }
  else if (_m->mux_master == 5) {
    _d[2] |= (uint8_t) ( ((_m->mux5_sig1 >> 8U) & (0xFFU)) );
  }
  else if (_m->mux_master == 4) {
    _d[2] |= (uint8_t) ( ((_m->mux4_sig4 >> 8U) & (0xFFU)) );
  }
  else if (_m->mux_master == 1) {
    _d[2] |= (uint8_t) ( ((_m->mux1_sig1_ro >> 8U) & (0x7FU)) );
  }
  else if (_m->mux_master == 0) {
    _d[2] |= (uint8_t) ( (_m->mux0_sig3 & (0xFFU)) );
  }
  else if (_m->mux_master == 2) {
    _d[2] |= (uint8_t) ( ((_m->mux2_sig1_ro >> 8U) & (0xFFU)) );
  }
  else {
    _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)) );
  }
  if (_m->mux_master == 8) {
    _d[3] |= (uint8_t) ( ((_m->mux8_sig2_ro >> 8U) & (0x7FU)) );
  }
  else if (_m->mux_master == 7) {
    _d[3] |= (uint8_t) ( ((_m->mux7_sig2_ro >> 8U) & (0x7FU)) );
  }
  else if (_m->mux_master == 6) {
    _d[3] |= (uint8_t) ( ((_m->mux6_sig1 >> 16U) & (0xFFU)) );
  }
  else if (_m->mux_master == 3) {
    _d[3] |= (uint8_t) ( (_m->mux4_sig3 & (0xFFU)) );
  }
  else if (_m->mux_master == 5) {
    _d[3] |= (uint8_t) ( ((_m->mux5_sig1 >> 16U) & (0xFFU)) );
  }
  else if (_m->mux_master == 4) {
    _d[3] |= (uint8_t) ( ((_m->mux4_sig4 >> 16U) & (0xFFU)) );
  }
  else if (_m->mux_master == 0) {
    _d[3] |= (uint8_t) ( (_m->mux0_sig2 & (0xFFU)) );
  }
  else {
    _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)) );
  }
  if (_m->mux_master == 6) {
    _d[4] |= (uint8_t) ( ((_m->mux6_sig1 >> 24U) & (0xFFU)) );
  }
  else if (_m->mux_master == 3) {
    _d[4] |= (uint8_t) ( ((_m->mux4_sig3 >> 8U) & (0xFFU)) );
  }
  else if (_m->mux_master == 5) {
    _d[4] |= (uint8_t) ( ((_m->mux5_sig1 >> 24U) & (0xFFU)) );
  }
  else if (_m->mux_master == 4) {
    _d[4] |= (uint8_t) ( ((_m->mux4_sig4 >> 24U) & (0xFFU)) );
  }
  else if (_m->mux_master == 0) {
    _d[4] |= (uint8_t) ( (_m->mux0_sig1 & (0xFFU)) );
  }
  else {
    _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)) );
  }
  if (_m->mux_master == 6) {
    _d[5] |= (uint8_t) ( ((_m->mux6_sig1 >> 32U) & (0xFFU)) );
  }
  else if (_m->mux_master == 5) {
    _d[5] |= (uint8_t) ( ((_m->mux5_sig1 >> 32U) & (0xFFU)) );
  }
  else {
    _d[5] |= (uint8_t) ( ((_m->mux6_sig1 >> 32U) & (0xFFU)) | ((_m->mux5_sig1 >> 32U) & (0xFFU)) );
  }
  if (_m->mux_master == 6) {
    _d[6] |= (uint8_t) ( ((_m->mux6_sig1 >> 40U) & (0xFFU)) );
  }
  else if (_m->mux_master == 5) {
    _d[6] |= (uint8_t) ( ((_m->mux5_sig1 >> 40U) & (0xFFU)) );
  }
  else {
    _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;
}

calumroy added 8 commits August 6, 2024 18:34
@calumroy
WIP: testing to fix multiplex signal pack functiins.
9b3e845
@calumroy
Fixed case where only the master multiplexor exists and no other mux …
ca43dbc 
…signals are in a CAN msg. Filter out bytes that have no signals in them.
@calumroy
Revert a small change to formatting.
fbcc741
@calumroy
Added example dbc that shows the issues with multiplex signals.
0e8b893
@calumroy
Revert changes to the CMakeLists.txt
69eda88
@calumroy
Merge branch 'fix_multiplex_sig_pack'
34d7cd6
@calumroy
Fixed the case where the multiplex master value does not equal any of…
25671de 
… the expected multiplex values. In this case just encode all signals for the byte since none of them are technically valid as they don't correspond to the master multiplexor value.
@calumroy
Merge commit '25671de74921c66c1c96253f116725caab680384' into fix_mult…
b2e5a95 
…iplex_sig_pack
@calumroy
Copy link
Author

calumroy commented Aug 15, 2024 •
edited
Loading

Just use #34 pull request which includes this pull requestr and a fix for #33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@calumroy