ebroecker / canmatrix

Converting Can (Controller Area Network) Database Formats .arxml .dbc .dbf .kcd ...
BSD 2-Clause "Simplified" License
933 stars 401 forks source link

Use individual signal's "GenSigStartValue" as default value when merging dbc files #795

Closed jmailloux closed 5 months ago

jmailloux commented 6 months ago

The previous code used the db's GenSigStartValue as the default value for all signals. That should only be used if an individual signal doesn't have a GenSigStartValue. This fixes signal's losing their GenSigStartValue after a dbc merge. Sometimes it would cause the GenSigStartValue to be outside of the min / max value. Also specifically imported Optional so the code would run in Python 3.12.

ebroecker commented 5 months ago

Hi @jmailloux ,

thanks for you pr. It seems, you are deleting some code for using the db's GenSigStartValue even if there is no GenSigStartValue for the signal defined. No sure, I need to have a closer look...

Meanwhile, could you resolve the conflicts? Thanks

jmailloux commented 5 months ago

Hi @jmailloux ,

thanks for you pr. It seems, you are deleting some code for using the db's GenSigStartValue even if there is no GenSigStartValue for the signal defined. No sure, I need to have a closer look...

Meanwhile, could you resolve the conflicts? Thanks

Hi @ebroecker , no problem. I've done the merge. I fixed a problem where the merging of dbc files would actually blow away individual signal GenSigStartValue. For merges, when the dump function is called the signal's initial value would already have been correctly set. The old code used the dbc's default GenSigStartValue, which would in effect overwrite the intended GenSigStartValue. The updates to the load function prevent the signal's initial to be set to something that is outside the min / max values for the signal.

ebroecker commented 5 months ago

@jmailloux

this patch fails 2 tests:

tests/test_dbc.py::test_signal_inital_value tests/test_dbc.py::test_default_initial_value

could you please have another look on it? [Sorry for this late finding, ...]

jmailloux commented 5 months ago

@ebroecker - sure. I will have a look at this.

ebroecker commented 4 months ago

hi @jmailloux

maybe you find time to have a look on https://github.com/ebroecker/canmatrix/pull/810

Hopefully I fixed the dbc initial value behaviour