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 sig unpack #34

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

calumroy
Copy link

@calumroy calumroy commented Aug 15, 2024

Fixes issues #33.

The generated code now only unpacks the correct multiplex signals associated with the mutliplex master value.
These changes do not affect CAN messages that have no mutliplex values. They also don't affect the decoding of unmultiplex signals or the multiplex master signal in a CAN message.

A comparison of decoding the following CAN msg below is shown:

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

Before this fix the following was generated.

uint32_t Unpack_MUX_SIG_TEST1_testdb(MUX_SIG_TEST1_t* _m, const uint8_t* _d, uint8_t dlc_)
{
 (void)dlc_;
 _m->mux_master = (uint8_t) ( (_d[0] & (0x7FU)) );
 _m->signal1 = (uint8_t) ( ((_d[0] >> 7U) & (0x01U)) );
 _m->mux8_sig1 = (uint8_t) ( (_d[1] & (0xFFU)) );
 _m->mux7_sig1 = (uint8_t) ( (_d[1] & (0xFFU)) );
 _m->mux6_sig1 = (uint64_t) ( ((uint64_t)(_d[6] & (0xFFU)) << 40U) | ((uint64_t)(_d[5] & (0xFFU)) << 32U) | ((_d[4] & (0xFFU)) << 24U) | ((_d[3] & (0xFFU)) << 16U) | ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
 _m->mux3_sig2 = (uint8_t) ( (_d[1] & (0xFFU)) );
 _m->mux5_sig1 = (uint64_t) ( ((uint64_t)(_d[6] & (0xFFU)) << 40U) | ((uint64_t)(_d[5] & (0xFFU)) << 32U) | ((_d[4] & (0xFFU)) << 24U) | ((_d[3] & (0xFFU)) << 16U) | ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
 _m->mux4_sig4 = (uint32_t) ( ((_d[4] & (0xFFU)) << 24U) | ((_d[3] & (0xFFU)) << 16U) | ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
 _m->mux1_sig1_ro = (uint16_t) ( ((_d[2] & (0x7FU)) << 8U) | (_d[1] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
 _m->mux1_sig1_phys = (sigfloat_t)(TESTDB_mux1_sig1_ro_fromS(_m->mux1_sig1_ro));
#endif // TESTDB_USE_SIGFLOAT

 _m->mux0_sig4 = (uint8_t) ( (_d[1] & (0xFFU)) );
 _m->mux2_sig1_ro = (uint16_t) ( ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
 _m->mux2_sig1_phys = (int32_t) TESTDB_mux2_sig1_ro_fromS(_m->mux2_sig1_ro);
#endif // TESTDB_USE_SIGFLOAT

 _m->mux8_sig2_ro = (uint16_t) ( ((_d[3] & (0x7FU)) << 8U) | (_d[2] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
 _m->mux8_sig2_phys = (sigfloat_t)(TESTDB_mux8_sig2_ro_fromS(_m->mux8_sig2_ro));
#endif // TESTDB_USE_SIGFLOAT

 _m->mux3_sig1 = (uint8_t) ( (_d[2] & (0xFFU)) );
 _m->mux0_sig3 = (uint8_t) ( (_d[2] & (0xFFU)) );
 _m->mux7_sig2_ro = (uint16_t) ( ((_d[3] & (0x7FU)) << 8U) | (_d[2] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
 _m->mux7_sig2_phys = (sigfloat_t)(TESTDB_mux7_sig2_ro_fromS(_m->mux7_sig2_ro));
#endif // TESTDB_USE_SIGFLOAT

 _m->mux4_sig3 = (uint16_t) ( ((_d[4] & (0xFFU)) << 8U) | (_d[3] & (0xFFU)) );
 _m->mux0_sig2 = (uint8_t) ( (_d[3] & (0xFFU)) );
 _m->mux0_sig1 = (uint8_t) ( (_d[4] & (0xFFU)) );

#ifdef TESTDB_USE_DIAG_MONITORS
 _m->mon1.dlc_error = (dlc_ < MUX_SIG_TEST1_DLC);
 _m->mon1.last_cycle = GetSystemTick();
 _m->mon1.frame_cnt++;

 FMon_MUX_SIG_TEST1_testdb(&_m->mon1, MUX_SIG_TEST1_CANID);
#endif // TESTDB_USE_DIAG_MONITORS

 return MUX_SIG_TEST1_CANID;
}

After this fix it now generates this Unpack function:

uint32_t Unpack_MUX_SIG_TEST1_testdb(MUX_SIG_TEST1_t* _m, const uint8_t* _d, uint8_t dlc_)
{
  (void)dlc_;
  _m->mux_master = (uint8_t) ( (_d[0] & (0x7FU)) );
  _m->signal1 = (uint8_t) ( ((_d[0] >> 7U) & (0x01U)) );
  if (_m->mux_master == 8) {
    _m->mux8_sig1 = (uint8_t) ( (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 7) {
    _m->mux7_sig1 = (uint8_t) ( (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 6) {
    _m->mux6_sig1 = (uint64_t) ( ((uint64_t)(_d[6] & (0xFFU)) << 40U) | ((uint64_t)(_d[5] & (0xFFU)) << 32U) | ((_d[4] & (0xFFU)) << 24U) | ((_d[3] & (0xFFU)) << 16U) | ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 3) {
    _m->mux3_sig2 = (uint8_t) ( (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 5) {
    _m->mux5_sig1 = (uint64_t) ( ((uint64_t)(_d[6] & (0xFFU)) << 40U) | ((uint64_t)(_d[5] & (0xFFU)) << 32U) | ((_d[4] & (0xFFU)) << 24U) | ((_d[3] & (0xFFU)) << 16U) | ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 4) {
    _m->mux4_sig4 = (uint32_t) ( ((_d[4] & (0xFFU)) << 24U) | ((_d[3] & (0xFFU)) << 16U) | ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 1) {
    _m->mux1_sig1_ro = (uint16_t) ( ((_d[2] & (0x7FU)) << 8U) | (_d[1] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
    _m->mux1_sig1_phys = (sigfloat_t)(TESTDB_mux1_sig1_ro_fromS(_m->mux1_sig1_ro));
#endif // TESTDB_USE_SIGFLOAT

  }
  if (_m->mux_master == 0) {
    _m->mux0_sig4 = (uint8_t) ( (_d[1] & (0xFFU)) );
  }
  if (_m->mux_master == 2) {
    _m->mux2_sig1_ro = (uint16_t) ( ((_d[2] & (0xFFU)) << 8U) | (_d[1] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
    _m->mux2_sig1_phys = (int32_t) TESTDB_mux2_sig1_ro_fromS(_m->mux2_sig1_ro);
#endif // TESTDB_USE_SIGFLOAT

  }
  if (_m->mux_master == 8) {
    _m->mux8_sig2_ro = (uint16_t) ( ((_d[3] & (0x7FU)) << 8U) | (_d[2] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
    _m->mux8_sig2_phys = (sigfloat_t)(TESTDB_mux8_sig2_ro_fromS(_m->mux8_sig2_ro));
#endif // TESTDB_USE_SIGFLOAT

  }
  if (_m->mux_master == 3) {
    _m->mux3_sig1 = (uint8_t) ( (_d[2] & (0xFFU)) );
  }
  if (_m->mux_master == 0) {
    _m->mux0_sig3 = (uint8_t) ( (_d[2] & (0xFFU)) );
  }
  if (_m->mux_master == 7) {
    _m->mux7_sig2_ro = (uint16_t) ( ((_d[3] & (0x7FU)) << 8U) | (_d[2] & (0xFFU)) );
#ifdef TESTDB_USE_SIGFLOAT
    _m->mux7_sig2_phys = (sigfloat_t)(TESTDB_mux7_sig2_ro_fromS(_m->mux7_sig2_ro));
#endif // TESTDB_USE_SIGFLOAT

  }
  if (_m->mux_master == 3) {
    _m->mux4_sig3 = (uint16_t) ( ((_d[4] & (0xFFU)) << 8U) | (_d[3] & (0xFFU)) );
  }
  if (_m->mux_master == 0) {
    _m->mux0_sig2 = (uint8_t) ( (_d[3] & (0xFFU)) );
  }
  if (_m->mux_master == 0) {
    _m->mux0_sig1 = (uint8_t) ( (_d[4] & (0xFFU)) );
  }

#ifdef TESTDB_USE_DIAG_MONITORS
  _m->mon1.dlc_error = (dlc_ < MUX_SIG_TEST1_DLC);
  _m->mon1.last_cycle = GetSystemTick();
  _m->mon1.frame_cnt++;

  FMon_MUX_SIG_TEST1_testdb(&_m->mon1, MUX_SIG_TEST1_CANID);
#endif // TESTDB_USE_DIAG_MONITORS

  return MUX_SIG_TEST1_CANID;
}

@calumroy
Copy link
Author

I noticed in this old pull request #22
you wanted a command line option to enable and disable this feature as well as a protected special macro like it is done for floating, monitoring ect.

I think a command line option is good to enable this fix #33 and #31 but I don't think adding a protected special macro would really be necessary. These changes in my opinion are to fix a bug so it would be weird to have generated c code that has the incorrect multiplex signal Pack and Unpack code and then a protected special macro around this correct signal Pack and Unpack code. I think it would be clearer to either generate one way or the other based on a command line option you give c-coderdbc?

… on multiplexor master signal values for multiplexed signals.

By default it is off so this new code has no affect.
@calumroy
Copy link
Author

calumroy commented Aug 15, 2024

I added the command line option -muxgen or -mutliplex. This is off by default so none of the code changes are applied to the genrated code. Turn on with for example:

./build/coderdbc -dbc test/testdb.dbc -out test/gencode -drvname testdb -nodeutils -rw -muxgen

I think this option should be on by default as it is fixes Unpack and Pack functions for multiplex signals but I guess this may break old code that upgrade c-coderdbc so maybe this is a bad idea.

… in the Unpack body for the multiplex master signals. This is fixed now.
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

Successfully merging this pull request may close these issues.

1 participant