search

linux-can / socketcand

Server to access CAN sockets over ASCII protocol
165 stars 41 forks source link

Update on Using can-utils with OBDLink SX cable #41 #19

Closed ca-webmaster closed 1 year ago

ca-webmaster commented 3 years ago

Hi All,

Is there is an update on the issue referenced at:

https://github.com/linux-can/can-utils/issues/41#issuecomment-305276445

I have an OBDLink SX cable and would like to monitor an automobiles CAN messages. It was mentioned that the OBDLinx SX can receive raw CAN frames and send them to a virtual CAN driver (e.g. vcan0) and use can-utils on this vcan0, however, this may not work and/or may not be the best practice.

Has there been any updates on implementing a CAN driver analogue to the slcan.c driver in Linux or anything else that would help sniff using the OBDLink SX? I appreciate the good start @crossband and @hartkopp.

Thanks in advance!

olerem commented 3 years ago

I was using this adapter: https://github.com/HubertD/candleLight with self made OBD2 16 Pin to DB9 cable. It works just fine. Please note, the OBD2 interface is not directly connected to any CAN segment of the car. It is behind a CAN gateway and you need to send requests to get some status instead of just monitoring the bus.

marckleinebudde commented 3 years ago

Has there been any updates on implementing a CAN driver analogue to the slcan.c driver in Linux or anything else that would help sniff using the OBDLink SX?

To my knowledge there isn't any driver available for the device. Maybe @crossband has something.

hartkopp commented 3 years ago

There was an attempt to add a driver for an ELM317 based diagnosis adapter from @norly here: https://marc.info/?t=156284862300005&r=1&w=2 https://github.com/norly/elmcan While the driver does not meet the best Kernel API to suppress out-of-order CAN frame reception it is worth a try IMO.

norly commented 3 years ago

There was an attempt to add a driver for an ELM317 based diagnosis adapter from @norly here:

ELM327, just to make sure there's no confusion ;)

While the driver does not meet the best Kernel API to suppress out-of-order CAN frame reception it is worth a try IMO.

Yes, it uses netif_rx_ni(). I've been meaning to fix it, but since its chances of upstreaming were sketchy due to its design as an ldisc (line discipline), I've not bothered. Within the limits documented in the README, it is complete and works as well as it possibly can. I also see no alternative to using an ldisc, until hopefully one day someone implements dynamically attaching DT fragments. That day, I'll be able to port it to serdev.

Let me know if you need something fixed by opening a GitHub issue or simply emailing me.

marckleinebudde commented 3 years ago

I talked to @jhovold on last year's ELCE and there's no way to dynamically attach serdev devices currently.

hartkopp commented 3 years ago

So the ldisc approach (like in the slcan.c driver) would be ok for you now - even when it has some netif_rx_ni() design issues?

norly commented 3 years ago

That means that elmcan must remain an ldisc for the time being. If you'd like to see it upstreamed as-is and are willing to argue for it to be added to the list of ldiscs, let me know and I'll prepare it for upstreaming (i.e. I'll fix the packet reordering).

marckleinebudde commented 3 years ago

So the ldisc approach (like in the slcan.c driver) would be ok for you now

As it's an external device, you probably want to dynamically attach it to your system, so serdev seems no option for now.

when it has some netif_rx_ni() design issues?

There's rx_offload that works around the packet ordering issue with netif_rx*().

marckleinebudde commented 3 years ago

That means that elmcan must remain an ldisc for the time being.

Sad, but true.

If you'd like to see it upstreamed as-is and are willing to argue for it to be added to the list of ldiscs, let me know and I'll prepare it for upstreaming (i.e. I'll fix the packet reordering).

Yes, convert to rx_offload, see the mcp251xfd driver:

https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tree/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c?h=linux-can-next-for-5.10-20200930#n2862 https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tree/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c?h=linux-can-next-for-5.10-20200930#n1441

If you don't have a timestamp use an increasing int for this.

norly commented 3 years ago

That means that elmcan must remain an ldisc for the time being.

Sad, but true.

Keep in mind that this may well mean the ldisc interface stays in the kernel forever, as we never, ever break userspace (TM). I believe this is why they are reluctant when it comes to adding new ldiscs, and rightly so. Would this driver see enough use to warrant a permanent ldisc entry?

Yes, convert to rx_offload, see the mcp251xfd driver:

Thanks, I shall look into that. The increasing int sounds just right, too. Unless you really need it upstreamed right now for some reason, I hope it's okay inserting this task at the bottom of my current priority list.

marckleinebudde commented 3 years ago

Keep in mind that this may well mean the ldisc interface stays in the kernel forever, as we never, ever break userspace (TM). I believe this is why they are reluctant when it comes to adding new ldiscs, and rightly so. Would this driver see enough use to warrant a permanent ldisc entry?

As serdev doesn't offer a kernel alternative here, ldisc is the way to go.

marckleinebudde commented 1 year ago

ELM327 devices are support since v6.0. The driver makes use of CAN_RX_OFFLOAD, so the networking stack should not reorder CAN frames.