Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ElectronicControlUnit.disconnect() explodes if called twice #55

Open
grant-allan-ctct opened this issue Dec 2, 2023 · 0 comments
Open

Comments

@grant-allan-ctct
Copy link
Collaborator

I am using a unittest-based enviroment for some exploratory coding for the purpose of learning how python-can-j1939 works. In the tearDown of my test class, I am making a call to ecu.disconnect() in order to perform housekeeping / clean-up tasks after each test runs. I accidentally discovered that if my testcase code itself calls ecu.disconnect() during the test, the tearDown explodes. This happens because the implementation of disconnect doesn't check for None-ness of the _bus before calling its shutdown() method. Calling the below function twice will cause an error.

    def disconnect(self):
        """Disconnect from the CAN bus.

        Must be overridden in a subclass if a custom interface is used.
        """
        self._notifier.stop()
        self._bus.shutdown()
        self._bus = None

I think that we should change this function so that it can be called safely multiple times. (This would match the philosophy used by the BusABC.shutdown() method itself, see documentation here.)

If we make a change to add safety to disconnect(), then someone can call that function in an error-handler / housekeeping situation, or in unit test tear-down, etc., and it will always work effortlessly - regardless of whether disconnect() happens to have been called successfully earlier or not.

If we do not make a change, the following workaround may help someone. It involves subclassing the ElectronicControlUnit class, something like this. Wherever you would normally use ElectronicControlUnit, use the subclass instead:

class RobustECU(j1939.ElectronicControlUnit):

  def disconnect(self):
    # workaround for parent class not being robust against multiple disconnect calls
    if self._bus is not None:
      super().disconnect()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant