Equipment-and-Tool-Institute / j1939-84

J1939-84 implementation for etools.org
MIT License
7 stars 6 forks source link

Tool Issues a Failure Incorrectly When Check Freeze Frame for DTCs #1211

Closed naroner closed 1 month ago

naroner commented 1 year ago

Tool doesn't find the Freeze Frame DTC that was reported earlier in DM23

Start Test 8.10 - DM25: Expanded freeze frame 14:25:23.5735 Destination Specific DM25 Request to Engine #1 (0) 14:25:23.5760 18EA00F9 [3] B7 FD 00 (TX) 14:25:23.5860 18E8FF00 [8] 01 FF FF FF F9 B7 FD 00 Acknowledgment from Engine #1 (0): Response: NACK, Group Function: 255, Address Acknowledged: 249, PGN Requested: 64951 14:25:23.5870 Destination Specific DM25 Request to Engine #2 (1) 14:25:23.5890 18EA01F9 [3] B7 FD 00 (TX) 14:25:24.0060 1CFDB701 [134] 85 44 15 01 01 C8 02 00 00 4F 25 65 F9 16 EE 30 5A 47 C5 87 69 52 08 01 20 25 55 30 A2 01 00 16 D2 3A 8B 8B 80 3B 60 1F C0 28 20 32 C6 A0 E1 E1 CD D0 09 1F 9A 24 D4 62 D4 62 D4 62 D4 62 D4 62 D4 62 D4 62 76 28 F2 4F 4F CC 24 FF 14 03 33 AB 35 B6 34 12 35 65 00 A0 13 47 00 00 00 00 00 00 00 01 03 AB 08 00 DA 45 33 58 1B AB 35 12 35 56 04 60 52 60 52 01 FF FE 00 85 00 E2 79 04 FF FF FF FE 3D 06 3E 06 DM25 from Engine #2 (1): Freeze Frames: [ Freeze Frame: { Length: 129 DTC 5444:1 - Engine Crankcase Breather Oil Separator Speed, Data Valid But Below Normal Operational Range - Most Severe Level - 1 times SPN Data: C8 02 00 00 4F 25 65 F9 16 EE 30 5A 47 C5 87 69 52 08 01 20 25 55 30 A2 01 00 16 D2 3A 8B 8B 80 3B 60 1F C0 28 20 32 C6 A0 E1 E1 CD D0 09 1F 9A 24 D4 62 D4 62 D4 62 D4 62 D4 62 D4 62 D4 62 76 28 F2 4F 4F CC 24 FF 14 03 33 AB 35 B6 34 12 35 65 00 A0 13 47 00 00 00 00 00 00 00 01 03 AB 08 00 DA 45 33 58 1B AB 35 12 35 56 04 60 52 60 52 01 FF FE 00 85 00 E2 79 04 FF FF FF FE 3D 06 3E 06 } ]

14:25:24.0090 Destination Specific DM25 Request to Exhaust Emission Controller (61) 14:25:24.0110 18EA3DF9 [3] B7 FD 00 (TX) 14:25:24.0135 18FDB73D [8] 00 00 00 00 00 FF FF FF DM25 from Exhaust Emission Controller (61): Freeze Frames: [ No Freeze Frames ]

FAIL: 6.8.10.2.a - DTC(s) reported in the freeze frame by Engine #2 (1) did not include either the DTC reported in DM12 or DM23 earlier in this part

End Test 8.10 - DM25: Expanded freeze frame

. . .

Start Test 8.4 - DM23: Emission related previously active DTCs 14:25:17.9860 Global DM23 Request 14:25:17.9885 18EAFFF9 [3] B5 FD 00 (TX) 14:25:17.9925 18FDB53D [8] 03 FF 00 00 00 00 FF FF DM23 from Exhaust Emission Controller (61): MIL: off, RSL: off, AWL: off, PL: not supported, No DTCs

14:25:17.9955 18FDB501 [8] 47 FF 44 15 01 01 FF FF DM23 from Engine #2 (1): MIL: on, RSL: off, AWL: on, PL: not supported DTC 5444:1 - Engine Crankcase Breather Oil Separator Speed, Data Valid But Below Normal Operational Range - Most Severe Level - 1 times

End Test 8.4 - DM23: Emission related previously active DTCs

battjt commented 1 year ago

"any v all" type of bug.

battjt commented 1 year ago

Fail if DTC(s) reported in the freeze frame does not include either the DTC reported in DM12 or the DTC reported in DM23 earlier in this part.

was interpreted as

      packets.forEach(pp -> {
            var ffDTCs = pp.getFreezeFrames().stream().map(ff -> ff.getDtc()).collect(Collectors.toList());
            if (!ffDTCs.containsAll(getDM12DTCs(pp.getSourceAddress()))
                    || !ffDTCs.containsAll(getDM23DTCs(pp.getSourceAddress()))) {
                addFailure("6.8.10.2.a - DTC(s) reported in the freeze frame by " + pp.getModuleName()
                        + " did not include either the DTC reported in DM12 or DM23 earlier in this part");
            }
        });

I think the issue is not that it's a distributed module system, but the ambiguity of logic operators in English.

This fail is also triggered if the DM12 reported a DTC that is not in the FF, but the report didn't include the DM12 for engine #2.

There are two ways to interpret the requirement:

  1. The way it is written. If the FF does not contain the DM12 DTCs or the FF does not contain the DM23 DTCs. this may be more clearly written as

            if (!ffDTCs.containsAll(getDM12AndDM23DTCs(pp.getSourceAddress()))) {

    If this is the case, I think we should make two different error messages, so it is clear which DM is was not supported by FF.

  2. If the FF does not contain the DM12 DTCs and the FF does not contain the DM23 DTCs. This would allow some DTCs to not have FF, but not all.

            if (!ffDTCs.containsAll(getDM12DTCs(pp.getSourceAddress()))
                    && !ffDTCs.containsAll(getDM23DTCs(pp.getSourceAddress()))) {
battjt commented 1 year ago

At least one DM12 DTC must be in FF or at least one DM23 DTCs must be in FF. Eric will clarify failure criteria in task 5 artifact.

ericthomasswenson commented 1 year ago

// Pass = (There exists an X that is an element of {DM12} that is also an element of {DM25}) // Or (There exists a Y that is an element of {DM23} that is also an element of {DM25}) // Fail = (There does not exist an X that is an element of {DM12} that is also an element of {DM25}) // And (There does not exist an X that is an element of {DM23} that is also an element of {DM25})

naroner commented 1 year ago

@ericthomasswenson This is fixed in 3.1.19

ericthomasswenson commented 4 months ago

The detailed text changes for C3 where multiple implanted DTCs address added uncertainty.