Skip to content

Tightly Coupled Dependency Model (Inverted Control Lack) #36

Description

@RaulSMS

Description

Classes managing higher-level functions often instantiate or tightly wrap their underlying network interface dependencies directly within their initialization blocks.

  • It is highly complex to instantiate an ECU or a Diagnostic Monitor without giving it direct, concrete access to an actual or fully simulated hardware layer.
  • This tight coupling severely limits the ability to write robust, hardware-agnostic unit tests or execute continuous integration (CI) test suites using mock buses or abstract network streams.

Proposed Architecture Fix

Enforce strict Dependency Injection (DI) across all protocol layers.

  1. The hardware network or Bus driver must be initialized completely outside the ECU/CA context and passed explicitly as an abstracted, interface-compliant argument during construction.
  2. Inject a mocked, async-compliant interface during testing environments to validate address claiming logic, protocol timeouts, and transport errors completely independent of physical or virtual CAN hardware.

Example:

in def connect(self, *args, **kwargs):

The ElectronicControlUnit (ECU) class exposes a .connect() method that explicitly takes low-level keyword arguments and passes them directly down to create a concrete python-can Bus instance internally.

# Analogy of the current pattern in the codebase
class ElectronicControlUnit:
    def __init__(self):
        self.bus = None
        self.channel = None
        
    def connect(self, bustype, channel, bitrate=250000, **kwargs):
        # TIGHT COUPLING: The ECU is taking hard responsibility 
        # for creating the raw hardware python-can interface.
        import can
        self.bus = can.interface.Bus(bustype=bustype, channel=channel, bitrate=bitrate, **kwargs)
        # It then instantiates its internal listener threads using this concrete bus object

Why this is problematic:

  1. Un-mockable for Unit Tests: To test your address claiming or state processing rules in a GitHub Actions CI environment, you cannot simply test the logic because the ECU expects to handle the lifecycle of an actual socket or interface driver.

  2. Hidden Instantiations: Higher-level logic layers cannot be tested without establishing a full hardware pipeline connection block.

The Clean Approach Using Pure python-can abstractions

import can

class ElectronicControlUnit:
    def __init__(self, bus: can.BusABC) -> None:
        """The ECU expects ANY valid python-can bus instance."""
        self._bus = bus
        self._listeners = []
        # The notifier can still be handled internally
        self._notifier = can.Notifier(self._bus, self._listeners, 1)

Why this is still an improvement over connect():

  1. Leveraging python-can's Virtual/Loopback Drivers: Because you pass an already instantiated can.BusABC, your unit tests don't need a custom mock class. We can use python-can's built-in Virtual interface directly in our test suite:
# Inside your test file:
import can

# Creates a real python-can bus object, but purely in memory
test_bus = can.interface.Bus(interface='virtual', channel='test_net')

# Pass it right in. No physical CAN hardware or drivers required.
ecu = ElectronicControlUnit(bus=test_bus)
  1. Flexibility for Multi-ECU Simulation:
    If we want to simulate multiple J1939 ECUs interacting with each other on the same virtual bus, we can instantiate a single virtual bus and pass that exact same bus reference to multiple ECU instances. If the ECU initializes the bus internally via .connect(), simulating multiple nodes talking to each other locally becomes highly complex.

  2. leaner Async Integration (asyncio):
    When moving to an asynchronous architecture, python-can handles async differently depending on the platform (e.g., using can.AsyncioBus). If the ECU takes a pre-configured bus, the application entry point can configure an async-compatible bus and inject it. The ECU itself doesn't need complex conditional logic inside its connect method to decide whether it should spin up a synchronous or asynchronous variant of can.interface.Bus.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions