fix: support large min/max values without precision loss

[trnila]

Min/max values are no longer represented as f64 internally that caused precision loss or value that couldnt fit in u64.

For example 2**64 was represented as 18446744073709552000 instead of 18446744073709551615.

[codecov]

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/lib.rs	83.33%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[nyurik]

I did a bunch of small changes - see what you think. I still feel i don't have enough domain expertise to make judgement calls on what it actually should do - i.e. if multiplexor is i8 - what should the behavior be? Currently I force it to u16::MAX - but is that correct? Still better than some random value due to casting.

[nyurik]

Domain expert advise is needed: What should be the DBC multiplexer behavior if the min/max values are signed or floating point values?

[trnila]

Thanks @nyurik - changes looks fine although I haven't personally used muxed messages deeply.
I wouldnt expect muxed value to be floating point as it would be hard to compare these values. I have checked dbc examples from cantools and havent seen any negative values - just smaller numbers. Ideally these values should be enumerators.

[nyurik]

The issue is that we must define our behavior for all of these - and it doesn'tt matter if we have seen these or not:

pub enum NumericValue {
    Uint(u64),
    Int(i64),
    Double(f64),
}

in other words, it could be totally ok to generate an error if the user used Int or Double - which would avoid the problem completely - if that's the valid behavior. I am just not certain we should automatically fallback to u16::MAX if the user gave -1 as multiplexor - but I'm not an expert in this, so no idea what is "right"

[trnila]

@nyurik Multiplex values (m<value>) defined in dbc should be unsigned integers only.

candb++ editor behaviour for setting multiplex value:

• unsigned integers are saved
• negative numbers are converted as 2's complement (-5 => 0xFFFFFFFB)
• any floating point is stored as zero

Currently, we are selecting multiplexed signals based on physical value (after applying linear conversion):

dbc-codegen/tests-snapshots/dbc-cantools/multiplex.snap.rs

Line 96 in 1cac7e3

match self.multiplexor_raw() {

Is this correct? Physical values might be negative or floating and cannot be compared with unsigned multiplex value m<value>? Shouldnt we rather compare directly with raw extracted bits? Its not caught in any test, as all multiplexed signals are having identity linear conversion.

codegen ${signal}_raw functions might be little bit confusing as they do the linear conversion and return primitive datatype.
DBC is using the following:

• raw value are extracted bits after endianity conversion
• physical value is applied linear conversion on raw value physical_value = raw_value * factor + offset

[nyurik]

Should any of the values/types result in an error? A failure to generate is a perfectly valid result in some cases

[trnila]

I think there shouldnt be any error in dbc-codegen.

can-dbc will parse only unsigned number u64 in multiplex value (m<value>):
https://github.com/oxibus/can-dbc/blob/1e0299bdb6d503954aee3ccd7c67829155129c7b/src/ast/multiplex_indicator.rs#L32-L37

And this value could be compared to raw unsigned value of multiplexor signal like:

self.raw.view_bits::<Lsb0>()[2..8].load_le::<u64>();

We are now comparing the same things, so there shouldnt be any issues, if i am not mistaken.