feat(machine): repeatable work zero via homing (save/restore offset)#242
Conversation
📝 WalkthroughWalkthroughФункциональность сохранения и восстановления рабочего нуля расширяет ЧПУ интерфейс: добавлены утилиты для расчета нулевой точки и обнаружения поддержки homing в прошивке, машинное состояние отслеживает параметр ChangesФункциональность сохранения и восстановления work-zero
Sequence DiagramsequenceDiagram
participant User as Пользователь
participant DRO as Компонент Dro
participant Profile as CncProfile
participant MachineStore as machineStore
participant API as api.machine
participant Firmware as Прошивка ЧПУ
rect rgb(200, 150, 255)
Note over User,Firmware: Сценарий: сохранение work-zero
User->>DRO: нажимает "Сохранить ноль"
DRO->>MachineStore: получает workZeroFromStatus()
MachineStore-->>DRO: координаты {x, y}
DRO->>Profile: setCncProfile({workZeroMm: {x, y}})
Profile-->>DRO: сохранено
DRO-->>User: показывает "Ноль сохранен X/Y"
end
rect rgb(150, 200, 255)
Note over User,Firmware: Сценарий: восстановление work-zero
User->>DRO: нажимает "Восстановить ноль"
DRO->>DRO: проверяет canRestore (connected, homingAvailable, workZeroMm, movable)
DRO->>API: restoreWorkZero({x, y})
API->>Firmware: отправляет $H (home)
Firmware-->>MachineStore: status state = "home"
MachineStore-->>API: ожидает состояние "idle/jog"
Firmware-->>MachineStore: status state = "idle"
MachineStore-->>API: готово
API->>Firmware: отправляет G10 L2 P1 X/Y
Firmware-->>DRO: выполнено
DRO-->>User: "Ноль восстановлен" или Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cuprum-ui/src/lib/workZero.test.ts (1)
4-20: 💤 Low valueОпционально: добавить тесты для граничных случаев
Если в
workZeroFromStatusбудет добавлена проверка границ массивов (как предложено в обзореworkZero.ts), стоит добавить соответствующий тест, проверяющий выброс ошибки при передаче коротких массивов.🧪 Пример дополнительного теста
it("throws on short arrays", () => { expect(() => workZeroFromStatus([10], [0, 0, 0])).toThrow(); expect(() => workZeroFromStatus([10, 20, 0], [0])).toThrow(); expect(() => workZeroFromStatus([], [])).toThrow(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cuprum-ui/src/lib/workZero.test.ts` around lines 4 - 20, Add tests to cuprum-ui/src/lib/workZero.test.ts that verify workZeroFromStatus throws when passed short arrays: call workZeroFromStatus with a single-element mpos or wpos (e.g., [10], [0,0,0] and [10,20,0], [0]) and with empty arrays and assert they throw; reference the existing describe block for workZeroFromStatus and add an it("throws on short arrays", ...) that contains these expect(...).toThrow() checks to cover the boundary validation proposed in workZero.ts.cuprum-ui/src/lib/workZero.ts (1)
3-8: ⚡ Quick winРекомендация: добавить проверку границ массивов
Функция обращается к индексам
[0]и[1]массивовmposиwposбез проверки их длины. Хотя в нормальной работеMachineStatusвсегда содержит 3-элементные массивы, при некорректном парсинге данных прошивки возможен доступ к несуществующим элементам, что приведёт кNaNв результате. ЭтиNaNбудут сохранены в профиль и при восстановлении отправлены в станок какG10 L2 P1 XNaN YNaN, что может вызвать непредсказуемое поведение.Рекомендуется добавить защитную проверку для повышения надёжности.
🛡️ Предлагаемое исправление
export function workZeroFromStatus( mpos: readonly number[], wpos: readonly number[], ): { x: number; y: number } { + if (mpos.length < 2 || wpos.length < 2) { + throw new Error("mpos and wpos must have at least 2 elements (X, Y)"); + } return { x: mpos[0] - wpos[0], y: mpos[1] - wpos[1] }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cuprum-ui/src/lib/workZero.ts` around lines 3 - 8, The function workZeroFromStatus reads mpos[0], mpos[1], wpos[0], wpos[1] without bounds checks which can produce NaN if arrays are short; update workZeroFromStatus to validate the lengths (or individual elements) for mpos and wpos and fall back to safe numeric defaults (e.g. 0) or coerce to numbers when an index is missing or not a finite number so the returned {x,y} are always numeric; locate and update the workZeroFromStatus function to perform these checks before computing x and y.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cuprum-ui/src/lib/restoreWorkZero.ts`:
- Around line 21-29: When the GUARD_MS timeout elapses in restoreWorkZero (use
symbols GUARD_MS, guardStart, st), do not fall through to the completion wait
for non-homing states; instead treat any state other than "home" or "run" as a
failure and abort so restoreZeroGcode() is never executed on an un-homed
machine. Change the branch that currently does "break" for states like
"door"/"hold"/"check"/"sleep"/"unknown" to throw an error (or return a clear
failure) with a descriptive message (e.g. "homing did not start/completed") so
only a successful homing path reaches the completion wait and eventual call to
restoreZeroGcode().
---
Nitpick comments:
In `@cuprum-ui/src/lib/workZero.test.ts`:
- Around line 4-20: Add tests to cuprum-ui/src/lib/workZero.test.ts that verify
workZeroFromStatus throws when passed short arrays: call workZeroFromStatus with
a single-element mpos or wpos (e.g., [10], [0,0,0] and [10,20,0], [0]) and with
empty arrays and assert they throw; reference the existing describe block for
workZeroFromStatus and add an it("throws on short arrays", ...) that contains
these expect(...).toThrow() checks to cover the boundary validation proposed in
workZero.ts.
In `@cuprum-ui/src/lib/workZero.ts`:
- Around line 3-8: The function workZeroFromStatus reads mpos[0], mpos[1],
wpos[0], wpos[1] without bounds checks which can produce NaN if arrays are
short; update workZeroFromStatus to validate the lengths (or individual
elements) for mpos and wpos and fall back to safe numeric defaults (e.g. 0) or
coerce to numbers when an index is missing or not a finite number so the
returned {x,y} are always numeric; locate and update the workZeroFromStatus
function to perform these checks before computing x and y.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2095f168-7a25-4fe6-8d40-5846fc189db3
📒 Files selected for processing (10)
cuprum-ui/src/components/machine/Dro.tsxcuprum-ui/src/lib/cncProfile.tscuprum-ui/src/lib/drillGcode.test.tscuprum-ui/src/lib/drillProgram.test.tscuprum-ui/src/lib/restoreWorkZero.tscuprum-ui/src/lib/workZero.test.tscuprum-ui/src/lib/workZero.tscuprum-ui/src/locales/en/machine.jsoncuprum-ui/src/locales/ru/machine.jsoncuprum-ui/src/machineStore.ts
| if (Date.now() - guardStart > GUARD_MS) { | ||
| // Homing never started within the guard window. If the machine is still | ||
| // idle/jog, $H was effectively a no-op (e.g. rejected) — refuse to apply | ||
| // the offset on an un-homed machine (would set a wrong datum). | ||
| if (st === "idle" || st === "jog") { | ||
| throw new Error("homing did not start"); | ||
| } | ||
| break; // left idle (some other state) — fall through to the completion wait | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GRBL state machine behavior when homing command fails or is rejected in non-idle states
💡 Result:
In GRBL, the system state is managed by the sys.state variable, which enforces specific operational restrictions [1][2]. The $H (homing) command is designed to be executed only when the system is in an IDLE or ALARM state [3][4]. If a homing command is issued while the system is in a non-idle, restricted state (such as CYCLE or HOLD), the command will typically be rejected, and the system will return an error, commonly error: 8 (Not idle) [5][6]. A critical historical issue existed in the GRBL state machine regarding homing failures: if the system state was updated to STATE_HOMING before validating the entire command or its execution requirements, the controller could become "stuck" in the homing state [3]. In such cases, because the system incorrectly perceived itself as still performing a homing cycle, it would reject subsequent commands, including further $H attempts, as it was not in the required IDLE or ALARM state [3]. Modern implementations of GRBL have addressed this by ensuring that the sys.state is only transitioned to STATE_HOMING after all initial safety checks and command validations are successfully completed [3]. If a failure occurs during the homing process itself (e.g., limit switch failure, abort, or timeout), the system typically transitions into an ALARM state [4][2]. Once in an ALARM state, most operations are locked until the user acknowledges the alarm (e.g., via a system reset or an $X override command) and successfully completes a homing cycle [4].
Citations:
- 1: https://github.com/charlch/grbl/blob/041109410ad6593050ec2e0b1b0d366e236bb80a/system.h
- 2: https://deepwiki.com/gnea/grbl/2.1-state-machine-and-control-flow
- 3: Fixed Homing state bug gnea/grbl#790
- 4: https://github.com/gnea/grbl/blob/master/doc/markdown/commands.md
- 5: https://github.com/terjeio/grblHAL/blob/master/doc/csv/error_codes_en_US.csv
- 6: Motor starts moving continously on "error : not idle" grbl/grbl#1337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant portion of restoreWorkZero.ts
FILE="cuprum-ui/src/lib/restoreWorkZero.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
# Show lines around the guard phase and also the completion wait / where offset is applied
sed -n '1,200p' "$FILE" | nl -ba | sed -n '1,200p' || true
# Narrow search for the specific guard/completion logic
rg -n "guard|GUARD_MS|completion|work[- ]zero|offset|homing did not start|\$H" "$FILE" -S || true
# Find where MachineStateName is defined (or imported) and what values exist
rg -n "MachineStateName" "cuprum-ui/src/lib/restoreWorkZero.ts" -S || trueRepository: fixcik/cuprum
Length of output: 605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="cuprum-ui/src/lib/restoreWorkZero.ts"
echo "=== file stats ==="
wc -l "$FILE"
echo "=== file head (with line numbers via awk) ==="
awk '{printf "%5d: %s\n", NR, $0}' "$FILE" | sed -n '1,220p'
echo "=== exact guard/completion/offset parts ==="
rg -n "guardStart|GUARD_MS|completion|work[- ]zero|offset|homing|G54|\$H|MachineStateName" "$FILE" -S
echo "=== find MachineStateName definition/usages ==="
rg -n "type\s+MachineStateName|enum\s+MachineStateName|MachineStateName" "cuprum-ui/src" -S
echo "=== print any state mapping/table near MachineStateName (if found) ==="
# best-effort: show surrounding lines for each match
rg -n "MachineStateName" "cuprum-ui/src" -S | cut -d: -f1 | sort -u | head -n 20 | while read -r f; do
echo "--- $f ---"
awk '{printf "%5d: %s\n", NR, $0}' "$f" | sed -n '1,260p'
doneRepository: fixcik/cuprum
Length of output: 17253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="cuprum-ui/src/lib/workZero.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
wc -l "$FILE"
echo "=== file excerpt (with line numbers) ==="
awk '{printf "%5d: %s\n", NR, $0}' "$FILE" | sed -n '1,260p'
echo "=== search for restoreZeroGcode implementation ==="
rg -n "restoreZeroGcode|G54|offset|work.*zero|homing|\\$H|GRBL" "$FILE" -SRepository: fixcik/cuprum
Length of output: 1189
Guard timeout не должен пропускать в completion wait при состояниях, отличных от home/run
В cuprum-ui/src/lib/restoreWorkZero.ts при истечении GUARD_MS (строки ~21-29) ошибка выбрасывается только если st === "idle" || st === "jog", а при door/hold/check/sleep/unknown выполняется break, затем в completion wait смещение всё равно применяется после возврата в idle/jog. restoreZeroGcode() без дополнительных проверок отправляет G10 L2 P1 ..., то есть это может привести к установке G54 на незахомированной машине.
Это особенно рискованно для GRBL: $H обычно разрешён только в IDLE/ALARM, а в прочих состояниях он отклоняется (например, error: 8 Not idle) и контроллер может позже вернуться в IDLE без успешного хоминга — текущая логика может ошибочно трактовать это как завершение.
Предлагаемая более строгая проверка
if (Date.now() - guardStart > GUARD_MS) {
- // Homing never started within the guard window. If the machine is still
- // idle/jog, $H was effectively a no-op (e.g. rejected) — refuse to apply
- // the offset on an un-homed machine (would set a wrong datum).
- if (st === "idle" || st === "jog") {
+ // Homing must have entered home/run state within the guard window.
+ if (st !== "home" && st !== "run") {
- throw new Error("homing did not start");
+ throw new Error(`homing did not start (state: ${st})`);
}
break; // left idle (some other state) — fall through to the completion wait
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Date.now() - guardStart > GUARD_MS) { | |
| // Homing never started within the guard window. If the machine is still | |
| // idle/jog, $H was effectively a no-op (e.g. rejected) — refuse to apply | |
| // the offset on an un-homed machine (would set a wrong datum). | |
| if (st === "idle" || st === "jog") { | |
| throw new Error("homing did not start"); | |
| } | |
| break; // left idle (some other state) — fall through to the completion wait | |
| } | |
| if (Date.now() - guardStart > GUARD_MS) { | |
| // Homing must have entered home/run state within the guard window. | |
| if (st !== "home" && st !== "run") { | |
| throw new Error(`homing did not start (state: ${st})`); | |
| } | |
| break; // left idle (some other state) — fall through to the completion wait | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cuprum-ui/src/lib/restoreWorkZero.ts` around lines 21 - 29, When the GUARD_MS
timeout elapses in restoreWorkZero (use symbols GUARD_MS, guardStart, st), do
not fall through to the completion wait for non-homing states; instead treat any
state other than "home" or "run" as a failure and abort so restoreZeroGcode() is
never executed on an un-homed machine. Change the branch that currently does
"break" for states like "door"/"hold"/"check"/"sleep"/"unknown" to throw an
error (or return a clear failure) with a descriptive message (e.g. "homing did
not start/completed") so only a successful homing path reaches the completion
wait and eventual call to restoreZeroGcode().
1 participant
Повторяемый рабочий ноль через базирование — чтобы не выставлять 0 джогом каждый раз при той же оснастке.
Что внутри
workZeroMm: {x,y}|null— машинные XY рабочего нуля (только XY; Z всегда touch-off вручную).MPos−WPos) в профиль.$H→ дождатьсяIdle→G10 L2 P1 X Y(восстанавливает G54-офсет в сохранённую точку). Если homing не стартовал — офсет НЕ применяется (ошибка).$$, парсим$22; при$22=0восстановление неактивно с подсказкой (показывается только если ноль сохранён).lib/workZero.ts(workZeroFromStatus/parseHomingEnabled/restoreZeroGcode) + юнит-тесты.Независимое ревью — APPROVE WITH NITS, обе правки внесены (guard на старт базирования; подсказка homing только при сохранённом нуле).
pnpm test(324) +pnpm buildзелёные. Проверка на железе — чек-лист в issue.Refs #240
Summary by CodeRabbit
Новые возможности