Fix J1939Reader

The reader erroneously referred to a non-existing (private) member of the DBCReader. This has been fixed. Also added some tests for verifying the J1939Reader's functionality.
eclipse-kuksa · Sep 12, 2023 · b929265 · b929265
1 parent a5a6e29
commit b929265
Show file tree

Hide file tree

Showing 8 changed files with 233 additions and 27 deletions.
diff --git a/dbc2val/dbcfeederlib/dbcparser.py b/dbc2val/dbcfeederlib/dbcparser.py
@@ -1,7 +1,7 @@
 #!/usr/bin/python3
 
 ########################################################################
-# Copyright (c) 2023 Robert Bosch GmbH
+# Copyright (c) 2023 Contributors to the Eclipse Foundation
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -54,8 +54,9 @@ def __init__(self,
                 # by default, do not mask any bits of standard (11-bit) frame IDs
                 mask = 0b11111111111
                 if expect_extended_frame_ids:
-                    # mask 3 priority bits of extended (29-bit) frame IDs
-                    mask = 0b00011111111111111111111111111
+                    # ignore 3 priority bits and 8 source address bits of extended
+                    # (29-bit) frame IDs when looking up message definitions
+                    mask = 0b00011111111111111111100000000
                 log.info("Reading definitions from DBC file %s", filename)
                 database = cantools.database.load_file(filename, strict=use_strict_parsing, frame_id_mask=mask)
                 # load_file can return multiple types of databases, make sure we have CAN database

diff --git a/dbc2val/dbcfeederlib/j1939reader.py b/dbc2val/dbcfeederlib/j1939reader.py
@@ -1,7 +1,7 @@
 #!/usr/bin/python3
 
 ########################################################################
-# Copyright (c) 2020,2023 Robert Bosch GmbH
+# Copyright (c) 2020,2023 Contributors to the Eclipse Foundation
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -65,9 +65,11 @@ def start_listening(self, *args, **kwargs):
         self.ecu.subscribe(self.on_message)
 
     def on_message(self, priority: int, pgn: int, sa: int, timestamp: int, data):
-        message = self.identify_message(pgn)
+        # create an extended CAN frame ID from PGN and source address
+        extended_can_id: int = pgn << 8 | sa
+        message = self.dbc_parser.get_message_for_canid(extended_can_id)
         if message is not None:
-            log.debug("processing j1939 message [PGN: %s]", pgn)
+            log.debug("processing j1939 message [PGN: %#x]", pgn)
             try:
                 decode = message.decode(bytes(data), allow_truncated=True)
                 # log.debug("Decoded message: %s", str(decode))
@@ -77,23 +79,9 @@ def on_message(self, priority: int, pgn: int, sa: int, timestamp: int, data):
                         # Now time is defined per VSS signal, so handling needs to be different
                         for signal in self.mapper[k]:
                             if signal.time_condition_fulfilled(rxTime):
-                                log.debug(f"Queueing {signal.vss_name}, triggered by {k}, raw value {v} ")
+                                log.debug("Queueing %s, triggered by %s, raw value %s", signal.vss_name, k, v)
                                 self.queue.put(dbc2vssmapper.VSSObservation(k, signal.vss_name, v, rxTime))
                             else:
-                                log.debug(f"Ignoring {signal.vss_name}, triggered by {k}, raw value {v} ")
+                                log.debug("Ignoring %s, triggered by %s, raw value %s", signal.vss_name, k, v)
             except Exception:
-                log.warning(
-                    "Error decoding message [PGN: {}]".format(message.name),
-                    exc_info=True,
-                )
-
-    def identify_message(self, pgn):
-        pgn_hex = hex(pgn)[2:]  # only hex(pgn) without '0x' prefix
-        for message in self.dbc_parser.db.messages:
-            message_hex = hex(message.frame_id)[
-                -6:-2
-            ]  # only hex(pgn) without '0x' prefix, priority and source address
-            if pgn_hex == message_hex:
-                return message
-        log.debug("no DBC mapping registered for j1939 message [PGN: %s]", pgn_hex)
-        return None
+                log.warning("Error decoding message %s [PGN: %#x]", message.name(), pgn, exc_info=True)
diff --git a/dbc2val/test/test_dbc/test1_1.kcd b/dbc2val/test/test_dbc/test1_1.kcd
@@ -1,13 +1,32 @@
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<NetworkDefinition xmlns="http://kayak.2codeornot2code.org/1.0">
+<!--
+Copyright (c) 2023 Contributors to the Eclipse Foundation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+SPDX-License-Identifier: Apache-2.0
+-->
+<NetworkDefinition xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://kayak.2codeornot2code.org/1.0 https://raw.githubusercontent.com/dschanoeh/Kayak/master/Kayak-kcd/src/main/resources/com/github/kayak/canio/kcd/loader/Definition.xsd"
+    xmlns="http://kayak.2codeornot2code.org/1.0">
     <Document name="test1_1.kcd" date="Wed Jul 05 15:13:39 CEST 2023"></Document>
     <Bus name="Private">
         <Message id="0x16" name="ID016DI_bmsRequest" length="1">
             <Signal name="DI_bmsOpenContactorsRequest" offset="4">
-                <Value/>
+                <Value />
             </Signal>
             <Signal length="4" name="DI_bmsRequestInterfaceVersion" offset="0">
-                <Value max="15.0"/>
+                <Value max="15.0" />
             </Signal>
         </Message>
     </Bus>

diff --git a/dbc2val/test/test_dbc/test1_2.kcd b/dbc2val/test/test_dbc/test1_2.kcd
@@ -1,5 +1,24 @@
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<NetworkDefinition xmlns="http://kayak.2codeornot2code.org/1.0">
+<!--
+Copyright (c) 2023 Contributors to the Eclipse Foundation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+SPDX-License-Identifier: Apache-2.0
+-->
+<NetworkDefinition xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://kayak.2codeornot2code.org/1.0 https://raw.githubusercontent.com/dschanoeh/Kayak/master/Kayak-kcd/src/main/resources/com/github/kayak/canio/kcd/loader/Definition.xsd"
+    xmlns="http://kayak.2codeornot2code.org/1.0">
     <Document name="test1_2.kcd" date="Wed Jul 05 15:13:39 CEST 2023"></Document>
     <Bus name="Private">
         <Message id="0x129" name="ID129SteeringAngle" length="8" interval="10">

diff --git a/dbc2val/test/test_j1939/j1939.kcd b/dbc2val/test/test_j1939/j1939.kcd
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+Copyright (c) 2023 Contributors to the Eclipse Foundation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+SPDX-License-Identifier: Apache-2.0
+-->
+<NetworkDefinition xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://kayak.2codeornot2code.org/1.0 https://raw.githubusercontent.com/dschanoeh/Kayak/master/Kayak-kcd/src/main/resources/com/github/kayak/canio/kcd/loader/Definition.xsd"
+    xmlns="http://kayak.2codeornot2code.org/1.0">
+    <Document />
+    <Bus name="Private">
+        <Message id="0x01FFFF10" name="Test_Message" length="24" format="extended">
+            <Signal length="16" name="Signal_Two" offset="8">
+                <Value type="signed" />
+            </Signal>
+            <Signal length="8" name="Signal_One" offset="0">
+                <Value max="213" type="unsigned" />
+            </Signal>
+        </Message>
+    </Bus>
+</NetworkDefinition>
diff --git a/dbc2val/test/test_j1939/simple_mapping.json b/dbc2val/test/test_j1939/simple_mapping.json
@@ -0,0 +1,28 @@
+{
+  "A": {
+    "children": {
+      "One": {
+        "datatype": "uint8",
+        "dbc2vss": {
+          "on_change": true,
+          "signal": "Signal_One"
+        },
+        "description": "...",
+        "type": "sensor",
+        "unit": "km"
+      },
+      "Two": {
+        "datatype": "int16",
+        "dbc2vss": {
+          "on_change": true,
+          "signal": "Signal_Two"
+        },
+        "description": "...",
+        "type": "sensor",
+        "unit": "ms"
+      }
+    },
+    "description": "Branch A.",
+    "type": "branch"
+  }
+}
diff --git a/dbc2val/test/test_j1939/simple_mapping.vspec b/dbc2val/test/test_j1939/simple_mapping.vspec
@@ -0,0 +1,46 @@
+########################################################################
+# Copyright (c) 2023 Contributors to the Eclipse Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0
+########################################################################
+
+#
+# Source file for tests
+# Generate file used by test (test.json) with help of vss-tools
+# https://github.com/COVESA/vss-tools
+# When cloned execute something like this from this folder:
+# ~/vss-tools/vspec2json.py -e dbc2vss --json-pretty <name>.vspec <name>.json
+#
+A:
+  type: branch
+  description: Branch A.
+
+A.One:
+  datatype: uint8
+  type: sensor
+  unit: km
+  description: "..."
+  dbc2vss:
+    signal: "Signal_One"
+    on_change: true
+
+A.Two:
+  datatype: int16
+  type: sensor
+  unit: ms
+  description: "..."
+  dbc2vss:
+    signal: "Signal_Two"
+    on_change: true
diff --git a/dbc2val/test/test_j1939/test_j1939.py b/dbc2val/test/test_j1939/test_j1939.py
@@ -0,0 +1,72 @@
+#!/usr/bin/python3
+
+########################################################################
+# Copyright (c) 2023 Contributors to the Eclipse Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0
+########################################################################
+
+import os
+
+from queue import Empty, Queue
+from typing import Dict
+
+import pytest  # type: ignore
+
+from dbcfeederlib.dbc2vssmapper import Mapper, VSSObservation
+from dbcfeederlib.dbcparser import DBCParser
+from dbcfeederlib.j1939reader import J1939Reader
+
+test_path = os.path.dirname(os.path.abspath(__file__))
+
+
+def test_on_message_processes_j1939_message():
+
+    # GIVEN a reader based on CAN message and mapping definitions
+    dbc2val_queue = Queue(10)
+    parser = DBCParser([test_path + "/j1939.kcd"], expect_extended_frame_ids=True)
+    mapper = Mapper(test_path + "/simple_mapping.json", parser, "")
+    reader = J1939Reader(rxqueue=dbc2val_queue, mapper=mapper, dbc_parser=parser)
+
+    # WHEN a message is received from the CAN bus for which a mapping gas been defined
+    reader.on_message(priority=1, pgn=0x1FFFF, sa=0x45, timestamp=0, data=[0x10, 0x32, 0x54])
+
+    # THEN the reader determines both VSS Data Entries that the CAN signals are mapped to
+    signal_mappings: Dict[str, VSSObservation] = {}
+    obs: VSSObservation = dbc2val_queue.get(block=False)
+    signal_mappings[obs.dbc_name] = obs
+    obs = dbc2val_queue.get(block=False)
+    signal_mappings[obs.dbc_name] = obs
+
+    assert signal_mappings["Signal_One"].vss_name == "A.One"
+    assert signal_mappings["Signal_One"].raw_value == 0x10
+    assert signal_mappings["Signal_Two"].vss_name == "A.Two"
+    assert signal_mappings["Signal_Two"].raw_value == 0x5432
+
+
+def test_on_message_ignores_unknown_CAN_messages():
+
+    # GIVEN a reader based on CAN message and mapping definitions
+    dbc2val_queue = Queue(10)
+    parser = DBCParser([test_path + "/j1939.kcd"], expect_extended_frame_ids=True)
+    mapper = Mapper(test_path + "/simple_mapping.json", parser, "")
+    reader = J1939Reader(rxqueue=dbc2val_queue, mapper=mapper, dbc_parser=parser)
+
+    # WHEN a  message with an unknown PGN is received from the CAN bus
+    reader.on_message(priority=1, pgn=0x1FFFA, sa=0x12, timestamp=0, data=[0x10, 0x32, 0x54])
+
+    # THEN the reader ignores the message
+    with pytest.raises(Empty):
+        dbc2val_queue.get(block=False)