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

[WIP] j1939 updates and upgrades #335

Merged
merged 27 commits into from
Apr 12, 2019
Merged

[WIP] j1939 updates and upgrades #335

merged 27 commits into from
Apr 12, 2019

Conversation

ebroecker
Copy link
Owner

@ebroecker ebroecker commented Mar 25, 2019

Add functionality to find a frame in matrix by given (j1939-)pgn

  • add Test data
  • create png extraction tests
  • move pgn functionality to ArbitrationId

@codecov-io
Copy link

codecov-io commented Mar 25, 2019

Codecov Report

Merging #335 into development will increase coverage by 0.79%.
The diff coverage is 68.55%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #335      +/-   ##
===============================================
+ Coverage        40.49%   41.29%   +0.79%     
===============================================
  Files               37       39       +2     
  Lines             7008     7168     +160     
  Branches          1649     1683      +34     
===============================================
+ Hits              2838     2960     +122     
- Misses            3968     3990      +22     
- Partials           202      218      +16
Impacted Files Coverage Δ
src/canmatrix/cli/convert.py 0% <ø> (ø) ⬆️
src/canmatrix/__init__.py 100% <ø> (ø) ⬆️
src/canmatrix/join.py 0% <0%> (ø) ⬆️
src/canmatrix/formats/arxml.py 11.73% <0%> (ø) ⬆️
src/canmatrix/tests/test_dbc.py 98.88% <100%> (+0.04%) ⬆️
src/canmatrix/tests/test_j1939_decoder.py 100% <100%> (ø)
src/canmatrix/tests/test_canmatrix.py 97.31% <100%> (+0.06%) ⬆️
src/canmatrix/j1939_decoder.py 53.96% <53.96%> (ø)
src/canmatrix/canmatrix.py 77.12% <60.65%> (-0.87%) ⬇️
src/canmatrix/formats/dbc.py 54.15% <66.66%> (+1.75%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ca617e...a00f7d8. Read the comment docs.

@ebroecker ebroecker changed the title [WIP] add "frame_by_pgn" to canmatrix add "frame_by_pgn" to canmatrix Mar 27, 2019
@ebroecker ebroecker changed the title add "frame_by_pgn" to canmatrix [WIP] j1939 updates and upgrades Mar 27, 2019
@ebroecker
Copy link
Owner Author

anyone out there to review the changes of CanId?

J1939 PGN and Destiation calculation seemed incorrect to me.

@Funth0mas
Copy link
Contributor

I don't know LKWs but quickly found an online converter which looks like the whole is a bit more complicated. No idea.

@@ -1843,7 +1861,12 @@ def __init__(self, arbitration_id): # type: (ArbitrationId) -> None
if arbitration_id.extended:
self.source = arbitration_id.id & int('0xFF', 16)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe if you're tochuing the code anyway, also refactor the two crazy int('0xFF', 16) to simply 0xFF

:rtype: Frame or None
"""

if pgn <= 0xEFFF:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of handle the pgn internals (for me "magic") here, I'd think about to defining some equality check to the CanId class itself (to prevent copy paste such if in another places).

@@ -731,14 +731,14 @@ def test_define_for_float():
def test_canid_parse_values():
can_id = canmatrix.canmatrix.CanId(canmatrix.ArbitrationId(id=0x01ABCD02, extended=True))
assert can_id.source == 0x02
assert can_id.destination == 0x01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of coure I wrote the tests to match the current code, not to match the J1939 specification :-(

@ebroecker
Copy link
Owner Author

I merged the CanID-Code a long time ago. I had no idea about J1939.

I also think, there is more wrong with it and it should be completely rewritten...
I'll try to do like spec. But I don't have any example data here...

@Funth0mas
Copy link
Contributor

The core code of the online javascript converter, which works with bits-in-string, not integers, is

idAsBinary = string_representing_bites_in_the_can_id
var sa = idAsBinary.slice(-8);  // byte
var ps = idAsBinary.slice(-16, -8);  // byte
var pf = idAsBinary.slice(-24, -16);  // byte
var edp = idAsBinary.slice(-26, -25); // single bit
var priority = idAsBinary.slice(-29, -26); //3 bits
if(int(pf) >= 240){
 pgn=pf+ps;  // note this is not aritmhmetic but string addition
}else{
 pgn=pf+'00000000';  //kind of zero the lower byte
}

It think the >= 240 (>= 0xF0) is the main magic. If the converter works correctly, obviously.

Copy link
Contributor

@Funth0mas Funth0mas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shall (later) add one test to cover the if self.pf >= 240 condition but otherwise I see no more problems.

@MatinF
Copy link

MatinF commented Mar 29, 2019

Hi all,

We're very interested in this functionality and I'd be happy to provide some input on J1939 - and answer any questions I can. We work with numerous J1939 end users across the globe. J1939 is special since it is a common standard across most heavy duty vehicles.

We've recently done a more detailed version of our online J1939 CAN ID to PGN converter, which I think can be useful in getting a detailed understanding of the inner workings in going from a J1939 29 bit CAN ID to the J1939 PGN. Also, this J1939 intro may also be helpful.

The key to understand about converting J1939 data vs. other CAN data is that the matching of IDs should be based only on the PGN, not the full 29 bit CAN ID. Therefore, J1939 has to be handled as a special case in the code, where the PGN is extracted from the full CAN ID - both from the raw data being converted, as well as the J1939 DBC file itself.

The J1939 PGN of a 29 bit CAN ID starts at bit 9, with length 18 (indexed from 1) - see also the below illustration:
dfma

Example:
29 bit CAN ID (hex): CF00400
18 bit J1939 PGN (hex): F004

Here, the PGN F004 equals 61444 in DEC, which corresponds in the J1939 standard to Electronic Engine Controller 1. For a detailed breakdown of how to extract the PGN, I suggest checking out the converter I linked as it breaks down the CAN ID into sub-components - and reconstructs these into the PGN.

For the sake of illustration, I've added below an example of converting 2 different J1939 CAN IDs to illustrate that they both result in the same J1939 PGN, despite having different source addresses.

Further, I've added the corresponding J1939 DBC entry (both incl/excl. the 3 bit "extended ID flag") a third composition - yet still results in the same PGN. This allows the two different CAN IDs to trigger the same J1939 conversion when using adequate conversion methodology.
J1939_ID_vs_PGN

Below I've attached a sample MDF4 (and CSV) log file containing one repeated CAN frame with CAN ID 0CF00400 and the same data bytes F4 DE DE 30 28 FF F0 FF. Using standard DBC conversion, this file will not yield the right result as there is no match in the DBC ID specifically 0CF00400 (the DBC has 29 bit CAN ID CF004FE). However, using PGN matching you'll be able to convert the data and find that the scaled value of EngSpeed is 1541 rpm:

J1939_MDF4_DBC_SAMPLE.zip

J1939 is one of the most used protocols globally in our experience - so adding the support for this as a special case would be worthwhile I believe. We're specifically interested in having this implemented as we hope to use J1939 conversion via ASAMMDF, which depends on canmatrix.

Happy to answer any questions on this topic!

Best,
Martin

@ebroecker
Copy link
Owner Author

ebroecker commented Mar 29, 2019

@MatinF
Hi Martin,

thanks for these lots of information.
I learned now that there is also a J1939 for 18-Bit CANIDs. ...

I'll add a checkbox-list to top of this PR with todos for J1939-PGN support

In branch ´j1939_decode´ I started with a j1939 decoder.
Currently only supports decoding BAM-Messages. But I also want to decode well-known SPN.
I have already found some Information about SPN decoding, but I'd be happy to get some correct example data for BAM, CMDT and several well known SPN .

@Funth0mas
Copy link
Contributor

I thing the 18 bits is misunderstanding. He meant that PGN is 18 bits length (and classic can is 11 bits only). @MatinF , the j1939 works only with extended IDs, right?

@ebroecker
Copy link
Owner Author

@Funth0mas
you are right, I need more coffee now!

@ebroecker
Copy link
Owner Author

@Funth0mas
what do you think about dismissing the CanId class and add j1939-functionality to ArbitrationId-Class?
Just a thought...

@Funth0mas
Copy link
Contributor

That might be a good idea if we only need to parse the id no pgn etc. What will be PNG value for a standard CAN id? None? Exception?

@MatinF
Copy link

MatinF commented Mar 29, 2019

Hi guys,

To clarify, what I mean is not that there will be an 18 bit CAN ID observed in J1939 raw data. The J1939 raw data will always have the extended 29 bit CAN IDs. You can see an example of raw J1939 data from a truck below:

Timestamp;Type;ID;Data
09T120102191;1;c00000b;fcffff7dffffffff
09T120102192;1;cef27fd;20fffae1ff00ffff
09T120102192;1;cffcafd;c0fffffffffff800
09T120102193;1;c000003;ecffffffffffffff
09T120102194;1;cf00203;cc00000000b812ff
09T120102195;1;18fe4a03;fffcffffffffffff
09T120102196;1;c010305;ccfffaffff204e0a

For each of the above IDs, you would need to extract the PGN (i.e. as per the method outlined in the illustration I parsed and in the converter Google Sheet). To verify that your CAN-ID to PGN extraction method leads to the right results, you can use our converter.

My suggestion would be that you maintain the current methodology for DBC conversion for both regular/extended CAN IDs as the default. Ideally, the user can then somewhere specify that he wants to convert J1939 data - as a special case.

When the user selects this method, the code will look for PGNs in the raw data - and compare this with the PGNs in the loaded DBC file.

Hope clear,
Martin

@Funth0mas
Copy link
Contributor

Martin, the "conversion" in canmatrix terminology is rather the catalog conversion between dbc, arxml, json etc. There are no raw data from CAN.
But what you're talking about is rather data representation, the Frame.decode() etc, right? FAIK there is no batch can-trace representer in this project, which would be provided by dbc (catalog) and asc (can trace) data providing parsed messages from the whole can trace.

You can programatically load the dbc to a CanMatrix object, and than for every message from your CAN trace convert the CANId to PGN (what is this ticket about) and than call matrix.frame_by_pgn(pgn).decode(my_raw_data) or something like that.


:rtype: tuple"""

if not self.extended:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be omitted, exception would be raised by self.j1939_destination when creating the return tuple.

src/canmatrix/canmatrix.py Show resolved Hide resolved
return _pgn

@property
def j1939_tuples(self): # type: () -> typing.Tuple[int, int, int]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to rename to j1939_tuple as it is single tuple carrying three values.

src/canmatrix/canmatrix.py Outdated Show resolved Hide resolved
Funth0mas and others added 4 commits April 2, 2019 08:55
@Funth0mas
Copy link
Contributor

You need to remove the CanId (I added recently) from the canmatrix/init.py to fix the build. Sorry :-)

ebroecker and others added 4 commits April 2, 2019 19:50
work on hints from Funth0mas
j1939-dbc is not validated in any way, so its just a compilation of informations I found in the net
make dbc more robust
@MatinF
Copy link

MatinF commented Apr 4, 2019

Thanks for the clarification on the conversion Funth0mas. My use case is "indirect" via the asammdf module, which has a dependency on canmatrix. Hence I'm hoping that these updates will pave the way for PGN based conversion of *.mf4 files in that project, as that would be really useful to a lot of people :-)

@ebroecker
Copy link
Owner Author

@MatinF
Do you have some example J1939-Can-Trace with J1939-transportprotocol in?

@MatinF
Copy link

MatinF commented Apr 4, 2019

Hi ebroecker,

Below is a MDF file (MDF 4.11) and 2x demo J1939 DBCs for PGN 61444 (EEC1).

The MDF data is simulated J1939 data in the buslogging format. It includes a number of CAN IDs (all with constant data byte frames), incl. the extended CAN ID CF004FE (which equals PGN 61444).

I've included two DBC demo files, both for PGN 61444. One DBC has an "exact" 29 bit match vs. the MDF file, i.e. it uses CAN ID CF004FE. The other has CAN ID CF00400. Both DBC files should be able to convert the MDF file when correctly utilizing the J1939 PGN match method - but only the CF004FE DBC will work with the regular DBC matching methodology.

Hope that is clear, else let me know. Happy to provide further input on this.

DBC_MDF.zip

@ebroecker
Copy link
Owner Author

@MatinF
I tried your mdf, I found only EEC1, CCVS and PGN 64587 (arbitration_id 0x18fc4bfe)

@MatinF
Copy link

MatinF commented Apr 6, 2019

@ebroecker
The MDF and the DBC in my zip is a "sample" with simulated data, so it should only return a limited set of data - so I'd assume this to be the expected behavior. I tried to supply a limited example to make it clear whether it would convert with both DBC files or not.

Let me know if further clarification is required, I'd be happy to provide input.

@MatinF
Copy link

MatinF commented Apr 7, 2019

Also, one quick note: A J1939 DBC may sometimes contain meta info indicating that it's a J1939 DBC (rather than some regular 29 bit CAN DBC). However, in our experience it's not always the case that such a meta info exists.

I'd therefore assume that the implementation would require that the user explicitly chooses to use J1939 decoding - rather than an automatic attempt to detect that a J1939 DBC is utilized.

Hope clear,
Martin

@ebroecker
Copy link
Owner Author

@MatinF
For now CanMatrix.decode takes the j1939 meta info into account and tries to decode a given frame by finding it by arbitration_id. If no arbitration_id matches, but j1939 database is detected it trys to match by pgn (only if frame is marked as j1939-frame by metadata).

Additionally I add canmatrix.j1939_decoder.j1939_decoder for explicit decode j1939 data.
This module additionally comes along with it's own j1939.dbc which should decode all well known j1939-frames (whereas the j1939.dbc is for sure not complete or correct for every pgn).
This own j1939.dbc is only used, if no match was found with given canmatrix.

Also I wanted this decoder to get arround with BAM and J1939-TP messages.
For BAM I have some little test, but for TP I don't have any sample data.

For an example usage of canmatrix.j1939_decoder.j1939_decoder look at src/canmatrix/tests/test_j1939_decoder.py

N.B: this is everything only in this PR - not merged to development yet.

@MatinF
Copy link

MatinF commented Apr 13, 2019

Hi ebroecker,

Can you provide an example of the DBC meta data you use to distinguish J1939 frames from regular extended CAN ID frames? That would help us understand how to ensure that this is done consistently when using this functionality.

Thanks,
Martin

@ebroecker
Copy link
Owner Author

Hi @MatinF ,

the j1939.dbc in this pr (which is merged already) contains the information.

BA_DEF_ BO_ "VFrameFormat" ENUM "StandardCAN","ExtendedCAN","StandardCAN_FD","ExtendedCAN_FD","J1939PG";

BA_ "VFrameFormat" BO_ 2147483648 4;

At last this means, that there is an Attribute VFrameFormat with the value J1939PG.

at last the a frame in canmatrix gets the attribute is_j1939, this is done here:

if "J1939PG" in frame.attributes.get("VFrameFormat", ""):
frame.is_j1939 = True

@ebroecker ebroecker deleted the frame_by_pgn branch May 6, 2019 13:23
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

Successfully merging this pull request may close these issues.

4 participants