Skip to content

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

Open
jgus wants to merge 1 commit into
sjmotew:masterfrom
jgus:fix/room-cleaning
Open

fix(clean): room cleaning via clean/start_clean with a parameterized CleanParam (#25, #37)#49
jgus wants to merge 1 commit into
sjmotew:masterfrom
jgus:fix/room-cleaning

Conversation

@jgus

@jgus jgus commented Jun 14, 2026

Copy link
Copy Markdown

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_cleanCleanTask{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-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

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 jgus force-pushed the fix/room-cleaning branch from 573be18 to 4e747c5 Compare June 14, 2026 21:23
@jgus

jgus commented Jun 14, 2026

Copy link
Copy Markdown
Author

Thanks — addressed all three points:

  1. Naming. Renamed the misleading TOPIC_CMD_START_CLEAN_LEGACYTOPIC_CMD_CLEAN_TASK (and TOPIC_CMD_START_CLEANTOPIC_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.

…CleanParam (sjmotew#25, sjmotew#37)

Room cleans were sent to clean/plan/start, but on Flow firmware that is
StartWithPlan{planId, mapId} — it starts a saved plan by id and ignores any room
payload, so the robot undocked and wandered instead of cleaning the selection.

Switch start_rooms() to clean/start_clean (StartClean → CleanTask). 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
(CommandResult.NOT_READY) — retry briefly while docked.

Build the CleanParam from named parameters: start_rooms() takes
work_mode/fan/water/mop_strength/passes with defaults at the call site
(vacuum-and-mop, standard suction/water/mop, single pass). Names and enums match the
app's CleanTask proto: WorkMode (= robot_work_mode_*, whose value is CleanTask.taskType),
corrected FanLevel/MopHumidity, added MopStrengthLevel; fan_speed labels use the app's
user-visible suction names (quiet/standard/strong/super powerful/ultra powerful).
Pre-rename fan_speed values (normal/max) remain accepted for back-compat.

Validated live on a Flow 2: room clean returns SUCCESS and the robot cleans the
selected rooms (confirmed via clean/current_clean_task/get). Both client copies synced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jgus jgus force-pushed the fix/room-cleaning branch from 4e747c5 to 1b5f02b Compare June 15, 2026 00:10
jgus added a commit to jgus/NarwalIntegration that referenced this pull request Jun 15, 2026
…water / mop strength / passes)

Adds Home Assistant controls for the room-clean parameters, backed by a CleanSettings
dataclass on the coordinator (the single source the clean-start path reads):

- select entities: work mode (Vacuum / Mop / Vacuum then mop / Vacuum and mop),
  mopping humidity (Slightly dry / Normal / Slightly wet), mop strength (Normal / High);
- number entity: cleaning passes (1-3);
- the vacuum's fan_speed is threaded through the same settings.

Entity labels use the app's user-visible wording. Values persist across restarts via
RestoreEntity (RestoreSelect / RestoreNumber / RestoreEntity) — set once and kept.
async_clean_segments threads them into start_rooms; water and fan also apply live while
cleaning.

Built on sjmotew#49 (parameterized start_rooms / WorkMode).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

start_rooms() crashes Flow 2 — command payload likely incompatible

1 participant