zoned_cleanup custom command not working

tlpbu commented 3 years ago

Describe the bug

zoned_cleanup custom command from Home Assitant not processed or not working.

From Valetudo UI selected zone cleaning is working fine but cannot be implemented in Home Assistant automation as described in https://valetudo.cloud/pages/integrations/home-assistant-integration.html 'Example scripts.yaml snippet in Home Assistant for zoned cleaning' section.

In Home Assistant vacuum is configured as follows:

  - platform: mqtt
    name: somename
    schema: state
    supported_features:
      - start
      - pause
      - stop
      - return_home
      - battery
      - status
      - locate
      - clean_spot
      - fan_speed
      - send_command
    command_topic: "vacuum/somename/command"
    state_topic: "vacuum/somename/state"
    send_command_topic: "vacuum/somename/custom_command"
    set_fan_speed_topic: "vacuum/somename/set_fan_speed"
    fan_speed_list:
      - min
      - medium
      - high
      - max

MQTT discovery is intentionally disabled to prevent uncontrolled entities appearing.

When MQTT message in topic vacuum/somename/custom_command is published by Home Assistant with body {"command": "zoned_cleanup", "zone_ids": ["Hallway"]} nothing happens.

Vacuum Model

Gen1

Valetudo Version

2021.01.1

Expected behavior

Zoned cleanup starts upon MQTT message publishing

Hypfer commented 3 years ago

That is not the Valetudo version 🤨

You're probably running 2021.01.1 and discovered that the documentation needs updating. The MQTT command takes the zone_id and not the zone name now

tlpbu commented 3 years ago

Yes, Valetudo version is 2021.01.1 - I updated issue description.

Indeed it finally after all the struggles started to work with id of the zone from vacuum/somename/ZoneCleaningCapability/presets MQTT topic.

Apart from documentation update I would propose showing Zone id somewhere in Valetudo UI, to make it easier to find for those creating HA automations.

Hypfer commented 3 years ago

Yep definitely. That would be something to tackle with the UI rewrite

kquinsland commented 3 years ago

Apart from documentation update I would propose showing Zone id somewhere in Valetudo UI, to make it easier to find for those creating HA automations.

Is it too much to ask for the MQTT endpoint to implement a "string to id" lookup? The server is the only place that has both the ID and the human-friendly name for thee zone...

Looking at the config I see that the zones are keyed via the UID and looking at the MQTT msg handler I think it would be easy to add a simple bit of logic to build the list of UIDs from human friendly names:

zone_ids = msg["zone_ids"];

if 'zone_names' in msg:
    for zone_id, zone_obj in this.config.get("zonePresets"):
        if zone_obj['name'] in msg['zone_names']:
            zone_ids.add(zone_id)

// proceede to visit each zone..

I think that would be easier than building functionality into the UI to discover the zoneID from name (or, putting together documentation about how to get the ID from the config file on the robot or via MQTT messages) and would make the transition from old docs / setup / scripts to new much smoother. No lookups needed, just change every instance of zone_ids to zone_names in the user's existing HA scripts/config and done!

I haven't written any JS since jQuery mobile was cutting edge, but I think it should be possible to write that simple lookup loop just before here:

https://github.com/Hypfer/Valetudo/blob/76a9c5234dfbcce864d70d1d6db781af86d6a98b/lib/mqtt/MqttClient.js#L466

Hypfer commented 3 years ago

@kquinsland The move away from calling presets by name was intentional.

The server is the only place that has both the ID and the human-friendly name for thee zone

No!

They're broadcasted via MQTT and you also get the ID if you query for all existing presets via REST.

Hypfer commented 3 years ago

5492e4da

Hypfer / Valetudo