Add overlapping signal bit tests. (#97)

This is to protect against accidental pollution of the signalsets due to
merging commands from multiple vehicles together.

Author: jverkoey

.claude/settings.local.json

@@ -0,0 +1,11 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(python -m pytest:*)",
+      "Bash(python3 -m pytest:*)",
+      "Bash(/usr/bin/python3 -m pytest:*)"
+    ],
+    "deny": [],
+    "ask": []
+  }
+}

python/json_formatter.py

@@ -1,6 +1,8 @@
 from typing import Dict, List, Any, Optional
 import json
 
+from signals_testing import check_overlapping_signals
+
 def format_commands(commands: List[Dict[str, Any]]) -> str:
     """Format a list of commands, sorting them by cmd parameter ID and removing duplicates.
 
@@ -699,13 +701,19 @@ def format_file(input_path: str, output_path: Optional[str] = None) -> str:
 
     Returns:
         Formatted JSON string
+
+    Raises:
+        OverlappingSignalError: If any signals within a command have overlapping bit definitions
     """
     # Read and parse input JSON
     with open(input_path, 'r') as f:
         data = json.load(f)
 
     formatted = format_json_data(data)
 
+    # Validate no overlapping signals after formatting (in case formatting merged commands)
+    check_overlapping_signals(formatted)
+
     # Write to output file if specified
     if output_path:
         with open(output_path, 'w') as f:

python/signals_testing.py

@@ -460,3 +460,87 @@ def test_method(self):
         setattr(target_module, class_name, test_class)
 
     return True
+
+
+class OverlappingSignalError(Exception):
+    """Raised when signals within a command have overlapping bit definitions."""
+    pass
+
+
+def check_overlapping_signals(signalset_json: str) -> List[Dict[str, Any]]:
+    """
+    Check a signalset for overlapping signal bit definitions within commands.
+
+    Uses a bitset approach: for each command, track which bits are occupied.
+    If a signal tries to use a bit that's already occupied, it's an overlap.
+
+    Args:
+        signalset_json: JSON string containing the signal set definition
+
+    Returns:
+        List of overlap errors, each containing:
+        - command: command identifier (hdr/cmd)
+        - signal_id: the signal that caused the overlap
+        - bit: the bit index that was already occupied
+        - conflicting_signal_id: the signal that already occupied the bit
+
+    Raises:
+        OverlappingSignalError: If any overlapping signals are found
+    """
+    import json
+    signalset = json.loads(signalset_json)
+
+    errors = []
+
+    for command in signalset.get('commands', []):
+        # Track which bits are occupied and by which signal
+        occupied_bits: Dict[int, str] = {}
+
+        cmd_identifier = f"hdr={command.get('hdr', '?')}, cmd={command.get('cmd', '?')}"
+
+        for signal in command.get('signals', []):
+            signal_id = signal.get('id', 'unknown')
+            fmt = signal.get('fmt', {})
+
+            start_bit = fmt.get('bix', 0)
+            bit_length = fmt.get('len', 0)
+
+            for bit in range(start_bit, start_bit + bit_length):
+                if bit in occupied_bits:
+                    errors.append({
+                        'command': cmd_identifier,
+                        'signal_id': signal_id,
+                        'bit': bit,
+                        'conflicting_signal_id': occupied_bits[bit]
+                    })
+                    # Only report the first conflicting bit per signal
+                    break
+                occupied_bits[bit] = signal_id
+
+    if errors:
+        error_messages = []
+        for err in errors:
+            error_messages.append(
+                f"Signal '{err['signal_id']}' overlaps with '{err['conflicting_signal_id']}' "
+                f"at bit {err['bit']} in command [{err['command']}]"
+            )
+        raise OverlappingSignalError(
+            f"Found {len(errors)} overlapping signal(s):\n" + "\n".join(error_messages)
+        )
+
+    return errors
+
+
+def test_no_overlapping_signals(signalset_json: str):
+    """
+    Test that a signalset has no overlapping signal bit definitions.
+
+    This function can be used by other repos to validate their signalset files.
+
+    Args:
+        signalset_json: JSON string containing the signal set definition
+
+    Raises:
+        OverlappingSignalError: If any overlapping signals are found
+    """
+    check_overlapping_signals(signalset_json)

python/test_overlappingsignals.py

@@ -0,0 +1,70 @@
+import os
+import pytest
+from pathlib import Path
+
+from json_formatter import format_file
+from signals_testing import check_overlapping_signals, OverlappingSignalError
+
+TESTDATA_DIR = os.path.join(Path(__file__).parent, 'testdata')
+
+
+def test_overlapping_signals_rejected_by_format_file():
+    """Test that format_file raises OverlappingSignalError for bad-overlappingsignals.json."""
+    bad_file = os.path.join(TESTDATA_DIR, 'bad-overlappingsignals.json')
+
+    with pytest.raises(OverlappingSignalError) as exc_info:
+        format_file(bad_file)
+
+    error_message = str(exc_info.value)
+    assert "F150_ODO_2" in error_message
+    assert "F150_ODO" in error_message
+
+
+def test_overlapping_signals_detected():
+    """Test that overlapping signals in bad-overlappingsignals.json are detected."""
+    bad_file = os.path.join(TESTDATA_DIR, 'bad-overlappingsignals.json')
+    with open(bad_file) as f:
+        signalset_json = f.read()
+
+    with pytest.raises(OverlappingSignalError) as exc_info:
+        check_overlapping_signals(signalset_json)
+
+    # Verify the error message contains the expected signal IDs
+    error_message = str(exc_info.value)
+    assert "F150_ODO_2" in error_message
+    assert "F150_ODO" in error_message
+
+
+def test_non_overlapping_signals_pass():
+    """Test that non-overlapping signals pass validation."""
+    signalset_json = '''{
+        "commands": [{
+            "hdr": "720", "rax": "728", "cmd": {"22": "404C"}, "freq": 5,
+            "signals": [
+                {"id": "SIG_A", "fmt": {"bix": 0, "len": 8}},
+                {"id": "SIG_B", "fmt": {"bix": 8, "len": 8}},
+                {"id": "SIG_C", "fmt": {"bix": 16, "len": 16}}
+            ]
+        }]
+    }'''
+    # Should not raise
+    check_overlapping_signals(signalset_json)
+
+
+def test_overlapping_signals_with_implicit_bix():
+    """Test that implicit bix=0 is handled correctly for overlap detection."""
+    signalset_json = '''{
+        "commands": [{
+            "hdr": "720", "rax": "728", "cmd": {"22": "404C"}, "freq": 5,
+            "signals": [
+                {"id": "SIG_A", "fmt": {"len": 8}},
+                {"id": "SIG_B", "fmt": {"len": 4}}
+            ]
+        }]
+    }'''
+    with pytest.raises(OverlappingSignalError) as exc_info:
+        check_overlapping_signals(signalset_json)
+
+    error_message = str(exc_info.value)
+    assert "SIG_B" in error_message
+    assert "SIG_A" in error_message

python/testdata/bad-overlappingsignals.json

@@ -0,0 +1,12 @@
+{ "commands": [
+{ "hdr": "720", "rax": "728", "cmd": {"22": "404C"}, "freq": 5,
+  "signals": [
+    {"id": "F150_ODO", "path": "Trips", "fmt": { "len": 24, "max": 1677721, "div": 10, "unit": "kilometers" }, "name": "Odometer (Instrument panel)", "suggestedMetric": "odometer"},
+    {"id": "F150_ODO_2", "path": "Trips", "fmt": { "len": 8, "max": 1677721, "div": 10, "unit": "kilometers" }, "name": "Odometer (Instrument panel)", "suggestedMetric": "odometer"}
+  ]},
+{ "hdr": "726", "rax": "72E", "cmd": {"22": "2813"}, "freq": 5,
+  "signals": [
+    {"id": "F150_TP_FL", "path": "Tires", "fmt": { "len": 16, "max": 3276, "div": 20, "unit": "psi" }, "name": "Front left tire pressure", "suggestedMetric": "frontLeftTirePressure"}
+  ]}
+]
+}