fix(clean): room cleaning via clean/start_clean with a parameterized CleanParam (#25, #37)

[jgus]

Summary

Room cleaning was broken on Flow firmware — selecting rooms sent the robot off the
dock to wander instead of cleaning the selection. This switches to the correct
command and builds the clean parameters from named, reverse-engineered settings.

Fixes #25, #37.

Root cause

start_rooms() sent clean/plan/start, which on Flow firmware is
StartWithPlan{planId, mapId} — it starts a saved plan by id and ignores any room
payload. The robot ran its last plan instead of cleaning the selected rooms.

Fix

• Switch to clean/start_clean → CleanTask{map_id, [CleanItem{ZoneOption, CleanParam, order}], taskType}. Track the active map id (MapData.map_id, get_map field 2.1),
  which the CleanTask requires.
• clean/start_clean only works docked; from STANDBY the robot returns a new code 4
  (NOT_READY) — retry briefly while docked.

Parameterized CleanParam

The CleanParam is built from named parameters with defaults at the call site:

start_rooms(rooms, *, work_mode=VACUUM_AND_MOP, fan=NORMAL,
            water=NORMAL, mop_strength=NORMAL, passes=1)

Names and enums match the app's CleanTask proto:

• WorkMode (the app's robot_work_mode_* selector): VACUUM / MOP / VACUUM_THEN_MOP / VACUUM_AND_MOP; its value is the CleanTask.taskType the robot executes.
• FanLevel / MopHumidity corrected to the robot's real enum values;
  MopStrengthLevel added.
• fan_speed labels use the app's user-visible suction names:
  quiet / standard / strong / super powerful / ultra powerful.

Enum class names and CleanParam field names are verbatim from the proto; the integer
values are live-validated on a Flow 2.

Changes

• narwal_client/ (+ embedded custom_components/narwal/narwal_client/ copy): rewrite
  start_rooms / _build_start_clean_payload; add WorkMode + MopStrengthLevel,
  correct FanLevel/MopHumidity; add MapData.map_id and track it in get_map.
• custom_components/narwal/const.py: FAN_SPEED_MAP → the app's user-visible suction labels.
  Back-compat: set_fan_speed still accepts the pre-rename values (normal → standard,
  max → top); they are not offered in the fan-speed list.
• Topic constants renamed to match their commands: TOPIC_CMD_PLAN_START (clean/plan/start)
  and TOPIC_CMD_CLEAN_TASK (clean/start_clean, was misleadingly …_LEGACY).
• tests/test_client_rooms.py: cover the CleanTask builder (taskType/mode dispatch,
  fan/water/strength encoding, settings forwarding) and start_rooms dispatch
  (whole-house fallback, missing map id, NOT_READY retry, CONFLICT passthrough).

Testing

• pytest tests/ — all green (167).
• Validated live on a Flow 2: room clean returns SUCCESS and the robot cleans the
  selected rooms (confirmed via clean/current_clean_task/get).

[greptile-apps]

Greptile Summary

This PR fixes room cleaning on Flow firmware by switching start_rooms from clean/plan/start (which ignored any room payload and ran the last saved plan) to clean/start_clean with a fully parameterized CleanTask. It also corrects FanLevel/MopHumidity enum values to match the robot's proto, adds MopStrengthLevel/WorkMode, tracks the active map_id required by CleanTask, and handles several new-firmware working-status values (DOCKED_V2, TASK_COMPLETED).

• start_rooms rewrite: builds a CleanTask protobuf with ZoneOption/CleanParam per room, adds a docked-retry loop for NOT_READY, and guards against missing map_id.
• Enum corrections: FanLevel, MopHumidity values corrected to robot's actual proto integers; MopStrengthLevel and WorkMode added; FAN_SPEED_MAP updated with backward-compat lowercase aliases so existing automations continue to work.
• Firmware compat: DOCKED_V2(2) and TASK_COMPLETED(19) statuses handled in is_docked, activity, and coordinator transition logic; field3 list-handling added for newer bbpb decodes.

Confidence Score: 5/5

Safe to merge — the root cause fix (wrong MQTT topic + incorrect proto schema) is well-understood, live-validated on a Flow 2, and covered by a thorough new test suite (167 tests, all green).

The core change is a clean protocol-level fix: wrong topic → right topic, wrong payload schema → correctly reverse-engineered CleanTask. All new dispatch paths (map_id guard, NOT_READY retry, CONFLICT passthrough, whole-house fallback) have explicit tests. Enum value corrections were validated against a live device. The two observations are both cosmetic or low-impact behavior changes that do not affect cleaning correctness.

No files require special attention. custom_components/narwal/const.py is worth a second glance if you want the new title-case fan-speed names reflected in _last_fan_speed for UI consistency, but the functional path is correct.

Important Files Changed

Filename	Overview
narwal_client/client.py	Core fix: replaces _build_room_clean_payload + clean/plan/start with _build_start_clean_payload + clean/start_clean; adds parameterized CleanParam dispatch per WorkMode, map_id guard, and docked-retry loop; well-covered by new tests.
narwal_client/const.py	Enum corrections (FanLevel, MopHumidity shift to 1-based), new MopStrengthLevel/WorkMode/NOT_READY/DOCKED_V2/TASK_COMPLETED; topic constants renamed to TOPIC_CMD_PLAN_START / TOPIC_CMD_CLEAN_TASK (resolves prior "LEGACY" naming concern).
narwal_client/models.py	Adds map_id to MapData (from field 1 of the get_map payload); hardens field3 parsing for list returns, unknown values, and new firmware sub-fields; updates is_docked for DOCKED_V2, dock_field11 >= 2, and dock_field47 in {1, 3}.
custom_components/narwal/const.py	FAN_SPEED_LIST renamed to title-case strings; backward-compat aliases ("quiet", "normal", "strong", "max") retained in FAN_SPEED_MAP so automations continue to work, but stored _last_fan_speed from old sessions won't match the new list entries.
tests/test_client_rooms.py	Comprehensive rewrite: covers taskType/CleanParam dispatch per WorkMode, fan/water/strength encoding, map_id guard, NOT_READY retry (docked vs. off-dock), and CONFLICT passthrough — all scenarios mentioned in the PR description now tested.
custom_components/narwal/vacuum.py	Adds DOCKED_V2 → DOCKED and TASK_COMPLETED → RETURNING to the activity map; fallback path for unmapped statuses logs a warning and returns CLEANING instead of IDLE while off-dock.
custom_components/narwal/coordinator.py	Extends the CLEANING→dock transition poll to also fire on DOCKED_V2 (new firmware dock state), ensuring prompt UI refresh after a clean on v01.07.23+ firmware.

Sequence Diagram

sequenceDiagram
    participant HA as HA vacuum entity
    participant Client as NarwalClient
    participant Robot as Narwal Robot

    HA->>Client: start_rooms(room_ids, work_mode, fan, ...)
    alt room_ids is empty
        Client->>Robot: clean/plan/start (whole-house)
        Robot-->>Client: CommandResponse(SUCCESS)
    else room_ids given
        Client->>Client: check state.map_data.map_id
        alt "map_id == 0"
            Client->>Robot: get_map
            Robot-->>Client: MapData(map_id, rooms, ...)
        end
        alt map_id still 0
            Client-->>HA: CommandResponse(NOT_APPLICABLE)
        else map_id available
            Client->>Client: _build_start_clean_payload(room_ids, map_id, CleanParam)
            Client->>Robot: clean/start_clean (CleanTask protobuf)
            Robot-->>Client: CommandResponse
            loop up to 3x while NOT_READY and is_docked
                Client->>Client: asyncio.sleep(3s)
                Client->>Robot: clean/start_clean (retry)
                Robot-->>Client: CommandResponse
            end
            Client-->>HA: final CommandResponse
        end
    end

Loading

_{Reviews (3): Last reviewed commit: "fix(clean): room cleaning via clean/star..." | Re-trigger Greptile}

[jgus]

Thanks — addressed all three points:

1. Naming. Renamed the misleading TOPIC_CMD_START_CLEAN_LEGACY → TOPIC_CMD_CLEAN_TASK (and TOPIC_CMD_START_CLEAN → TOPIC_CMD_PLAN_START) so the constants match their topics — clean/start_clean is the correct command, not a legacy one.
2. Migration. set_fan_speed still accepts the pre-rename values (normal → standard, max → top); they're just not offered in the fan-speed list, so existing automations keep working.
3. Tests. Restored the start_rooms dispatch tests (missing map id, NOT_READY retry on/off dock, CONFLICT passthrough) — 167 passing.

[sjmotew]

@jgus — this is the correct root cause and I'm making your stack the merge base. Sequencing for all four forks currently working on this is in #66; yours is steps 1–7.

I verified the diagnosis against master before planning around it: const.py:85 sends clean/plan/start from start_rooms(), :86 mislabels clean/start_clean as legacy, and nothing parses map_id. Everything I shipped for #37 (516ed1c, 3f7c17b, #41, staged #38) was fixing a payload on a topic that discards it. #38 is closed.

Two things before I merge this one:

1. Does _build_start_clean_payload emit ZoneOption field 4 (coverage path) and a non-empty TaskOption? @sytchi reports NOT_APPLICABLE without both on Flow 1 v01.08.03.07 — details in Room clean ignores HA selection, reverts to Narwal-app shortcut (newer firmware) #37. If this PR omits them, it may pass on your Flow 2 and still fail for @saeft2003 on v01.08.00.07, which is the firmware line that has been stuck on this issue longest. Not asking you to implement it — @sytchi is opening a PR on top of yours — just want to know whether it's already covered.

2. I'll take fix(rooms): use the app's own RoomType→name strings; drop per-model overrides (#22) #48 first so the room-name table is correct before this reads room names.

Worth noting separately: #48 shows my own #22 fix (5b4dac7, per-model override map) was papering over a mis-derived base table. Good catch, and thank you for chasing it to the i18n source rather than stopping at the symptom.

[sjmotew]

Answering my own question from above — I pulled this branch locally and read _build_start_clean_payload rather than making @jgus round-trip on it.

#49 emits neither of the two fields @sytchi flagged in #37.

# client.py:1102 — ZoneOption carries only fields 1 and 2, no field 4
items = [
    {"1": {"1": 1, "2": rid}, "2": dict(param), "3": idx + 1}
    for idx, rid in enumerate(room_ids)
]
task = {
    "1": map_id,
    "2": items if len(items) > 1 else items[0],
    "3": {},          # ← TaskOption is empty
    "5": int(work_mode),
}

and the typedef confirms it isn't a serialization artifact — "3": {"type": "message", "message_typedef": {}} is an intentionally empty message.

@sytchi's report is that on Flow 1 firmware v01.08.03.07, ZoneOption field 4 (coverage path) is required or the robot returns NOT_APPLICABLE, and TaskOption {1:1} is required because an empty {} is rejected.

Why this probably blocks merging #49 on its own

I checked my own hardware: my Flow 1 is on v01.08.03.07 — the same firmware as @sytchi. So I'd expect this branch to return NOT_APPLICABLE here, and I'll confirm that on hardware shortly.

More importantly, that's the same firmware line as the users this issue exists for — @saeft2003 is on v01.08.00.07 and @Duqino on v01.07.23.00 (#37). If the requirement is firmware-gated at 01.07/01.08 rather than model-gated, #49 would work on @jgus's Flow 2 and still not fix #37 for the people who reported it.

Two readings fit the evidence so far, and they lead to different fixes:

	Implication
Firmware-gated (01.07/01.08+ tightened validation)	Fields must be sent, conditionally or always. @sytchi's PR is a prerequisite, not a follow-on
Model-gated (Flow 2 lenient, Flow 1 strict)	Needs a per-model branch, which I'd rather avoid — see #48 for how that went last time

@jgus — no criticism intended here; your branch is live-validated and clearly correct on Flow 2. This looks like genuine firmware divergence that only shows up across hardware neither of us has alone. If you have any signal on which reading is right, I'd value it.

One useful data point either way: my get_map field 2.1 returns map_id = 1 — which is what master hardcodes. So on my hardware master's hardcoded value happens to be correct, meaning I would never have reproduced #55 locally no matter how much I tested. Worth knowing for anyone else trying to repro it.

[sjmotew]

Status: approved in principle, blocked on my hardware for several days

I want to be straight about the timeline rather than leave this sitting silently.

I can't run the validating test for several days. It undocks a live robot and I need to be physically present; realistically that's early next week. This is not review latency — the code review is done and I think this is right.

Where I've landed on the substance

I've read _build_start_clean_payload locally rather than making you round-trip answers. Two things I got wrong earlier and want on the record:

1. The topic. clean/plan/start discards its payload — it runs the app's saved plan. That assumption survived in this repo from Phase 9 until last week and cost several release cycles. clean/start_clean is the correct topic and this PR is right about it. @pspik's log in [Bug]: cleaning stops after 6 sec, removes map selection from Narwal app #55 is independent confirmation from a second user's hardware: clean/plan/start (81 bytes) returns code=1, success=True while the robot ignores the selection entirely — and notably that reproduces with no room selection at all, which rules out the room-encoding half being the cause.

2. ZoneOption field 4. I predicted on 07-27 that this PR would fail because it omits f4, which @sytchi reports as required on v01.08.03.07. I now think it will succeed. @ken99999's capture of the Narwal app's own clean command (start_rooms() crashes Flow 2 — command payload likely incompatible #25) shows the app omits f4 too. If the app doesn't send it, a firmware requiring it would break the app.

That reversal is exactly why I want the hardware measurement before merging rather than after.

The test, if anyone wants to beat me to it

My Flow 1 runs v01.08.03.07 — byte-identical to @sytchi's firmware, which is what makes this the discriminating measurement rather than just another data point. Either f4 is required on that firmware or it isn't, and this settles it.

1. Install this PR's branch (fix/room-cleaning)
2. Robot docked, one room selected — start a room clean
3. Report: does it clean that room, or the whole floor?

Plus model, firmware version, and whether the response code was SUCCESS.

@sytchi — you're the one whose report this contradicts, so your result carries the most weight. @saeft2003 (v01.08.00.07), @Duqino (v01.07.23.00), @ken99999 (Flow 2, v01.09.05.01), @pspik — same firmware line, all useful. Any single one of you running this closes it days before I can.

If it fails on v01.08.03.07 but works elsewhere, we've learned the requirement is firmware-gated and #49 needs a conditional f4 — that's a small follow-up, not a redesign, and I'd rather find out from a real robot than from more decompilation.

Thanks for your patience on this one @jgus. It's the right fix and it's been ready longer than it should have been.

[shin906710]

Tested #49 (branch fix/room-cleaning) — works. 🎉

• Model: Narwal Freo X10 Pro
• Firmware: v01.02.00.15
• Test: docked, single room selected, started via vacuum.clean_area

Two rooms tested, two different segment IDs, both cleaned the selected room only
(verified in the Narwal app):

Starting room-specific clean: rooms=[5]
Sent command: clean/start_clean (96 bytes)
Room clean response: SUCCESS (code=1), rooms=[5] -> kitchen only ✅

Starting room-specific clean: rooms=[2]
Sent command: clean/start_clean (96 bytes)
Room clean response: SUCCESS (code=1), rooms=[2] -> office only ✅

Command now correctly goes to clean/start_clean (was clean/plan/start on v1.0.0,
which ignored the selection and ran a full clean). No whole-floor fallback.

Data point on the ZoneOption f4 question: this is a different firmware line
(v01.02.00.15) from the v01.07/01.08 reporters, and omitting f4 works fine here —
so the "app omits f4, so firmware can't require it" reading holds on this line too.

Happy to run more tests if useful.

[sjmotew]

First hardware confirmation — this PR works.

@shin906710 ran the test on a Freo Z10 Pro (AX26, firmware v01.02.00.15), reported in #70:

clean/start_clean → SUCCESS (code=1), rooms=[5] (kitchen only); rooms=[2] (office only).

Two rooms, two distinct segment IDs, each cleaning only the selected room, verified in the Narwal app. Started via vacuum.clean_area from a docked state.

@jgus — your fix is no longer unproven on real hardware. Given this project already shipped one wrong room-clean fix on reasoning alone, that distinction matters.

What this does and doesn't settle

Settles: the clean/plan/start → clean/start_clean topic change is correct, and the parameterized CleanParam payload is accepted and honoured. The v01.02 line needs no conditional handling.

Doesn't settle: @sytchi's report that firmware v01.08.03.07 requires ZoneOption field 4, which this PR omits. v01.02 and v01.08 are different generations, so a positive result on one doesn't clear the other. It remains true that @ken99999's app capture shows the official app omitting f4 as well, which is strong circumstantial evidence the requirement isn't real — a firmware demanding a field the vendor's own app never sends would break the app.

Still needed: one v01.08.03.07 result

My Flow is on v01.08.03.07 and my test is still pending hardware access (early this week). Second candidate as of today: @Duqino surfaced in #37 on exactly that firmware, and I've asked them to run the same test.

If v01.08 passes, this merges as-is. If it fails, f4 becomes conditional on firmware — a small change, not a redesign.

No changes requested to this branch. Holding purely on the firmware question.

[Zebble]

Confirming on Narwal Flow (AX12), firmware v01.08.03.07 — the firmware line this PR was waiting on.

Setup: upstream master (35509cd, post-v1.0.1) with this PR cherry-picked on top, Home Assistant 2026.7.4, robot segments mapped to HA areas, started from the dock via vacuum.clean_area.

Result: room cleaning works on v01.08.03.07, and ZoneOption field 4 is not required.

Two rooms, selected in order Main Hallway → Kitchen:

It cleaned those two rooms and nothing else — no wandering onto the rest of the floor. Accepted on the first attempt: no NOT_READY, no retry, and no warnings or errors for the whole ~35 minute run.

Clean order is honoured. The robot did the Main Hallway first, then the Kitchen, matching both the selection order and the CleanItem order field. An earlier run in the same session with rooms=[2, 7] (Main Hallway → Living Room) also returned SUCCESS (code=1) and ran in the order given, so the repeated-CleanItem path is confirmed across two different room pairs.

Single-room selections have been running correctly here for several days.

The payload omits field 4, matching the app capture you cited — so on this firmware the f4-required theory doesn't hold.

[sjmotew]

Merged to master as 05af870. Thank you, @jgus — this is the most consequential fix this project has taken, and you found the root cause while I was still iterating on the payload.

Two independent hardware confirmations, on two different model and firmware lines:

Reporter	Model	Firmware	Result
@shin906710	Freo Z10 Pro (AX26)	v01.02.00.15	rooms=[5] → kitchen only, rooms=[2] → office only, verified in the app
@Zebble	Flow (AX12)	v01.08.03.07	Two rooms in the selected order, SUCCESS first attempt, ~35 min run, no errors

@Zebble — thank you. Yours is the one this PR was actually waiting on: v01.08.03.07 is the firmware line #37's reporters run, and it's the line where the ZoneOption field 4 requirement was claimed. Your run omitted f4 and worked, which together with @ken99999's app capture in #25 (the app omits f4 too) closes that question. It also confirms the repeated-CleanItem path and that clean order is honoured, which nobody had tested before.

Conflict resolution

This PR predated the 07-31/08-02 merges, so I resolved two conflicts rather than asking for a rebase:

• custom_components/narwal/const.py — took this PR's fan-speed model (the 5-tier _FAN_SPEED_CANONICAL plus the lowercase back-compat aliases) and kept ERROR_HELP_URL_TEMPLATE from feat(sensor): audit base_status fields, add station/consumable diagnostics #52.
• tests/test_client_rooms.py — took this PR's rewrite wholesale, then converted its _run() event-loop helper to @pytest.mark.asyncio coroutines so it matches the style fix(tests): resolve asyncio get_event_loop deprecation in Python 3.12 #71 established. No assertions changed.

175 tests passing, both client copies in sync, CI green on master.

One thing still open

FanLevel and MopHumidity shift by a tier here (QUIET=0..MAX=3 → MUTE=1..SUPER=5, DRY=0..WET=2 → DRY=1..WET=3), so the live set_fan_level / set_mop_humidity paths now send the app's own integers. That's the correct reading of the proto, and it means every fan speed this integration has ever sent was off by one tier.

What isn't settled is whether DEEP is a real fifth suction tier on all models — @shin906710 reports the AX26 app UI shows only four. Tracking that in #70; it doesn't block this merge, and worst case one label needs model-gating.

Ships in v1.0.2.