Skip to content

Expose cleaning scenes as segments when native room support is unavailable - #151

Open
ModernTTY wants to merge 7 commits into
jeppesens:mainfrom
ModernTTY:feature/scene-segments
Open

ModernTTY wants to merge 7 commits into
jeppesens:mainfrom
ModernTTY:feature/scene-segments

Conversation

@ModernTTY

@ModernTTY ModernTTY commented Jul 4, 2026

Copy link
Copy Markdown

Summary

This PR adds a fallback that exposes cleaning scenes as Home Assistant segments for devices that do not provide native room/segment information.

When native rooms are available, the existing behavior is unchanged. If no rooms are exposed but the device provides cleaning scenes, those scenes are exposed as segments instead. This enables Home Assistant’s native room mapping and segment cleaning UI on models such as the Eufy Omni C20.

Changes

  • Fall back to scenes when no native room/segment data is available.
  • Expose scene-based segments for Home Assistant room mapping.
  • Route app_segment_clean and async_clean_segments to scene_clean when using scene-based segments.
  • When multiple scene-based segments are selected, clean the first scene and log a warning (matching current device capabilities).
  • Add regression tests covering the new fallback behavior.

Compatibility

  • Devices with native room support continue to use native rooms.
  • The scene fallback is only used when no native rooms are available.

Related Issues

#130

Attribution

The implementation in this PR is based on work by @acdu1. I cherry-picked the two scene-support commits from their fork so this feature can be reviewed independently of the other unrelated changes in that branch. All implementation credit goes to them.

acdu1 added 2 commits July 4, 2026 10:29
… cleaning

Fallback to scenes when no native rooms exist in RoboVacMQTTEntity._get_room_segments
Expose scene-based segments through HA segment APIs for room mapping
Route app_segment_clean and async_clean_segments to scene_clean when using scenes
Add regression tests covering scene fallback and app segment behavior
When no native rooms exist and scenes are exposed as segments, use only the first selected scene
Prevent random/no-op behavior for multi-selection
Add regression tests for app_segment_clean and async_clean_segments

@m11tch m11tch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution, it seems like a useful addition, but I do have some minor remarks.

Comment thread custom_components/robovac_mqtt/vacuum.py Outdated
Comment thread custom_components/robovac_mqtt/vacuum.py Outdated
@ModernTTY
ModernTTY requested a review from m11tch July 5, 2026 16:07
@m11tch

m11tch commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Thanks, the production code now routes scene cleaning through self.coordinator.build_device_command(...), which addresses my previous protocol-routing concern.

There are two remaining items:

  1. The updated tests still patch/assert custom_components.robovac_mqtt.vacuum.build_command, but the implementation now correctly calls self.coordinator.build_device_command(...). This currently makes pytest tests/test_vacuum.py -q fail with 4 failures. The tests should be updated to assert mock_coordinator.build_device_command instead.

  2. The multiple-selection behavior is still concerning: clean_segments(["1", "2"]) only cleans the first scene-backed segment and logs a warning. Since HA segment cleaning semantically accepts multiple selected segments, I’d prefer either raising HomeAssistantError for multiple scene-backed IDs or not exposing scene-backed segments as multi-select-capable.

@ModernTTY

ModernTTY commented Jul 22, 2026

Copy link
Copy Markdown
Author

@m11tch Done in latest commit.

@acdu1

acdu1 commented Jul 22, 2026

Copy link
Copy Markdown

@ModernTTY Thanks for helping me!

@m11tch

m11tch commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Thank you for your contribution, Please fix the linting issues, after that I think we're good to merge :)

@ModernTTY

Copy link
Copy Markdown
Author

@m11tch Done in latest commit.

@m11tch

m11tch commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

@m11tch Done in latest commit.

checks are still failing

@ModernTTY

Copy link
Copy Markdown
Author

@m11tch Lint should pass now

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.

3 participants