Repository navigation
fix: arrival budget, diagnostics, and protocol-sync robustness - #12
Merged
Merged
Conversation
`split_lines` split on '\n' only, and ECMAScript's `.` does not match '\r', so the `(.*)$` tail of the `fw_defines` regex could not absorb the trailing CR of a CRLF line. Every `#define CMD_*` / `RSP_*` line then failed to match. The result was a **false drift report**: on a CRLF firmware tree the suite failed 6/12 and printed "SDK declares commands the firmware lacks (drift)" for all 48 commands, while the real cause was a parser that had matched nothing. The `min_expected` floor is what made it loud instead of silently green — that part of the design is right; only the diagnosis pointed the wrong way. Strip a trailing '\r' in `split_lines` (per-line and final-line path) and pin it with `protocol_sync_split_lines_tolerates_crlf`. Verified: the same CRLF firmware tree goes from 6/12 fail to 13/13 pass.
Four related fixes on the read/diagnose side. No wire-protocol change.
1. `Arm::arrive()` no longer gives up on a wall-clock budget. `move_timeout` is
now a **no-progress** budget: while the arm is still moving at a meaningful
rate (`max|dq| >= dq_tol`, the same yardstick the arrival criterion uses) the
deadline is pushed out; a hard cap of 4x the budget bounds the wait so a
jammed arm cannot extend it forever.
Measured on hardware: homing from J4 = -3.06 rad (175 deg of travel) takes
17.7 s and used to be reported as "未到位, 超时 15.0s" **while J4 was still
moving at 0.21 rad/s** — a healthy move reported as a failure, and a second
`home` was needed to finish it. It is now one `home` call that returns on
arrival.
2. The timeout report now separates the two cases: still moving ("not a failure,
just not enough patience — raise move_timeout or pass a longer timeout") vs.
genuinely stalled ("未到位, 超时 Ns (已静止: ...)"), the latter keeping the
original prefix.
3. `CART_ERR_LIMIT` (err=4) now names the axis. The firmware's 0x4E payload is
8 bytes and carries no axis number, so the SDK infers the tightest axis
locally from the last cached state plus the soft limits, and says what to do
next. It reads only the local cache (no wire traffic on an error path),
degrades to the original message if anything is unavailable, and never
throws. Measured message: "...最紧的是 J4, 距软限位仅 8.1 mrad (q=-3.0634,
限位 -3.0715..+0.0175)...先 movej 到关节空间中部的"工作位形"再发笛卡尔".
4. `litearm-cpp status` prints `joint_fault`, listing the faulted axes when it
is non-zero. The 0x06 rejection tells the user to look at `joint_fault`, but
the CLI printed only `flags` and the per-joint `err` byte — a different
field — so the faulted axis was invisible.
Tests: two criteria in test_commands (a still-moving arm must not time out; a
stalled one still must), one in test_cart (the err=4 message names the tightest
axis), plus the CRLF one above. The test double gained `dq_override`: its status
frames used to carry an empty dq vector, which made "still moving" unreachable
in offline tests.
`./build.sh` is 29/29, with and without `-DLITEARM_WERROR=ON`.
|
🎉 This PR is included in version 0.2.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes on the read/diagnose side of the SDK. No wire-protocol change.
Arm::arrive():move_timeoutis a no-progress budget, not a wall-clock one (hard cap = 4x)CART_ERR_LIMIT(err=4) names the tightest axis and says what to do nextlitearm-cpp statusprintsjoint_fault(and the faulted axes)test_protocol_sync: tolerate a CRLF firmware checkoutWhy
1 + 2 — a healthy move was being reported as a failure. Homing from
J4 = -3.06 rad(175 deg of travel) takes 17.7 s on our bench. The old code threw未到位, 超时 15.0swhile J4 was still moving at 0.21 rad/s, and a secondhomewas needed to finish the job.move_timeoutis now a no-progress budget(
max|dq| >= dq_tol— the same yardstick the arrival criterion uses), so a movethat is still making progress is not given up on; a jammed arm still is, bounded
by a 4x hard cap.
3 — the err=4 message did not say which axis. The firmware's
0x4Epayload is8 bytes and carries no axis number, so the SDK infers the tightest axis locally
from the last cached state plus the soft limits. It reads only the local cache
(no wire traffic on an error path), degrades to the original message if anything
is unavailable, and never throws.
4 — the
0x06rejection tells you to look atjoint_fault, butstatusneverprinted it — only
flagsand the per-jointerrbyte, which is a differentfield. The faulted axis was invisible at exactly the moment you need it.
5 — a CRLF firmware tree produced a false drift report.
split_linessplit on\nonly and ECMAScript's.does not match\r, so(.*)$could not absorbthe trailing CR: every
#define CMD_*/RSP_*line failed to match and the suitefailed 6/12 with "SDK declares commands the firmware lacks" for all 48 commands.
(The
min_expectedfloor is what kept it loud instead of silently green — thatpart of the design works; only the diagnosis pointed the wrong way.)
Verified
./build.sh: 29/29, and 29/29 with-DLITEARM_WERROR=ON.未到位, 超时 15.0splus asecond
home; after: onehome doneat 17.7 s.red (
CHECK(used > 300)).Notes
testing::FakeTransportgaineddq_override: its status frames used to carryan empty dq vector, which made "still moving" unreachable in offline tests.