From 3b03ab2559d3678634a90773c3a819d59311b62f Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Thu, 16 May 2024 14:21:16 +0200 Subject: [PATCH] Remove vss2dbc mapping for sensor Related to #23 Current CAN provider implementation always subscribe to target value for "vss2dbc". That does not really make sense for sensors, as they do not have target values --- dbcfeederlib/dbc2vssmapper.py | 7 ++++++- mapping/README.md | 2 ++ mapping/vss_4.0/dbc_overlay.vspec | 2 -- mapping/vss_4.0/vss_dbc.json | 5 +---- mapping/vss_4.1/dbc_overlay.vspec | 2 -- mapping/vss_4.1/vss_dbc.json | 5 +---- .../mapping_for_ambiguous_signal.json | 2 +- .../mapping_for_unused_ambiguous_signal.json | 2 +- .../mapping_vss2dbc_not_actuator.json | 17 +++++++++++++++++ test/test_mapping_error/test_mapping_error.py | 13 +++++++++++++ 10 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 test/test_mapping_error/mapping_vss2dbc_not_actuator.json diff --git a/dbcfeederlib/dbc2vssmapper.py b/dbcfeederlib/dbc2vssmapper.py index 3e6cd72..218f693 100644 --- a/dbcfeederlib/dbc2vssmapper.py +++ b/dbcfeederlib/dbc2vssmapper.py @@ -426,7 +426,12 @@ def _analyze_signal(self, expanded_name, node): if dbc2vss_def is not None: self._analyze_dbc2vss(expanded_name, node, dbc2vss_def) if "vss2dbc" in node: - self._analyze_vss2dbc(expanded_name, node, node["vss2dbc"]) + if node["type"] == "actuator": + self._analyze_vss2dbc(expanded_name, node, node["vss2dbc"]) + else: + # vss2dbc is handled by subscription to target value, so only makes sense for actuators + log.error("vss2dbc only allowed for actuators, VSS signal %s is not an actuator!", expanded_name) + sys.exit(-1) def _traverse_vss_node(self, name, node, prefix=""): """ diff --git a/mapping/README.md b/mapping/README.md index ea6de07..2155f28 100644 --- a/mapping/README.md +++ b/mapping/README.md @@ -44,6 +44,8 @@ This is built on the assumption that the DBC provider always send target values Having separate configurations (`dbc2vss` and `vss2dbc`) is needed as wanted value and actual value never are sent by the same DBC signal, they are not even part of the same CAN-frame. +*This means that `vss2dbc` only can be used for actuators, as only actuators have target values!* + ## Example mapping files Example mapping files for various VSS versions can be found in this folder. diff --git a/mapping/vss_4.0/dbc_overlay.vspec b/mapping/vss_4.0/dbc_overlay.vspec index 2c8970d..1cc2ccb 100644 --- a/mapping/vss_4.0/dbc_overlay.vspec +++ b/mapping/vss_4.0/dbc_overlay.vspec @@ -301,8 +301,6 @@ Vehicle.Powertrain.ElectricMotor.Temperature: dbc2vss: signal: PTC_rightTempIGBT interval_ms: 1000 - vss2dbc: - signal: PTC_rightTempIGBT Vehicle.Cabin.Door.Row1.DriverSide.IsOpen: type: actuator diff --git a/mapping/vss_4.0/vss_dbc.json b/mapping/vss_4.0/vss_dbc.json index c3b067f..2ef89f3 100644 --- a/mapping/vss_4.0/vss_dbc.json +++ b/mapping/vss_4.0/vss_dbc.json @@ -7105,10 +7105,7 @@ }, "description": "Motor temperature.", "type": "sensor", - "unit": "celsius", - "vss2dbc": { - "signal": "PTC_rightTempIGBT" - } + "unit": "celsius" }, "Torque": { "datatype": "int16", diff --git a/mapping/vss_4.1/dbc_overlay.vspec b/mapping/vss_4.1/dbc_overlay.vspec index 7b81f30..4ad6430 100644 --- a/mapping/vss_4.1/dbc_overlay.vspec +++ b/mapping/vss_4.1/dbc_overlay.vspec @@ -301,8 +301,6 @@ Vehicle.Powertrain.ElectricMotor.Temperature: dbc2vss: signal: PTC_rightTempIGBT interval_ms: 1000 - vss2dbc: - signal: PTC_rightTempIGBT Vehicle.Cabin.Door.Row1.DriverSide.IsOpen: type: actuator diff --git a/mapping/vss_4.1/vss_dbc.json b/mapping/vss_4.1/vss_dbc.json index e4231df..49e7f9d 100644 --- a/mapping/vss_4.1/vss_dbc.json +++ b/mapping/vss_4.1/vss_dbc.json @@ -7476,10 +7476,7 @@ }, "description": "Motor temperature.", "type": "sensor", - "unit": "celsius", - "vss2dbc": { - "signal": "PTC_rightTempIGBT" - } + "unit": "celsius" }, "Torque": { "datatype": "int16", diff --git a/test/test_mapping_error/mapping_for_ambiguous_signal.json b/test/test_mapping_error/mapping_for_ambiguous_signal.json index 559d327..53ac0bf 100644 --- a/test/test_mapping_error/mapping_for_ambiguous_signal.json +++ b/test/test_mapping_error/mapping_for_ambiguous_signal.json @@ -4,7 +4,7 @@ "B": { "datatype": "uint8", "description": "...", - "type": "sensor", + "type": "actuator", "unit": "km", "vss2dbc": { "signal": "SignalOne" diff --git a/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json b/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json index a32845a..467b302 100644 --- a/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json +++ b/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json @@ -4,7 +4,7 @@ "B": { "datatype": "uint8", "description": "...", - "type": "sensor", + "type": "actuator", "unit": "km", "vss2dbc": { "signal": "SignalTwo" diff --git a/test/test_mapping_error/mapping_vss2dbc_not_actuator.json b/test/test_mapping_error/mapping_vss2dbc_not_actuator.json new file mode 100644 index 0000000..559d327 --- /dev/null +++ b/test/test_mapping_error/mapping_vss2dbc_not_actuator.json @@ -0,0 +1,17 @@ +{ + "A": { + "children": { + "B": { + "datatype": "uint8", + "description": "...", + "type": "sensor", + "unit": "km", + "vss2dbc": { + "signal": "SignalOne" + } + } + }, + "description": "Branch A.", + "type": "branch" + } +} diff --git a/test/test_mapping_error/test_mapping_error.py b/test/test_mapping_error/test_mapping_error.py index 413870e..1c35671 100644 --- a/test/test_mapping_error/test_mapping_error.py +++ b/test/test_mapping_error/test_mapping_error.py @@ -40,6 +40,19 @@ def test_unknown_transform(caplog: pytest.LogCaptureFixture): assert error_msg in caplog.record_tuples +def test_vss2dbc_sensor(caplog: pytest.LogCaptureFixture): + + mapping_path = test_path + "/mapping_vss2dbc_not_actuator.json" + dbc_file_names = [test_path + "/../../Model3CAN.dbc"] + + with pytest.raises(SystemExit) as excinfo: + dbc2vssmapper.Mapper(mapping_path, dbc_file_names) + assert excinfo.value.code == -1 + error_msg = ("dbcfeederlib.dbc2vssmapper", logging.ERROR, + "vss2dbc only allowed for actuators, VSS signal A.B is not an actuator!") + assert error_msg in caplog.record_tuples + + def test_mapper_fails_for_duplicate_signal_definition(): mapping_path = test_path + "/mapping_for_ambiguous_signal.json"