juergenH87 / python-can-j1939

The package provides SAE J1939 support for Python developers
MIT License
80 stars 49 forks source link

MessageListener No Longer Works With Latest Python-Can Listener #78

Open drewr95 opened 1 month ago

drewr95 commented 1 month ago

Looks like Python-Can had a recent update to add a new abstract method stop

kossnikita commented 3 weeks ago

Does subscribe work after this PR? For some reason I don't have any messages.

drewr95 commented 3 weeks ago

I can take a look, but I can't say I've noticed anything unusual where I've used it. Maybe @khauersp can comment on this as well.

Is there anywhere you're calling stop from? It only stops notifying if stop() was called on the message listener.

kossnikita commented 3 weeks ago

This works good

import can

bus = can.Bus(interface="canalystii", channel="0", bitrate=250000)
while True:
    try:
        msg = bus.recv(1)
        print(msg)
    except KeyboardInterrupt:
        break
bus.shutdown()
Timestamp:     6305.823600    ID: 182756f4    X Rx                DL:  2    36 10                       Channel: 0
Timestamp:     6306.077300    ID: 182756f4    X Rx                DL:  2    36 10                       Channel: 0

But this doesn't output anything

from time import sleep
import j1939

def on_message(priority, pgn, sa, timestamp, data):
    print(f"PGN {pgn} length {len(data)}")

ecu = j1939.ElectronicControlUnit()
ecu.connect(interface="canalystii", channel="0", bitrate=250000)
ecu.subscribe(on_message)

while True:
    try:
        sleep(1)
    except KeyboardInterrupt:
        break

print("Deinitializing")
ecu.disconnect()
drewr95 commented 3 weeks ago

I'm not quite sure the issues you are having.

I did your example and am seeing traffic from the bus:

import j1939
import time

def on_message(priority, pgn, sa, timestamp, data):
    print(f"PGN {pgn} length {len(data)}")

def main():
    ecu = j1939.ElectronicControlUnit()
    ecu.connect(interface="vector", channel="0", bitrate=250000)
    ecu.subscribe(on_message)

    while True:
        try:
            time.sleep(0.5)
        except KeyboardInterrupt:
            break

    print("Deinitializing")
    ecu.disconnect()

if __name__ == "__main__":
    main()
PGN 65391 length 8
PGN 65393 length 8
PGN 65394 length 8
PGN 65392 length 8
PGN 65396 length 8
PGN 65400 length 8
PGN 65406 length 8
PGN 65408 length 8
PGN 65440 length 8
PGN 65405 length 8
PGN 65393 length 8
PGN 65394 length 8
PGN 65392 length 8
kossnikita commented 3 weeks ago

I needed to add the address to my subscription to get the same result.

ecu.subscribe(on_message, 0x56)

Looks like my fault, sorry for misunderstanding

drewr95 commented 3 weeks ago

Looks like my fault, sorry for misunderstanding

No worries :)

kossnikita commented 3 weeks ago

By the way, should this work without the address being declared? I have a problem where I only need to read CAN traffic without interfering, but subscribing with an address causes my CAN analyzer to interfere with the bus. Perhaps this does not related to this issue, then I will create a new one.

khauersp commented 3 weeks ago

I don't think this issue you're having is directly related to this issue about the listener, but not entirely sure on that. It looks like the subscribe method should potentially support subscribing without a destination address. One thing I noticed between the two examples you had linked @kossnikita and one thing to keep in mind is the can.Bus method to make the bus may not entirely be the same as the way the ecu.connect() works. It probably should be but they do call different functions in the code.