Initial J1939 changes

[JohnLussmyer]

Initial set of changes, test failing

see #54

[pschichtel]

If I understand the can-utils code below correctly, then you can only write to sockets after connect() (so you have a known destination address for write()) and and you can only read from sockets after bind() (so you have a known receive address for read())

• https://github.com/linux-can/can-utils/blob/master/j1939sr.c (sender + receiver)
• https://github.com/linux-can/can-utils/blob/master/j1939spy.c (pure receiver)

(can-utils is mostly using send, sendmsg, recv and recvmsg because they also process message attribute, they are equivalent to read and write for JavaCAN's purposes)

[JohnLussmyer]

Ahh ok, I hadn't quite made sense of the J1939 docs yet. I'll try setting up tests with addresses.

[JohnLussmyer]

Well, I'm getting a bit closer. Now it fails during write with a:

tel.schich.javacan.platform.linux.LinuxNativeOperationException: Unable to get FD frame support - errorNumber=22, errorMessage='Invalid argument'

[pschichtel]

that exception does not happen during write, but during the setOption call of the FD_FRAMES option. not sure why it would be invalid, I guess J1939 might not support FD frames?

[JohnLussmyer]

It's happening on the test code line 202 - which is commented out. Looking at what was being called, it's failing on final CanFrame output = b.read();

[pschichtel]

your read() method does call getOption(FD_FRAMES) to allocate a correctly sized buffer, which I assume you copied over from the raw channel. When J1939 doesn't support FD, you can just hardcode MTU (the else-value in the ternary).

[pschichtel]

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/can/j1939/socket.c#n681

there you can see the supported socket options for J1939 sockets.

[JohnLussmyer]

Yeah, lots of copied code. Your do realize that i'm new to the whole canbus programming thing? :-) I'm completely un-surprised at missing odd interactions and restrictions. I'll make some changes and try again!

[JohnLussmyer]

Whee!!! that test passed!

[JohnLussmyer]

In my hunt for actual documentation on the options, I did find this: https://patchwork.ozlabs.org/project/netdev/patch/20110427090011.GE757@kurt.e-circ.dyndns.org/ Sure would be nice to find whatever doc it's a patch for...

[pschichtel]

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/j1939.rst

I think that's what you are looking for

[JohnLussmyer]

Not really, that's pretty much the same as a couple of the other pages I've found, where it mentions the options, but gives no description of what they do, or what values are valid. The patch link I gave has descriptions and values.

[pschichtel]

What are you looking for? Except for SO_J1939_ERRQUEUE the J1939 specific options seem to all be mentioned and explained. I think parts of the options you found in that patch never materialized upstream. From scrolling through it I guess some of the options are covered by existing generic CAN options

[JohnLussmyer]

I'm used to full docs on an API. The link you provided mentions SO_J1939_PROMISC and SO_J1939_SEND_PRIO. No specifics on what the values should be, almost no description of what they do. Admittedly, this seems to be the normal case for Unix/Linux API's, where it's expected that you go look at the source code to figure out the details. It's not what I consider a good API doc.

[pschichtel]

Their domain is actually documented, though not as explicitly as you'd hope. I think the "issue" is that they assume knowledge of the specs which are sadly not as accessible as I would prefer them to be. I learned that when I tried implementing ISO-TP in user space.

If you want to improve on this, the can subsystem mailing list is pretty responsive.

Given you are at the point that this is working for an initial set of tests, I'm thinking about merging this early and cleaning it up a bit on master. What do you think?

[JohnLussmyer]

Sure , I take it need to do another pull request?

[pschichtel]

I would merge it and refine it on master to be a little more in line with the rest of the codebase. Is there any major functionality left you'd like to add?

[JohnLussmyer]

Not that I can think of right now.
Once I start using it, maybe...

[pschichtel]

I've locally reworked your commits to remove noisy whitespace changes, I'll also do some other cleanups

[JohnLussmyer]

Not surprised at the whitespace changes. I'm so used to letting eclipse clean up the little things...

[JohnLussmyer]

I've setup a tiny test app, and getting a weird error:

john@raspberrypi4:~/PiCanTest $ java -cp PiCanTest_lib -jar PiCanTest.jar Exception in thread "main" java.lang.IllegalArgumentException: length must be either MTU or FD_MTU, but was 8! at tel.schich.javacan.CanFrame.createUnsafe(CanFrame.java:458) at tel.schich.javacan.CanFrame.create(CanFrame.java:431) at tel.schich.javacan.J1939CanChannelImpl.read(J1939CanChannelImpl.java:101) at tel.schich.javacan.J1939CanChannelImpl.read(J1939CanChannelImpl.java:95) at com.casadelgato.cantest.PiCANTest.main(PiCANTest.java:32)

I've even tried forcing it to 16, like this:

public CanFrame read() throws IOException {
    int length = 16; //MTU;

Same error.

[pschichtel]

have you checked how many bytes have actually been read? Also: you shouldn't be reading CAN frames from the J1939 socket. the read() syscall reads the data bytes only. I've already deleted the CanFrame read() methods on master as they don't make sense. CanFrame is meant for raw frames (I know, the name doesn't really reflect that.)

[pschichtel]

I think we might have to add send and recv style APIs as J1939 doesn't seem to be useful without metadata in certain situations. ISO-TP is only point-to-point (with a tiny exception), so that got away with just read and write, but J1939 supports multicasts more similar to raw.

[JohnLussmyer]

So, it looks like I need to add the Send and Recv calls? Ok, will do that today. (As you might have guessed, I don't get much free time during the work week...)

[pschichtel]

I was actually about to start on it

[JohnLussmyer]

If you want to, that would be great! You are likely far more experienced with these updates, and won't mess up the formatting like I do.

[JohnLussmyer]

Note that I do have a J1939 device sitting on my desk, connected to a Pi Can Duo on a Pi4. I've been setting up a little test app that will just listen and and display/log what it hears. Once I can do that, I'll be able to move much faster with testing the J1939 support.

[JohnLussmyer]

I just did an experiment, using the latest build, and this test code with my J1939 device. channel.bind(CAN_INTERFACE); channel.connect(CAN_INTERFACE, J1939CanChannel.NO_NAME, J1939CanChannel.NO_PGN, (short) 0x20); channel.setOption(J1939CanSocketOptions.SO_J1939_PROMISC, true);

        while (true) {
            ByteBuffer buffer = JavaCAN.allocateOrdered(32);
            channel.read(buffer);
            System.out.println(toHexStr(buffer));
        }

This results in steady stream of: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

Note that when I used a can bus analyzer, I got:

0x18FFDAA1x;8;0xFF;0xFF;0xFF;0xFF;0x40;0x00;0x20;0x80; 0x18FFD2A1x;8;0x12;0x00;0x00;0x00;0x00;0x00;0x00;0x00; 0x18FFD3A1x;8;0x01;0x01;0x01;0x01;0x02;0x0A;0x02;0x0A;

Hmm, I'll see what CanUtils gives me - once I figure out the command line...

[pschichtel]

you have to flip the buffer after reading into it

[pschichtel]

ByteBuffer buffer = JavaCAN.allocateOrdered(32);
while (true) {
    buffer.clear();
    channel.read(buffer);
    buffer.flip();
    System.out.println(toHexStr(buffer));
}

[JohnLussmyer]

Ahh, ok. Just tried that.

0 e3 1c a0 28 10 0 3 0 e3 1c a0 28 10 0 3 0 e3 1c a0 28 10 0 3

hmm, not a clue as to how that relates to the data the bus analyzer gave. I find it suspicious that the values aren't changing, as the analyzer showed a steady stream of different msgs. Looking at the bus analyzer output, it's providing 8 bytes of data. Though it doesn't match AT ALL the data recv is getting. Definitely indicates that I'll need the header info as well as the data, as there is critical info in the header - which doesn't get returned by read.

[pschichtel]

try the latest build

[JohnLussmyer]

Just noticed your msg, and tried.

    try (final J1939CanChannel channel = CanChannels.newJ1939Channel()) {
        J1939Address addr = new J1939Address(CAN_INTERFACE);
        channel.bind(addr);
        channel.connect(addr);
        channel.setOption(J1939CanSocketOptions.SO_J1939_PROMISC, true);

        while (true) {
            ByteBuffer buffer = JavaCAN.allocateOrdered(32);
            channel.receiveMessage(buffer, addr);
            buffer.flip();
            System.out.println(toHexStr(buffer));
        }

Gets this error:

john@raspberrypi4:~/PiCanTest $ java -cp PiCanTest_Lib/* -jar PiCanTest.jar tel.schich.javacan.platform.linux.LinuxNativeOperationException: Unable to connect - errorNumber=13, errorMessage='Permission denied' at tel.schich.javacan.SocketCAN.connectJ1939Address(Native Method) at tel.schich.javacan.J1939CanChannelImpl.connect(J1939CanChannelImpl.java:67) at com.casadelgato.cantest.PiCANTest.main(PiCANTest.java:30)

[pschichtel]

The kernel module does not allow to connect to addresses if they have neither a name nor an addr and the socket does not have SO_BROADCAST. I've added a test that in my last commit probing for exactly that.

btw: It seems you have your own toHexStr() function, JavaCAN already brings CanUtils.hexDump().

[JohnLussmyer]

Ok, I've modified my code to be:

    try (final J1939CanChannel channel = CanChannels.newJ1939Channel()) {
        J1939Address addr = new J1939Address(CAN_INTERFACE);
        channel.bind(addr);
        channel.setOption(J1939CanSocketOptions.SO_J1939_PROMISC, true);

        while (true) {
            ByteBuffer buffer = JavaCAN.allocateOrdered(32);
            // channel.receiveMessage(buffer, addr);
            channel.receiveData(buffer);
            buffer.flip();
            System.out.println(toHexStr(buffer));
        }

It now starts receiving data, but I'm fairly certain that it's wrong. I've tried with both receiveData() and receiveMesage(), and both got this result:

0 e3 1c a0 28 10 0 3 0 e3 1c a0 28 10 0 3 0 e3 1c a0 28 10 0 3

When I use the can analyzer tool, the values it gives (provided previously), Vary with every message, and none of these values seem to match what I expect from the device.

And just a reminder, that I am pretty much clueless as to CanBus protocol, usage, etc...

[pschichtel]

Would you be able to use any of the j1939* tools from can-utils to get similar output to what your analyser too produces ? then I can investigate who can-utils configures the socket to produce that output and possible reproduce it in a test. Sadly I don't have any J1939 hardware handy.

[JohnLussmyer]

I can try, but the canUtils docs are fairly opaque to me. I really don't know where to start with them.

[JohnLussmyer]

Hmm, I tried "candump can1" and got similar results:

john@raspberrypi4:~/PiCanTest $ candump can1 can1 18FFD4A1 [8] 00 E3 1C A0 28 10 00 03 can1 18FFD4A1 [8] 00 E3 1C A0 28 10 00 03

Let me dig into the units docs, and figure out which message that is. Though it should be sending multiple different ones

[JohnLussmyer]

Ahh, ha! I found that I had to reset the device, since it went into a standby state due to lack of commands. Note that Candump gives me lines like:

can1 18FFD8A1 [8] 13 00 E5 7C F9 00 3E 3E can1 18FFD9A1 [8] 12 00 00 7D 13 00 00 7D can1 18FFDAA1 [8] FF FF FF FF 3E 00 20 80 can1 18FFD2A1 [8] 12 00 00 00 00 00 00 00 can1 18FFD3A1 [8] 01 01 01 01 02 0A 02 0A can1 18FFD4A1 [8] 00 E3 1C 00 28 10 00 01

and my app just gives the 8 bytes of data. I REALLY REALLY need the PGN as well, since it uses different ones for different data items. Any suggestions on how to get that?

[JohnLussmyer]

And note that I have tried using receiveMessage(), and that doesn't seem to fill in the addr block at all.

    try (final J1939CanChannel channel = CanChannels.newJ1939Channel()) {
        J1939Address addr = new J1939Address(CAN_INTERFACE);
        channel.bind(addr);
        channel.setOption(J1939CanSocketOptions.SO_J1939_PROMISC, true);

        while (true) {
            ByteBuffer buffer = JavaCAN.allocateOrdered(32);
            J1939Address addr2 = new J1939Address(CAN_INTERFACE);
            channel.receiveMessage(buffer, addr2);
            buffer.flip();
            System.out.println(String.format("name=%016X, PGN=%08X, addr=%02X, data=%s", addr.getName(), addr.getParameterGroupName(), addr.getAddress(), CanUtils.hexDump(buffer)));
        }

results in : name=0000000000000000, PGN=00040000, addr=FF, data=00.00.21.7D.00.7D.00.00 name=0000000000000000, PGN=00040000, addr=FF, data=13.00.E5.7C.F9.00.3E.3E name=0000000000000000, PGN=00040000, addr=FF, data=13.00.00.7D.14.00.00.7D name=0000000000000000, PGN=00040000, addr=FF, data=FF.FF.FF.FF.3E.00.20.80 name=0000000000000000, PGN=00040000, addr=FF, data=12.00.00.00.00.00.00.00 name=0000000000000000, PGN=00040000, addr=FF, data=01.01.01.01.02.0A.02.0A

[pschichtel]

receiveMessage does not modify the address (addresses are immutable), it returns a new object of type J1939ReceivedMessageHeader with all the information supplied by the control messages. that's the current design, but I'm considering to change it towards something that requires fewer/no allocations.

[JohnLussmyer]

Yup, just figured that out. changing code now.

[JohnLussmyer]

Doesn't seem to help.

J1939ReceivedMessageHeader{bytesReceived=8, timestamp=1970-01-01T00:00:00Z, destinationAddress=-1, destinationName=0, priority=6} 0 e3 1c a0 28 10 0 3

[JohnLussmyer]

Any chance of just returning the ENTIRE packet, address and data, in the one bytebuffer?

[pschichtel]

J1939ReceivedMessageHeader{bytesReceived=8, timestamp=1970-01-01T00:00:00Z, destinationAddress=-1, destinationName=0, priority=6} 0 e3 1c a0 28 10 0 3

yeah I already suspected that something regarding the control message parsing might not be working. The values you see are the defaults used by my logic

Any chance of just returning the ENTIRE packet, address and data, in the one bytebuffer?

JavaCAN works with the SocketCAN API. there is no "entire packet" unless you are using CAN_RAW sockets. the socket API supplies the contents and separately a control message structure, both dynamically sized.

[pschichtel]

I just realized I can provide you with the source address, completely missed that. I have to say, that the kernel code of the module is actually surprisingly nice to read and get information from...

[pschichtel]

ok your confusion about the source address parameter came from me... it was actually my confusion about how recvmsg() works. I fixed that in the latest commit: The parameter is gone now, instead if source address is included in the header object.

[JohnLussmyer]

That looks like it's working! name=0000000000000000, PGN=0000FFD3, addr=A1, data=01.01.01.01.02.0A.02.0A name=0000000000000000, PGN=0000FFD4, addr=A1, data=00.E3.1C.00.28.10.00.01 name=0000000000000000, PGN=0000FFD7, addr=A1, data=00.00.00.7D.00.7D.00.00

[JohnLussmyer]

Next I have to figure out how to send a packet to the device. I'm assuming that sendMessage() will be what I need.

[pschichtel]

cool, do timestamps work for you?