From 69c5a4c41dee487ea6f1652204204e80e0245096 Mon Sep 17 00:00:00 2001 From: Matysh Date: Fri, 31 Jul 2026 13:07:16 +0300 Subject: [PATCH] Vacuum first-use path: materialize the marker, count rectangle rooms, fix the toasts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit HP-1540-01 (High): an auto-discovered vacuum has no config marker until the device dialog is saved once, yet the live-position section was already interactive. setVac, _vacSaveMatrix and auto-calibration all did cfg.markers.find(...) and silently bailed out — while the auto-calibration toast still claimed success. Every vacuum edit now materializes a minimal marker (same id/binding the dialog Save would produce), _vacSaveMatrix reports whether the write landed, and success toasts are gated on it. HP-1540-04: the auto-calibration room matcher accepted only polygon rooms and told users their room names did not match. It goes through the shared roomPoly() now, so legacy x/y/w/h rectangles count like everywhere else. HP-1540-06: the no-rooms/no-match/rough-fit toasts pointed at the removed point calibration; they now point at the fit panel that shipped instead, and docs/VACUUM.md Setup UX describes the actual UI. Also extracted vacMapIdFromAttrs as the explicit frontend half of the map-id contract (backend half lands with HP-1540-02). Regressions: demo/smoke_vacuum_firstuse.mjs starts from cfg.markers=[] (the fixture gap the audit called out) with rectangle plan rooms and a zero map_index, and fails 11 checks on the v1.54.0 bundle; i18n unit test asserts no point/точк wording in either language. --- demo/smoke_vacuum_firstuse.mjs | 108 +++++++++++++++++++++++++++++++++ docs/VACUUM.md | 16 +++-- src/houseplan-card.ts | 71 +++++++++++++++++----- src/i18n/en.json | 6 +- src/i18n/ru.json | 6 +- src/vacuum.ts | 15 ++++- test/i18n.test.mjs | 8 +++ test/vacuum.test.mjs | 19 +++++- 8 files changed, 221 insertions(+), 28 deletions(-) create mode 100644 demo/smoke_vacuum_firstuse.mjs diff --git a/demo/smoke_vacuum_firstuse.mjs b/demo/smoke_vacuum_firstuse.mjs new file mode 100644 index 0000000..fd56403 --- /dev/null +++ b/demo/smoke_vacuum_firstuse.mjs @@ -0,0 +1,108 @@ +// v1.54.0 audit regressions (HP-1540-01/-04/-06): the FIRST-USE path. +// An auto-discovered vacuum has NO marker in the config — cfg.markers stays +// empty here, unlike smoke_vacuum.mjs which always pushes one by hand (the +// audit called that gap out explicitly). Every vacuum action must materialise +// the marker and actually persist; plan rooms are legacy RECTANGLES. +import { launch, checkAll, finish } from './serve.mjs'; +const { page, browser } = await launch(); + +const out = await page.evaluate(async () => { + const c = window.__card; + const o = {}; + const sr = () => c.shadowRoot || c.renderRoot; + const cfg = c._serverCfg; + + // legacy rectangle rooms (x/y/w/h, HP-1540-04) in the garden space where + // the auto-discovered vacuum.mower already lives + const g = cfg.spaces.find((s) => s.id === 'garden'); + g.rooms = [ + { id: 'ga', name: 'Кухня', area: 'garden', x: 0.05, y: 0.10, w: 0.25, h: 0.30 }, + { id: 'gb', name: 'Зал', x: 0.40, y: 0.10, w: 0.25, h: 0.30 }, + { id: 'gc', name: 'Спальня', x: 0.05, y: 0.50, w: 0.25, h: 0.30 }, + ]; + // robot rooms: same centres in "robot mm" (scale 4), map_index is a + // NUMERIC ZERO — the cross-runtime zero-map contract (HP-1540-02) + const robotRooms = { + 1: { name: 'Кухня', x0: 300, y0: 600, x1: 1100, y1: 1400 }, + 2: { name: 'Зал', x0: 1700, y0: 600, x1: 2500, y1: 1400 }, + 3: { name: 'Спальня', x0: 300, y0: 2200, x1: 1100, y1: 3000 }, + }; + const camAttrs = { vacuum_position: { x: 700, y: 1000, a: 0 }, map_index: 0, rooms: robotRooms }; + c.hass = { ...c.hass, + entities: { ...c.hass.entities, + 'camera.mower_map': { entity_id: 'camera.mower_map', device_id: 'd_mower', platform: 'demo' } }, + states: { ...c.hass.states, + 'camera.mower_map': { state: 'idle', attributes: camAttrs } } }; + c._regSignature = ''; + c.requestUpdate(); await c.updateComplete; + + // spy on config writes: a success toast is only honest after one of these + const writes = []; + const realWS = c.hass.callWS; + c.hass.callWS = async (m) => { if (m.type === 'houseplan/config/set') writes.push(m); return realWS(m); }; + + const dev = c._devices.find((x) => x.id === 'd_mower'); + o.devFound = !!dev; + o.freshNoMarker = (cfg.markers || []).length === 0 && !dev.marker; + + // ---- auto-calibration from scratch (HP-1540-01 + HP-1540-04) ---- + c._vacAutoCalibrate(dev); + await c.updateComplete; + const m1 = (cfg.markers || []).find((x) => x.id === 'd_mower'); + o.markerMaterialised = !!m1 && m1.binding === 'device:d_mower'; + const cal0 = m1?.vacuum?.calibration?.['0']; + o.rectRoomsCalibrated = Array.isArray(cal0) && cal0.length === 6 && cal0.every(Number.isFinite); + o.sourceStored = m1?.vacuum?.source === 'camera.mower_map'; + // the matrix really maps robot mm → canvas: centre of «Кухня» → (175, 250) + const ap = (mm, x, y) => [mm[0] * x + mm[1] * y + mm[2], mm[3] * x + mm[4] * y + mm[5]]; + const hit = cal0 ? ap(cal0, 700, 1000) : [0, 0]; + o.matrixMapsRooms = Math.hypot(hit[0] - 175, hit[1] - 250) < 2; + o.successToastShown = (c._toast || '').startsWith('Done: bound via 3 rooms'); + await new Promise((r) => setTimeout(r, 700)); // debounced _saveConfig + o.configWritePersisted = writes.length >= 1 + && !!writes[writes.length - 1].config.markers.find((x) => x.id === 'd_mower')?.vacuum?.calibration?.['0']; + + // ---- manual fit from scratch (HP-1540-01) ---- + cfg.markers = []; + c._regSignature = ''; c.requestUpdate(); await c.updateComplete; + const dev2 = c._devices.find((x) => x.id === 'd_mower'); + o.fitFreshNoMarker = !dev2.marker; + c._vacStartFit(dev2); await c.updateComplete; + c._vacFitSave(); await c.updateComplete; + const m2 = (cfg.markers || []).find((x) => x.id === 'd_mower'); + const calFit = m2?.vacuum?.calibration?.['0']; + o.fitMaterialises = !!m2 && Array.isArray(calFit) && calFit.length === 6; + o.fitToastAfterSave = c._toast === c._t('vac.cal_done'); + + // ---- live checkbox + trail select from scratch (HP-1540-01) ---- + cfg.markers = []; + c._regSignature = ''; c.requestUpdate(); await c.updateComplete; + c._setMode('devices'); await c.updateComplete; + c._openMarkerDialog(c._devices.find((x) => x.id === 'd_mower')); await c.updateComplete; + const liveBox = sr().querySelector('.vacbox input[type=checkbox]'); + o.vacSectionShown = !!liveBox; + if (liveBox) { liveBox.click(); await c.updateComplete; } + const m3 = (cfg.markers || []).find((x) => x.id === 'd_mower'); + o.liveToggleMaterialises = !!m3 && m3.vacuum?.live === false; + const sel = sr().querySelector('.vacbox select'); + if (sel) { sel.value = 'always'; sel.dispatchEvent(new Event('change')); await c.updateComplete; } + o.trailModePersists = m3?.vacuum?.trail_mode === 'always'; + c._markerDialog = null; c._setMode('view'); await c.updateComplete; + + // ---- error copy never points at the removed point calibration (HP-1540-06) ---- + const toasts = ['vac.autocal_no_rooms', 'vac.autocal_no_match', 'vac.autocal_res_warn'] + .map((k) => c._t(k)).join(' | '); + o.noPointCalibrationCopy = !/point|точк/i.test(toasts); + // and the real no-rooms path shows the reworded toast + c.hass = { ...c.hass, states: { ...c.hass.states, + 'camera.mower_map': { state: 'idle', attributes: { vacuum_position: { x: 1, y: 2, a: 0 }, map_index: 0 } } } }; + await c.updateComplete; + c._vacAutoCalibrate(c._devices.find((x) => x.id === 'd_mower')); + o.noRoomsToastReworded = c._toast === c._t('vac.autocal_no_rooms') && !/point|точк/i.test(c._toast); + + c.hass.callWS = realWS; + return o; +}); + +checkAll(out); +await finish(browser, out); diff --git a/docs/VACUUM.md b/docs/VACUUM.md index adedd4c..375b77d 100644 --- a/docs/VACUUM.md +++ b/docs/VACUUM.md @@ -91,12 +91,16 @@ marker is edited as usual. ## Setup UX A «Живая позиция» section in the device dialog, vacuum markers only: -status line (which source was found / room-only / nothing), one button -(«Настроить автоматически» for Tier A, «Калибровка по трём точкам» for -Tier B), three checkboxes (live position / trail / room highlight, all -on), and for multi-map robots a per-map calibration status list. The -source entity is discovered via the device registry — no YAML, no -entity pickers. +status line (which source was found / nothing), the «Настроить +автоматически» button (Tier A — the integration reports a room list), +the «Подогнать вручную» button opening the fit panel (drag the ghost +map, stretch by the corners, rotate/mirror), one «Живая позиция на +плане» checkbox (on by default), the «Показывать путь робота» select +(never / while cleaning / always), and for multi-map robots a list of +calibrated maps. The source entity is discovered via the device +registry — no YAML, no entity pickers. The section works before the +marker is ever saved: the first vacuum edit materialises the marker +itself (HP-1540-01). ## Storage & validation diff --git a/src/houseplan-card.ts b/src/houseplan-card.ts index 09b71d8..14faae1 100755 --- a/src/houseplan-card.ts +++ b/src/houseplan-card.ts @@ -42,7 +42,7 @@ import { cardStyles } from './styles'; import { fitInSquare, contentBounds, spaceModels } from './space-geometry'; import { langOf, t, type I18nKey } from './i18n'; -const CARD_VERSION = '1.54.0'; +const CARD_VERSION = '1.54.1'; const LS_KEY = 'houseplan_card_layout_v1'; const LS_CFG = 'houseplan_card_cfg_v1'; // cache of the server config+layout for instant rendering const LS_ZOOM = 'houseplan_card_zoom_v1'; @@ -4423,6 +4423,35 @@ class HouseplanCard extends LitElement { + /** + * HP-1540-01: an auto-discovered vacuum has NO config marker until the first + * general Save of its dialog, yet the «Живая позиция» section is already + * interactive. Every vacuum handler used to `cfg.markers.find(...)` and + * silently no-op without one — auto-calibration even toasted success while + * saving nothing. Materialise a minimal marker (same id/binding the dialog + * Save would produce — see _saveMarker/markerIdForBinding) before any + * vacuum write, so the write has somewhere real to land. + */ + private _vacEnsureMarker(d: DevItem): Marker | null { + const cfg = this._serverCfg; + if (!cfg) return null; + cfg.markers = cfg.markers || []; + const existing = cfg.markers.find((x: Marker) => x.id === d.id); + if (existing) return existing; + if ((d.bindingKind !== 'device' && d.bindingKind !== 'entity') || !d.bindingRef) return null; + const m: Marker = { + id: d.id, + binding: d.bindingKind + ':' + d.bindingRef, + space: d.space || null, + area: d.area || null, + // explicit like _saveMarker: a marker of any kind tells the seeder + // "the user decided" (docs/FILTERING.md) + hidden: d.hidden ? true : false, + }; + cfg.markers.push(m); + return m; + } + /** «Живая позиция» in the device dialog — vacuum markers only. */ private _renderVacSection(dlg: any): TemplateResult | typeof nothing { const dev = this._devices.find((x) => x.id === dlg.devId); @@ -4436,8 +4465,9 @@ class HouseplanCard extends LitElement { : this._t('vac.status_none'); const cals = Object.keys(v.calibration || {}); const setVac = (patch: Record) => { - const cfg = this._serverCfg; - const m = cfg?.markers?.find((x: Marker) => x.id === dev.id); + // HP-1540-01: materialise the marker first — find() alone silently + // dropped every edit for a vacuum that had never been saved + const m = this._vacEnsureMarker(dev); if (!m) return; m.vacuum = { ...(m.vacuum || {}), ...patch }; this._regSignature = ''; @@ -4481,11 +4511,15 @@ class HouseplanCard extends LitElement { return sel ? String(sel) : 'default'; } - /** Persist a solved matrix into marker.vacuum.calibration[mapId]. */ - private _vacSaveMatrix(markerId: string, source: string, mapId: string, matrix: Affine): void { - const cfg = this._serverCfg; - const m = cfg?.markers?.find((x: Marker) => x.id === markerId); - if (!m) return; + /** Persist a solved matrix into marker.vacuum.calibration[mapId]. + * Returns whether the write actually landed — callers must not toast + * success otherwise (HP-1540-01). */ + private _vacSaveMatrix(markerId: string, source: string, mapId: string, matrix: Affine): boolean { + // HP-1540-01: a first-use vacuum has no marker yet — materialise it + const dev = this._devices.find((x) => x.id === markerId); + const m = dev ? this._vacEnsureMarker(dev) + : this._serverCfg?.markers?.find((x: Marker) => x.id === markerId); + if (!m) return false; const v = { ...(m.vacuum || {}) }; v.source = source; v.calibration = { ...(v.calibration || {}), [mapId]: matrix.map((n) => Number(n.toFixed(6))) }; @@ -4493,6 +4527,7 @@ class HouseplanCard extends LitElement { this._regSignature = ''; this._saveConfig(); this.requestUpdate(); + return true; } /** «Настроить автоматически»: robot rooms ↔ plan rooms by name. */ @@ -4505,9 +4540,14 @@ class HouseplanCard extends LitElement { } const sp = this._spaceModel(d.space); const planRooms = (sp?.rooms || []) - .filter((r: any) => r.name && r.poly?.length >= 3) - .map((r: any) => { - const c = poleOfInaccessibility(r.poly); + // HP-1540-04: legacy rectangle rooms (x/y/w/h) are still first-class + // everywhere else — roomPoly() gives the same 4-corner outline the + // renderer uses, so they must count for name-matching too. The old + // r.poly?.length filter dropped them and then blamed the room names. + .map((r: any) => ({ r, poly: roomPoly(r) })) + .filter(({ r, poly }: any) => r.name && poly) + .map(({ r, poly }: any) => { + const c = poleOfInaccessibility(poly); return { name: r.name, cx: c[0], cy: c[1] }; }); const res = autoCalibrate(tele.rooms, planRooms); @@ -4516,10 +4556,12 @@ class HouseplanCard extends LitElement { return; } // residual gate: 5% of the canvas ≈ 40 cm in a typical house + // HP-1540-01: toast success ONLY after the matrix verifiably landed in + // the config — the old code always claimed victory, even as a no-op + if (!this._vacSaveMatrix(d.id, src, this._vacMapId(d, tele), res.matrix)) return; if (res.residual > NORM_W * 0.05) { this._showToast(subst(this._t('vac.autocal_res_warn'), { rooms: String(res.matched.length) })); } - this._vacSaveMatrix(d.id, src, this._vacMapId(d, tele), res.matrix); this._showToast(subst(this._t('vac.autocal_done'), { rooms: String(res.matched.length) })); } @@ -4545,9 +4587,10 @@ class HouseplanCard extends LitElement { private _vacFitSave(): void { const f = this._vacFit; if (!f) return; - this._vacSaveMatrix(f.markerId, f.source, f.mapId, fitMatrix(f.p)); + // HP-1540-01: no success toast for a save that did not happen + const ok = this._vacSaveMatrix(f.markerId, f.source, f.mapId, fitMatrix(f.p)); this._vacFit = null; - this._showToast(this._t('vac.cal_done')); + if (ok) this._showToast(this._t('vac.cal_done')); } /** Rotate/mirror around the ghost centre so the map does not fly away. */ diff --git a/src/i18n/en.json b/src/i18n/en.json index 0ac0786..1c5d96d 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -363,9 +363,9 @@ "vac.live": "Live position on the plan", "vac.trail": "Show the robot's path", "vac.cal_maps": "Calibrated maps: {maps}", - "vac.autocal_no_rooms": "The integration reports no room list — use point calibration", - "vac.autocal_no_match": "Room names did not match (need ≥3 in common) — use point calibration", - "vac.autocal_res_warn": "Matched {rooms} rooms but the fit is rough — verify and refine with points if needed", + "vac.autocal_no_rooms": "The integration reports no room list — open “Fit manually”", + "vac.autocal_no_match": "Room names did not match (need ≥3 in common) — open “Fit manually”", + "vac.autocal_res_warn": "Matched {rooms} rooms but the fit is rough — verify and refine via “Fit manually” if needed", "vac.autocal_done": "Done: bound via {rooms} rooms. Start a cleanup and check", "vac.cal_need_pos": "The robot is not reporting coordinates — start a cleanup and pause it", "vac.cal_done": "Calibration saved. Start a cleanup and check", diff --git a/src/i18n/ru.json b/src/i18n/ru.json index 5fc0264..8c1161b 100644 --- a/src/i18n/ru.json +++ b/src/i18n/ru.json @@ -363,9 +363,9 @@ "vac.live": "Живая позиция на плане", "vac.trail": "Показывать путь робота", "vac.cal_maps": "Откалиброваны карты: {maps}", - "vac.autocal_no_rooms": "Интеграция не отдаёт список комнат — используйте калибровку по точкам", - "vac.autocal_no_match": "Не совпали имена комнат (нужно ≥3 общих) — используйте калибровку по точкам", - "vac.autocal_res_warn": "Совпало комнат: {rooms}, но привязка грубовата — проверьте и при необходимости откалибруйте по точкам", + "vac.autocal_no_rooms": "Интеграция не отдаёт список комнат — откройте «Подогнать вручную»", + "vac.autocal_no_match": "Не совпали имена комнат (нужно ≥3 общих) — откройте «Подогнать вручную»", + "vac.autocal_res_warn": "Совпало комнат: {rooms}, но привязка грубовата — проверьте и при необходимости откройте «Подогнать вручную»", "vac.autocal_done": "Готово: привязка по {rooms} комнатам. Запустите уборку и проверьте", "vac.cal_need_pos": "Робот сейчас не отдаёт координаты — запустите уборку и поставьте на паузу", "vac.cal_done": "Калибровка сохранена. Запустите уборку и проверьте", diff --git a/src/vacuum.ts b/src/vacuum.ts index 8cba9b2..2142e58 100755 --- a/src/vacuum.ts +++ b/src/vacuum.ts @@ -81,6 +81,19 @@ const num = (v: unknown): number | null => { return Number.isFinite(n) ? n : null; }; +/** + * Map-id normalisation contract, shared with the backend recorder + * (custom_components/houseplan/trails.py: resolve_map_id). The FIRST value + * that is not null/undefined wins — truthiness is wrong here, because a + * zero-based `map_index: 0` is a perfectly valid first map and an empty + * string is still an id. The backend used an `or`-chain and dropped the + * zero, so server trails were stored under a key the renderer never looked + * up (HP-1540-02). + */ +export function vacMapIdFromAttrs(attrs: Record): string { + return String(attrs.map_name ?? attrs.current_map ?? attrs.map_index ?? attrs.selected_map ?? 'default'); +} + /** * Normalise the attribute zoo. One parser instead of per-brand classes: the * three Tier-A integrations (Xiaomi Cloud Map Extractor, Tasshack @@ -134,7 +147,7 @@ export function readVacTelemetry(attrs: Record | null | undefined): rooms.push(room); } } - const mapId = String(attrs.map_name ?? attrs.current_map ?? attrs.map_index ?? attrs.selected_map ?? 'default'); + const mapId = vacMapIdFromAttrs(attrs); if (!pos && !rooms.length && !path) return null; return { pos, path, rooms, mapId }; } diff --git a/test/i18n.test.mjs b/test/i18n.test.mjs index af7795c..b418854 100644 --- a/test/i18n.test.mjs +++ b/test/i18n.test.mjs @@ -25,3 +25,11 @@ test('i18n: placeholders match between languages', () => { assert.equal(ph(en[k]), ph(ru[k]), `placeholder mismatch in ${k}`); } }); + +test('vac toasts never mention the removed point calibration (HP-1540-06)', () => { + for (const [lang, d] of [['en', en], ['ru', ru]]) { + for (const k of ['vac.autocal_no_rooms', 'vac.autocal_no_match', 'vac.autocal_res_warn']) { + assert.ok(!/point|точк/i.test(d[k]), `${lang}:${k} still points at point calibration`); + } + } +}); diff --git a/test/vacuum.test.mjs b/test/vacuum.test.mjs index 354ae2c..829d3e6 100644 --- a/test/vacuum.test.mjs +++ b/test/vacuum.test.mjs @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { solveAffine, applyAffine, affineResidual, readVacTelemetry, autoCalibrate, thinPath, pushTrailPoint, isVacMoving, isVacSourceState } from '../test-build/vacuum.js'; +import { solveAffine, applyAffine, affineResidual, readVacTelemetry, autoCalibrate, thinPath, pushTrailPoint, isVacMoving, isVacSourceState, vacMapIdFromAttrs } from '../test-build/vacuum.js'; test('solveAffine recovers rotation+scale+mirror+offset exactly', () => { // target = mirror-X, rotate 90°, scale 0.02, offset (300, 400) @@ -135,3 +135,20 @@ test('fitMatrix/fitFromMatrix round-trip, mirror and quarters', async () => { const c2 = applyAffine(fitMatrix(r2), 500, 500); assert.ok(Math.abs(c2[0] - c[0]) < 1e-6 && Math.abs(c2[1] - c[1]) < 1e-6); }); + +// HP-1540-02: the map-id contract, mirrored by trails.py resolve_map_id — +// the first value that is not null/undefined wins; zero and '' are valid ids. +test('vacMapIdFromAttrs: first NOT-nullish value wins, zero survives', () => { + assert.equal(vacMapIdFromAttrs({ map_index: 0 }), '0'); + assert.equal(vacMapIdFromAttrs({ map_index: '0' }), '0'); + assert.equal(vacMapIdFromAttrs({ map_name: '', selected_map: 'Floor' }), ''); + assert.equal(vacMapIdFromAttrs({ map_name: 'A', map_index: 0 }), 'A'); + assert.equal(vacMapIdFromAttrs({ current_map: 2 }), '2'); + assert.equal(vacMapIdFromAttrs({ selected_map: 'Vac' }), 'Vac'); + assert.equal(vacMapIdFromAttrs({}), 'default'); +}); + +test('readVacTelemetry keeps numeric zero map_index as map id (HP-1540-02)', () => { + const t = readVacTelemetry({ vacuum_position: { x: 1, y: 2 }, map_index: 0 }); + assert.equal(t.mapId, '0'); +});