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

Address arbitration doesn't allow for a bus created with receive_own_messages=True setting #56

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

Comments

@grant-allan-ctct
Copy link
Collaborator

If the ElectronicControlUnit.connect function is called with the receive_own_messages value set to True, then

Received ADDRESS CLAIMED message with conflicting address

message will be seen over and over in the console window if:

  • start() is called on the ControllerApplication instance, or if
  • send_message(0x18EAFF00 + ca._device_address, extended_id=True, data=(0, 0xEE, 0, 0, 0, 0, 0, 0)) is called on the ECU.

When looking at the messages, the MessageListener code is not checking whether the incoming msg has is_rx value of True or False before passing the message along to the listeners, and consequently the outgoing address-claim message from the CA itself is interpreted as if another ECU on the bus is competing for the same address.

@grant-allan-ctct
Copy link
Collaborator Author

I have come up with a workaround for this behavior. I create a subclass of ElectronicControlUnit class, and add these three methods to it:

    def add_listener(self, listener):
        self._listeners.append(listener)
        if self._notifier is not None:
            self._notifier.add_listener(listener)

    def remove_listener(self, listener):
        self._listeners.remove(listener)
        if self._notifier is not None:
            self._notifier.remove_listener(listener)

    def substitute_message_listener(self, listener : MessageListener):
        # Remove any existing MessageListener first:
        for alreadyListening in self._listeners:
          if isinstance(alreadyListening, MessageListener):
            self.remove_listener(alreadyListening)
        self.add_listener(listener)  

and I also create a subclass of MessageListener which has the bug-fix in its on_message_received function, like this:

class CustomMessageListener(MessageListener):

    def on_message_received(self, msg : can.Message):
        if msg.is_rx:
            super().on_message_received(msg)

Finally, in my application code, after creating my ECU instance, I call the substitute_message_listener of my ECU, passing in an instance of CustomMessageListener.

@grant-allan-ctct
Copy link
Collaborator Author

My substitute_message_listener function in prior comment had a mistake - the same kind of mistake that's in our remove_timer function here where we are removing items while inside a for-loop that's cycling through that same list. I have changed my function to this instead now:

    def substitute_message_listener(self, listener : MessageListener):
        # Remove any existing MessageListener first:
        self._listeners[:] = [item for item in self._listeners if not isinstance(item, MessageListener)]
        self.add_listener(listener)    

which was inspired by this stackoverflow post.

(If someday we try to rework #42 we might be able to do something similar in the remove_timer function.)

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