Add SharkIQ room targeting #89350
Add SharkIQ room targeting #89350
Conversation
|
Hey there @JeffResc, @AritroSaha10, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Marking as draft, because it depends on a previous PR. |
MartinHjelmare
changed the title
|
Dependency requirements have been merged to |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
@funkybunch can you mash ready for re-review on emontnemery? I believe I have comments addressed now. @emontnemery no apparently I never dove into the link. It makes complete sense that this history does not need to be tracked. I've added that update. Exception type has been changed as well. |
|
Ayyy! Awesome! I was just thinking about how amazing this feature would be. Is there anything I can do to help? Do we just need to implement the above-suggested change, and we'll be good to go? |
|
Yeah I'll probably get to the fix this weekend. it has been through
several reviews so it should be really close
…On Mon, Mar 11, 2024 at 4:30 PM Justin Walsh ***@***.***> wrote:
Ayyy! Awesome! I was just thinking about how amazing this feature would
be. Is there anything I can do to help? Do we just need to implement the
above-suggested change, and we'll be good to go?
—
Reply to this email directly, view it on GitHub
<#89350 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADFCADJTRM6HZ4ZOKV6VJ4TYXYO6DAVCNFSM6AAAAAAVTMY5KGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBZGM4DMNRVHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@funkybunch should be ready for re-review again. I've updated from upstream/dev and addressed the comment. |
|
Thanks @jrlambs! |
| fields: | ||
| rooms: | ||
| required: true | ||
| advanced: true |
There was a problem hiding this comment.
The purpose of the advanced flag is to hide service fields from users which have not enabled the "advanced" flag in their user profile. I don't think that's useful here since the rooms service field is not optional?
There was a problem hiding this comment.
Thanks for the detailed explanation here. I think since I updated services.yaml the commit checks yelled at me because my new "clean_rooms" service did not have an icon defined. I created and icons.json file and specified an icon for it and that seems to have done the trick.
| def send_command( | ||
| self, | ||
| command: str, | ||
| params: dict[str, Any] | list[Any] | None = None, | ||
| **kwargs: Any, | ||
| ) -> None: | ||
| """Send a command to the vacuum. Not yet implemented.""" | ||
| raise NotImplementedError | ||
|
|
There was a problem hiding this comment.
Removing this makes sense, but it's not related to adding the room target service as I understand it? Please move this change to a separate PR.
|
@funkybunch, ready one more time |
|
@jrlambs if you haven’t already make sure you sign the CLA. I don’t think it prompted you because I opened the PR originally: https://www.home-assistant.io/developers/cla_sign_start/ |
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: #89346
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: