Error generating multiplexed signals

[andresv]

@marcelbuesing I tried multiplexed signal parsing on one of the opendbc files and got some errors.

Offending message: https://github.com/commaai/opendbc/blob/master/hyundai_kia_generic.dbc#L1061-L1080

There seems to be problem with floats:

40322 |     pub const CAN_VERS_MAX: u8 = 7.7_u8;
      |                                  ^^^^^^ invalid suffix `u8`

Also barks about this signal: https://github.com/commaai/opendbc/blob/master/hyundai_kia_generic.dbc#L476

17469 |         match self.lv_gsl_map_raw() {
      |               --------------------- this expression has type `bool`
17470 |             0 => Ok(Ems13LvGslMap::M0(Ems13LvGslMapM0{ raw: self.raw })),
17471 |             1 => Ok(Ems13LvGslMap::M1(Ems13LvGslMapM1{ raw: self.raw })),
      |             ^ expected `bool`, found integer

PS: this file also needs this patch: https://github.com/technocreatives/dbc-codegen/pull/41

[andresv]

As far as I know fractional M should not be used: https://github.com/commaai/opendbc/blob/master/hyundai_kia_generic.dbc#L476

[andresv]

This should be a valid message:

 BO_ 200 MultiplexTest: 8 SENSOR
  SG_ Multiplexor M : 0|4@1+ (1,0) [0|2] "" Vector__XXX
  SG_ UnmultiplexedSignal : 4|8@1+ (1,0) [0|4] "" Vector__XXX
- SG_ MultiplexedSignalZeroA m0 : 12|8@1+ (0.1,0) [0|3] "" Vector__XXX
+ SG_ MultiplexedSignalZeroA m0 : 12|8@1+ (1.0,0.0) [0.0|3.0] "" Vector__XXX
  SG_ MultiplexedSignalZeroB m0 : 20|8@1+ (0.1,0) [0|3] "" Vector__XXX
- SG_ MultiplexedSignalOneA m1 : 12|8@1+ (0.1,0) [0|6] "" Vector__XXX
+ SG_ MultiplexedSignalOneA m1 : 12|8@1+ (1.0,0.0) [0.0|6.0] "" Vector__XXX
  SG_ MultiplexedSignalOneB m1 : 20|8@1+ (0.1,0) [0|6] "" Vector__XXX

But this is treated as u8:

   |
65 |     m0.set_multiplexed_signal_zero_a(1.2).unwrap();
   |                                      ^^^ expected `u8`, found floating-point number

Here is a good overview about generated types:

    pub const MULTIPLEXOR_MIN: u8 = 0_u8;
    pub const MULTIPLEXOR_MAX: u8 = 2_u8;
    pub const UNMULTIPLEXED_SIGNAL_MIN: u8 = 0_u8;
    pub const UNMULTIPLEXED_SIGNAL_MAX: u8 = 4_u8;
    pub const MULTIPLEXED_SIGNAL_ZERO_A_MIN: u8 = 0_u8;
    pub const MULTIPLEXED_SIGNAL_ZERO_A_MAX: u8 = 3_u8;
    pub const MULTIPLEXED_SIGNAL_ZERO_B_MIN: f32 = 0_f32;
    pub const MULTIPLEXED_SIGNAL_ZERO_B_MAX: f32 = 3_f32;
    pub const MULTIPLEXED_SIGNAL_ONE_A_MIN: u8 = 0_u8;
    pub const MULTIPLEXED_SIGNAL_ONE_A_MAX: u8 = 6_u8;
    pub const MULTIPLEXED_SIGNAL_ONE_B_MIN: f32 = 0_f32;
    pub const MULTIPLEXED_SIGNAL_ONE_B_MAX: f32 = 6_f32;

[andresv]

Okay so parser always uses u8 in case of 8@1+ if scaling is 1 or 1.0 and if scaling is something different like 0.1 or 2.0 then it uses f32.

[andresv]

Original issue about floats came from such signal:

SG_ CAN_VERS m0 : 0|6@1+ (1.0,0.0) [0.0|7.7] ""  _4WD,ABS,ESC,IBOX

So this signal itself must be wrong, max cannot be 7.7.

[andresv]

However this seems to be a valid signal to define multiplexer M:

SG_ LV_GSL_MAP M : 4|1@1+ (1.0,0.0) [0.0|1.0] ""  LPI

So it seems M is treated as bool, but match treats it as a number:

17469 |         match self.lv_gsl_map_raw() {
      |               --------------------- this expression has type `bool`
17470 |             0 => Ok(Ems13LvGslMap::M0(Ems13LvGslMapM0{ raw: self.raw })),
17471 |             1 => Ok(Ems13LvGslMap::M1(Ems13LvGslMapM1{ raw: self.raw })),
      |             ^ expected `bool`, found integer

https://github.com/technocreatives/dbc-codegen/blob/main/src/lib.rs#L550

[andresv]

I tried to fix it in code generation side, it is doable, however it seems that maybe it would be better to treat multiplexer signal always as u8 etc and never as bool on can-dbc side. What do you think @marcelbuesing?

[marcelbuesing]

Yes I agree it's probably better to basically make sure multiplexor fns in this case lv_gsl_map_raw always return an u8 or maybe better u16.

[marcelbuesing]

Maybe just a

fn signal_to_rust_type(signal: &Signal) -> String {
    if signal.signal_size == 1 && *signal.multiplexer_indicator() != MultiplexIndicator::Multiplexor {
        String::from("bool")
    } else if signal_is_float_in_rust(signal) {
        // If there is any scaling needed, go for float
        String::from("f32")
    } else {
        signal_to_rust_int(signal)
    }
}