Skip to content

Commit a502920

Browse files
RyodoTanakaRyodoTanaka
authored
Merge pull request #14 from OEDO-Dynamics/feat/can-tools-c-diag-ci
fix(can_tools): fix can_diag.sh error-frame classification, restore, and lock/pause ownership
2 parents 563a25c + 2cf9047 commit a502920

7 files changed

Lines changed: 1631 additions & 0 deletions

File tree

‎can_diag.sh‎

Lines changed: 670 additions & 0 deletions
Large diffs are not rendered by default.

‎tests/diag_fakes/candump‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#!/bin/bash
2+
3+
# Fake candump for testing can_diag.sh (PR-C's own fake; distinct from the
4+
# PR-A1 tests/fakes/ contract, which this file does not touch).
5+
#
6+
# Emits `candump -L` lines: "(<ts>) <iface> <id_hex>#<data_hex>". can_diag.sh
7+
# is fixed to -L (see Issue #9 review C2), so this is the only format this
8+
# fake needs to produce.
9+
#
10+
# Env:
11+
# CAN_TOOLS_FAKE_DIR base dir for scenario files (default: .)
12+
# CAN_TOOLS_CANDUMP_SCENARIO name of a file under
13+
# "$CAN_TOOLS_FAKE_DIR/candump/" to cat
14+
# verbatim instead of the built-in default
15+
# CAN_TOOLS_DIAG_CANDUMP_HANG if set, ignore SIGTERM and never exit
16+
# (simulates an unresponsive candump, for
17+
# forced-kill / SIGKILL tests)
18+
19+
fake_dir="${CAN_TOOLS_FAKE_DIR:-.}"
20+
21+
if [[ -n "${CAN_TOOLS_DIAG_CANDUMP_HANG:-}" ]]; then
22+
trap '' TERM
23+
while true; do sleep 1; done
24+
fi
25+
26+
if [[ -n "${CAN_TOOLS_CANDUMP_SCENARIO:-}" ]]; then
27+
if [[ -f "$fake_dir/candump/$CAN_TOOLS_CANDUMP_SCENARIO" ]]; then
28+
cat "$fake_dir/candump/$CAN_TOOLS_CANDUMP_SCENARIO"
29+
exit 0
30+
fi
31+
fi
32+
33+
# Default scenario: a realistic mix of ordinary traffic plus error frames
34+
# that exercise every category can_diag.sh's diag_classify_line() decodes.
35+
# can_id has CAN_ERR_FLAG (0x20000000) set for every error frame; the class
36+
# bits live in the low bits of can_id, data[1]/[2]/[3]/[4] per
37+
# linux/can/error.h. Expected counts (checked by tests/test_diag_classify.sh):
38+
# can0 class=ACK count=1 (line 2: 20000020)
39+
# can0 class=BUSOFF count=1 (line 3: 20000040)
40+
# can0 class=PROT count=2 (lines 4, 5)
41+
# can0 class=BUSERROR count=1 (line 4: 20000088)
42+
# can0 prot=TX count=1 (line 4: data[2]=0x80)
43+
# can0 loc=ACK count=1 (line 4: data[3]=0x19)
44+
# can0 prot=FORM count=1 (line 5: data[2]=0x02)
45+
# can0 class=CRTL count=1 (line 6: 20000004)
46+
# can0 ctrl=RX_WARNING count=1 (line 6: data[1]=0x04)
47+
# can1 class=TRX count=1 (line 7: 20000010)
48+
# can1 trx=CANH_SHORT_TO_GND count=1 (line 7: data[4]=0x07)
49+
cat <<'EOF'
50+
(1697000000.000001) can0 18DA31F9#0011223344556677
51+
(1697000000.000002) can0 20000020#0000000000000000
52+
(1697000000.000003) can0 20000040#0000000000000000
53+
(1697000000.000004) can0 20000088#0000801900080000
54+
(1697000000.000005) can0 20000008#0000020000000000
55+
(1697000000.000006) can0 20000004#0004000000000000
56+
(1697000000.000007) can1 20000010#0000000007000000
57+
EOF
58+
59+
exit 0

‎tests/diag_fakes/ip_slow_berr_off‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#!/bin/bash
2+
3+
# tests/diag_fakes/ip_slow_berr_off - wraps tests/fakes/ip (PR-A1's fake,
4+
# unmodified) and adds a delay to the "up ... berr-reporting off" call, i.e.
5+
# the RESTORE step, to test can_diag.sh being signalled a SECOND time while
6+
# cleanup is itself still in progress (Issue #9 review H3).
7+
# CAN_TOOLS_DIAG_IP_SLEEP controls the delay (default 2s).
8+
9+
real_ip="$(cd "$(dirname "$0")/../fakes" && pwd)/ip"
10+
11+
if [[ "$*" == *"berr-reporting off"* ]]; then
12+
sleep "${CAN_TOOLS_DIAG_IP_SLEEP:-2}"
13+
fi
14+
15+
exec "$real_ip" "$@"

‎tests/diag_fakes/ip_slow_berr_on‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#!/bin/bash
2+
3+
# tests/diag_fakes/ip_slow_berr_on - wraps tests/fakes/ip (PR-A1's fake,
4+
# unmodified) and adds a delay to the "up ... berr-reporting on" call, to
5+
# test can_diag.sh being signalled WHILE that call is still in flight
6+
# (Issue #9 review H2). CAN_TOOLS_DIAG_IP_SLEEP controls the delay (default
7+
# 3s).
8+
9+
real_ip="$(cd "$(dirname "$0")/../fakes" && pwd)/ip"
10+
11+
if [[ "$*" == *"berr-reporting on"* ]]; then
12+
sleep "${CAN_TOOLS_DIAG_IP_SLEEP:-3}"
13+
fi
14+
15+
exec "$real_ip" "$@"

‎tests/test_diag_classify.sh‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
#!/bin/bash
2+
3+
# Test can_diag.sh error frame classification (Issue #9 review C1-C4):
4+
# - the candump -L line regex actually matches and parses (C1)
5+
# - the format matches real `candump -L -e any,0~0,#FFFFFFFF` output (C2)
6+
# - classification follows linux/can/error.h bit-for-bit, counting every
7+
# set bit independently rather than overwriting with "last one wins" (C3)
8+
# - the test checks exact per-category COUNTS, not just that section
9+
# headings exist (C4)
10+
# Also covers M5 (sysfs stats come from CAN_TOOLS_SYSFS_ROOT via the lib's
11+
# can_read_stats(), not a hardcoded /sys/class/net path).
12+
13+
tmpdir=$(mktemp -d)
14+
# shellcheck disable=SC2064
15+
trap "rm -rf '$tmpdir'" EXIT
16+
17+
pass=0 fail=0
18+
19+
check() {
20+
if [[ "$1" == "0" ]]; then
21+
pass=$((pass+1))
22+
else
23+
fail=$((fail+1))
24+
echo "FAIL: $2"
25+
fi
26+
}
27+
28+
export CAN_TOOLS_RUN_DIR="$tmpdir/run"
29+
export CAN_TOOLS_FAKE_DIR="$tmpdir/fake"
30+
31+
fakes_path=$(cd "$(dirname "$0")/fakes" && pwd) || exit 1
32+
export CAN_TOOLS_IP_CMD="$fakes_path/ip"
33+
export CAN_TOOLS_JOURNALCTL_CMD="$fakes_path/journalctl"
34+
35+
diag_fakes_path=$(cd "$(dirname "$0")/diag_fakes" && pwd) || exit 1
36+
export CAN_TOOLS_CANDUMP_CMD="$diag_fakes_path/candump"
37+
38+
mkdir -p "$tmpdir/run" "$tmpdir/fake" "$tmpdir/fake/can0" "$tmpdir/fake/can1"
39+
40+
echo "ERROR-ACTIVE" >"$tmpdir/fake/can0/state"
41+
echo "restart_ms=100 berr=off fd=off admin=up" >"$tmpdir/fake/can0/config"
42+
echo "ERROR-ACTIVE" >"$tmpdir/fake/can1/state"
43+
echo "restart_ms=100 berr=off fd=off admin=up" >"$tmpdir/fake/can1/config"
44+
45+
mkdir -p "$tmpdir/sys/class/net/can0/statistics"
46+
mkdir -p "$tmpdir/sys/class/net/can1/statistics"
47+
echo "123" >"$tmpdir/sys/class/net/can0/statistics/rx_packets"
48+
echo "50" >"$tmpdir/sys/class/net/can0/statistics/tx_packets"
49+
echo "7" >"$tmpdir/sys/class/net/can0/statistics/rx_fifo_errors"
50+
echo "0" >"$tmpdir/sys/class/net/can0/statistics/tx_fifo_errors"
51+
echo "1" >"$tmpdir/sys/class/net/can0/ifindex"
52+
echo "100" >"$tmpdir/sys/class/net/can1/statistics/rx_packets"
53+
echo "0" >"$tmpdir/sys/class/net/can1/statistics/tx_packets"
54+
echo "0" >"$tmpdir/sys/class/net/can1/statistics/rx_fifo_errors"
55+
echo "0" >"$tmpdir/sys/class/net/can1/statistics/tx_fifo_errors"
56+
57+
export CAN_TOOLS_SYSFS_ROOT="$tmpdir/sys"
58+
export CAN_TOOLS_FAKE_TIME_FILE="$tmpdir/time"
59+
echo "1700000000" >"$CAN_TOOLS_FAKE_TIME_FILE"
60+
61+
echo "Testing can_diag.sh classification logic..."
62+
63+
cd "$(dirname "$0")/.." || exit 1
64+
65+
output=$(bash can_diag.sh 3 2>&1)
66+
rc=$?
67+
check "$rc" "can_diag.sh should exit 0 (got rc=$rc); output follows:\n$output"
68+
69+
diag_file=$(echo "$output" | grep "Diagnostics saved" | awk '{print $NF}')
70+
if [[ -n "$diag_file" && -f "$diag_file" ]]; then
71+
check 0 "can_diag.sh should create output file"
72+
else
73+
check 1 "can_diag.sh should create output file"
74+
fi
75+
76+
if [[ -f "$diag_file" ]]; then
77+
echo "DEBUG: Full diag file:" >&2
78+
cat "$diag_file" >&2
79+
80+
for section in "IP Link Show" "sysfs Statistics" "kernel log" "CAN Error Frames" "Error Frame Summary" "Interface Setup"; do
81+
grep -q "$section" "$diag_file"
82+
check $? "Output should contain '$section' section"
83+
done
84+
85+
# --- C1/C2: the regex actually matched real -L lines; raw matched error
86+
# frame lines (not the one ordinary data frame) must be echoed verbatim.
87+
grep -q '^(1697000000\.000002) can0 20000020#0000000000000000$' "$diag_file"
88+
check $? "raw -L error-frame line should be preserved verbatim (C1/C2)"
89+
90+
# the ordinary (non-error) data frame must not be written to the diag
91+
# file at all: diag_classify_line() rejects it (no CAN_ERR_FLAG) before
92+
# it would ever be echoed.
93+
if grep -qF '18DA31F9' "$diag_file"; then
94+
check 1 "ordinary (non-error) frame must not appear in the diag file"
95+
else
96+
check 0 "ordinary (non-error) frame must not appear in the diag file"
97+
fi
98+
99+
# --- C3/C4: exact expected counts per category, from the fake's own
100+
# documented scenario (tests/diag_fakes/candump).
101+
declare -A expected=(
102+
["can0 class=ACK count=1"]=1
103+
["can0 class=BUSOFF count=1"]=1
104+
["can0 class=PROT count=2"]=1
105+
["can0 class=BUSERROR count=1"]=1
106+
["can0 class=CRTL count=1"]=1
107+
["can0 prot=TX count=1"]=1
108+
["can0 loc=ACK count=1"]=1
109+
["can0 prot=FORM count=1"]=1
110+
["can0 ctrl=RX_WARNING count=1"]=1
111+
["can1 class=TRX count=1"]=1
112+
["can1 trx=CANH_SHORT_TO_GND count=1"]=1
113+
)
114+
for want in "${!expected[@]}"; do
115+
grep -qF "$want" "$diag_file"
116+
check $? "Error Frame Summary should contain exact count: '$want'"
117+
done
118+
119+
# A key that must NOT appear (sanity: no spurious class=ACK on can1, no
120+
# bit0 misfire on the FORM-only frame).
121+
if grep -qF "can1 class=ACK" "$diag_file"; then
122+
check 1 "must not classify can1 as ACK (no such frame in the scenario)"
123+
else
124+
check 0 "must not classify can1 as ACK (no such frame in the scenario)"
125+
fi
126+
127+
# --- M5: sysfs values came from CAN_TOOLS_SYSFS_ROOT (can_read_stats),
128+
# not a hardcoded /sys/class/net path - check the actual fixture values.
129+
grep -q "rx_packets: 123" "$diag_file"
130+
check $? "sysfs rx_packets should reflect CAN_TOOLS_SYSFS_ROOT fixture (M5)"
131+
grep -q "rx_fifo_errors: 7" "$diag_file"
132+
check $? "sysfs rx_fifo_errors should reflect CAN_TOOLS_SYSFS_ROOT fixture (M5)"
133+
fi
134+
135+
# berr-reporting restored to original (off) after a clean run (H1)
136+
final_berr=$(grep -o "berr=[^ ]*" "$tmpdir/fake/can0/config")
137+
[[ "$final_berr" == "berr=off" ]]
138+
check $? "berr-reporting should be restored to off (got: $final_berr)"
139+
140+
[[ ! -f "$tmpdir/run/watchdog.pause" ]]
141+
check $? "watchdog.pause should be cleaned up"
142+
143+
[[ ! -f "$tmpdir/run/lock" || ! -f "/proc/$$/fd/9" ]]
144+
check $? "Lock should be released"
145+
146+
echo ""
147+
echo "Results: $pass passed, $fail failed"
148+
149+
if (( fail > 0 )); then
150+
exit 1
151+
fi
152+
153+
exit 0

0 commit comments

Comments
 (0)