New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add neato dismiss alert button #97572
Add neato dismiss alert button #97572
Conversation
Hey there @dshokouhi, @Santobert, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Looks like this should be a button? |
Being a service you can use it however you want. Pushing a button or in an automation. I think it's more versatile |
You can push a button in an automation. |
Yeah, I know but that would be overhead and imho not conceptually correct. It's a service which exposes a functionality |
Can't understand why it would be conceptually wrong? As probably there is a button or something you can click in the app to do this. |
There's a button in the app to start the cleaning too. Nonetheless the custom cleaning is a service and not a button. I think it's better if it's a service, even for coherence. I think it's best not to think UI first, and then interact with UI entities in code, but to think the other way around and expose something in the UI only when/if needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh man this would be great.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
I just read the discussion and I share the opinion of @gjohansson-ST. The other service you mentioned takes arguments, so it requires more information to execute that action. In this case, it's something you can fire without extra information. For automations, making it a service or making it a button makes no difference as both can be called and both would work. But the benefit for the button is in the usability for people who don't use it in an automation. If the button shares the same device info as the vacuum, the button will show up at the device page, allowing users to press it directly from the UI without having to create an integration or to run developer tools to call the service directly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a button
and not a custom service.
Please change this as already earlier said by myself and another developer.
Oh and please include a changelog for the dependency you are updating in the PR description |
I linked the release in the PR description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny remarks left and then we're good
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
Co-authored-by: G Johansson <goran.johansson@shiftit.se>
Co-authored-by: G Johansson <goran.johansson@shiftit.se>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Breaking change
Proposed change
Adding a Dismiss Alert button to
neato
integration to be able to dismiss alerts preventing cleaning from startType 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: