Add SharkIQ room targeting

[funkybunch]

Proposed change

This update adds room targeting support to the SharkIQ integration. Shark has a feature, that is different from spot cleaning, where you can instruct an AI-class robot vacuum to clean specific rooms that the robot knows. This allows for this room-level control in Home Assistant through a new service: sharkiq.clean_room.

NOTE: This PR will require this dependency update to merge: https://github.com/home-assistant/core/pull/89346

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] New feature (which adds functionality to an existing integration)
• [ ] Deprecation (breaking change to happen in the future)
• [ ] Breaking change (fix/feature causing existing functionality to break)
• [ ] Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue: #75676
• Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/26491

Checklist

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] The code has been formatted using Black (black --fast homeassistant tests)
• [x] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• [x] Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• [ ] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [ ] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• [ ] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

Hey there @jeffresc, @aritrosaha10, mind taking a look at this pull request as it has been labeled with an integration (sharkiq) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of sharkiq can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign sharkiq Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[epenet]

Marking as draft, because it depends on a previous PR. Please avoid opening too many PRs until the dependency bump is accepted/merged as it creates pointless CI full runs.

[funkybunch]

Dependency requirements have been merged to dev - marking as ready for review.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[funkybunch]

Tests improved. Ready for re-review.

[funkybunch]

Merge Conflicts Resolved 👍

@epenet mind taking another look when you have a moment so we can try to get this in 2023.4?

[epenet]

Merge Conflicts Resolved 👍

@epenet mind taking another look when you have a moment so we can try to get this in 2023.4?

2023.4 cut-off was on 29th March for new features, see upcoming events on https://developers.home-assistant.io/

[daviddavey1994]

Will room targeting be available @core 2023.05?

[charger68]

Is there by chance an expected timeframe for this update? We love our Shark robots and we have great plans for your new features in Home Assistant. Thank you for all of your efforts!

[gyordanov]

yes please - and it seems the PR this was waiting for is merged ?

[jrlambs]

@funkybunch @epenet , I believe I have addressed all open comments on this PR, please take a look when you get some time. @funkybunch we can probably mark this ready for review?

[funkybunch]

Thanks for your help @jrlambs !

I think we're ready for another review on this one. The beta release scheduled for 8/30 seems feasible to me for this feature.

[frenck]

Can you please not ping for reviews?

See more about that also here: https://developers.home-assistant.io/docs/review-process#prs-are-being-drafted-when-changes-are-needed

Thanks 👍

../Frenck

[jrlambs]

@funkybunch , think I have all comments addressed. Can you mark this ready for review again?

[bohregard]

Took some time to download this and test it out and it works great except for a couple UI bugs related to the room selection:

1. If you try to edit the rooms using the UI, it sets the room names to their ids (Living Room becomes living_room) which is incompatible with how Shark maps their rooms. This is problematic since the service call will fail with the error message detailing that the service cannot find the rooms. I would suggest (if at all possible) changing this to either default to the room's friendly name (instead of id) or making this a free text field that is not dependent on rooms being configured in HA. Obviously a work around here could be to rename the shark rooms to match the ids of Home assistant. This would likely solve the below issue as well.

2. If you create an Automation and add the rooms using yaml and then switch to the UI, the rooms default to a blank array []. I think this is only true if you room ids don't match what is in the sharkiq app. I haven't verified but I'll look into and update later. If you save the automation at this point, the changes are overwritten. (see below)

alias: Run Roomba
description: ""
trigger:
  - platform: time
    at: "12:00:00"
condition: []
action:
  - service: sharkiq.clean_room
    data:
      rooms: []
mode: single

EDIT: Updated the room label to match the Shark and it seems like it works good

[jrlambs]

Took some time to download this and test it out and it works great except for a couple UI bugs related to the room selection:

1. If you try to edit the rooms using the UI, it sets the room names to their ids (Living Room becomes living_room) which is incompatible with how Shark maps their rooms. This is problematic since the service call will fail with the error message detailing that the service cannot find the rooms. I would suggest (if at all possible) changing this to either default to the room's friendly name (instead of id) or making this a free text field that is not dependent on rooms being configured in HA. Obviously a work around here could be to rename the shark rooms to match the ids of Home assistant. This would likely solve the below issue as well.

2. If you create an Automation and add the rooms using yaml and then switch to the UI, the rooms default to a blank array []. I think this is only true if you room ids don't match what is in the sharkiq app. I haven't verified but I'll look into and update later. If you save the automation at this point, the changes are overwritten. (see below)

alias: Run Roomba
description: ""
trigger:
  - platform: time
    at: "12:00:00"
condition: []
action:
  - service: sharkiq.clean_room
    data:
      rooms: []
mode: single

EDIT: Updated the room label to match the Shark and it seems like it works good

@bohregard Any suggestions here? Ultimately I'd LOVE to have them be able to pick from a list of available rooms.. which the integration already knows. I didn't see that as an option though. Seems like the options are

• make it a normal string field and have people enter room names as csv (Living Room, Kitchen, Bathroom)
• make it use "location", might be good for it to auto populate one room, but ultimately you need to go into advanced mode to enter more than one room.. and it's prone to the errors you mentioned above.
• Enter in a hard-coded list of room users could choose from and if a users room doesn't match that they're out of luck.. or can switch to advanced mode.

any other suggestions would be welcome.. or if anyone thinks one of the above options is better than the other also let me know.

[bohregard]

@jrlambs It looks like there are already a list of available rooms in the attributes. There is an attribute selector but I can't seem to get it working on my end. If this selector can by dynamically driven based on the selected target, that would be the ideal scenario. I'll keep playing around with it and see what I can get.

EDIT: in retrospect this still won't work because of this: The output of this selector is the selected attribute key (not the translated or prettified name shown in the frontend). For example: next_dawn.

[gyordanov]

is there an easy way to test this ?

i tried copying the files in custom_components/sharkiq , but it does not like that at all ...

[bohregard]

is there an easy way to test this ?

i tried copying the files in custom_components/sharkiq , but it does not like that at all ...

Make sure you add a version to the manifest.json file if you haven't already. Also check the logs to see why it's not working.

[gyordanov]

Make sure you add a version to the manifest.json file if you haven't already. Also check the logs to see why it's not working.

I did that , and the logs ... well the error did not seem to help me narrow it down

Platform not found (cannot import name 'DeviceInfo' from 'homeassistant.helpers.device_registry' (/usr/src/homeassistant/homeassistant/helpers/device_registry.py

most of what i found online was related to using old HA ...and i'm on the latest version so ...unclear on what im doing wrong...

[bohregard]

Make sure you add a version to the manifest.json file if you haven't already. Also check the logs to see why it's not working.

I did that , and the logs ... well the error did not seem to help me narrow it down

Platform not found (cannot import name 'DeviceInfo' from 'homeassistant.helpers.device_registry' (/usr/src/homeassistant/homeassistant/helpers/device_registry.py

most of what i found online was related to using old HA ...and i'm on the latest version so ...unclear on what im doing wrong...

There was a PR merged not long ago that changed the from homeassistant.helpers.entity import DeviceInfo to from homeassistant.helpers.device_registry import DeviceInfo. Find that line and rename the import back to entity in vacuum.py

[gyordanov]

There was a PR merged not long ago that changed the from homeassistant.helpers.entity import DeviceInfo to from homeassistant.helpers.device_registry import DeviceInfo. Find that line and rename the import back to entity in vacuum.py

Perfect - that did it - Thank you !!

Works nicely - my main use case will be to schedule automatic per room cleanings so while the current "picker" might be confusing - this feature is already super useful in its current state!

[gyordanov]

to confirm - tested this by renaming my rooms in the shark app to be like "living_room" and it works perfectly, HA ran my automations and the vacuum did what it was told !

[jrlambs]

Ok, I did some testing. For selector there's a few options.

1. leave it the way it is, it works, user can edit the yaml manually if necessary and it may fill in at least one room for them.
2. change it to 'object' which just defaults it to yaml entry. entry would need to be formatted as a string array ['Kitchen','Livingroom'], but we can add this to the example in the documentation site so that people looking there will know how to enter.
3. Make the selector "string" and actually expect comma separated string ie: 'Kitchen, Living Room'. again we can put this example in the documentation

@bohregard , in reference to the attribute selector this sounds perfect but will only allow you to select one.. we need to allow selection of multiple.

@edenhaus @frenck Sorry for the pings if this is not allowed, curious if you have any thoughts on what you believe the best "Selector" to use here would be or if I should leave it alone. If you have no recommendations I'm tempted to leave this be and we'll just let it ride and await further reviews.

Thanks for everything an for putting up with me as I stumble through this process.

[bohregard]

@bohregard , in reference to the attribute selector this sounds perfect but will only allow you to select one.. we need to allow selection of multiple.

Dang that's a shame. I haven't spent a lot of time on this but it did seem really promising.

For me, I like options 1 or 3. I prefer 1 since I just renamed all my shark rooms to match home assistant's room ids which makes it work flawlessly. However 3 is probably more suited for rooms that may not exist in your Home Assistant configuration.

[disforw]

I did that , and the logs ... well the error did not seem to help me narrow it down Platform not found (cannot import name 'DeviceInfo' from 'homeassistant.helpers.device_registry' (/usr/src/homeassistant/homeassistant/helpers/device_registry.py most of what i found online was related to using old HA ...and i'm on the latest version so ...unclear on what im doing wrong...

There was a PR merged not long ago that changed the from homeassistant.helpers.entity import DeviceInfo to from homeassistant.helpers.device_registry import DeviceInfo. Find that line and rename the import back to entity in vacuum.py

Can you explain this further? I am adding a new integration and CLI is requesting that I change to import from device_registry, but when I test it live, I am seeing the error posted above.

[jrlambs]

@bohregard , in reference to the attribute selector this sounds perfect but will only allow you to select one.. we need to allow selection of multiple.

Dang that's a shame. I haven't spent a lot of time on this but it did seem really promising.

For me, I like options 1 or 3. I prefer 1 since I just renamed all my shark rooms to match home assistant's room ids which makes it work flawlessly. However 3 is probably more suited for rooms that may not exist in your Home Assistant configuration.

I'm going to leave it the way it is for now. I suspect that any updates to the platform to allow us to multiselect items from a list of properties would produce a string array so it makes sense to leave the input as a string array

[funkybunch]

@bohregard , in reference to the attribute selector this sounds perfect but will only allow you to select one.. we need to allow selection of multiple.

Dang that's a shame. I haven't spent a lot of time on this but it did seem really promising. For me, I like options 1 or 3. I prefer 1 since I just renamed all my shark rooms to match home assistant's room ids which makes it work flawlessly. However 3 is probably more suited for rooms that may not exist in your Home Assistant configuration.

I'm going to leave it the way it is for now. I suspect that any updates to the platform to allow us to multiselect items from a list of properties would produce a string array so it makes sense to leave the input as a string array

For what its worth, thats why I had it as the native HA room selector originally :) I have a draft documentation update (linked to this PR) that explains the nuance, and says to edit the YAML if the names are different in the app than in HA.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

[jrlambs]

Thanks for the thorough review @edenhaus . I think I have everything addressed now other than the room list. But I think that is important/necessary as it's the only way the average user has to see the way the list of rooms is defined in the shark API.