Skip to content
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

Roborock/Capability Do Not Disturb #656

Closed
wants to merge 3 commits into from
Closed

Roborock/Capability Do Not Disturb #656

wants to merge 3 commits into from

Conversation

bensweet86
Copy link
Contributor

Do not disturb capability for Roborock vacuums, brought back on the relevant WebUI for now given that the rest of the timers page should be replaced with the internal job logic. Do not disturb could be added to the job functionality too, main reason to bring back separately is the need to set DND on board the vacuum so it doesn't resume a clean or perform its own functions within the DND timeframe outside of the control of Valetudo. Includes

  • Capability router with get / post (multiple dnd support later on?) and delete routes
  • Delete API requires delete in body to confirm operation
  • Set DND API takes new structured json object with start / end times each with hour / minute parameters
  • State attribute for use later on with Jobs holding the status including enabled / disabled and time in similarly structured json object to what the set DND requires

@Hypfer
Copy link
Owner

Hypfer commented Jan 19, 2021

I've done some research regarding the situation of other brands (namely Viomi, Dreame, 360, Ecovacs) and the DND feature consists of exactly these parameters for all of them:

  • State (Enabled/Disabled)
  • StartTime
    • Hour
    • Minute
  • EndTime
    • Hour
    • Minute

Therefore, multiple DNDs shouldn't be something supported by this capability.
If there's ever something like that, I'd say that we'd rather add a new capability for it.

lib/entities/state/attributes/DoNotDisturbAttribute.js Outdated Show resolved Hide resolved
lib/core/capabilities/DoNotDisturbCapability.js Outdated Show resolved Hide resolved
lib/core/capabilities/DoNotDisturbCapability.js Outdated Show resolved Hide resolved
lib/core/capabilities/DoNotDisturbCapability.js Outdated Show resolved Hide resolved
@Hypfer
Copy link
Owner

Hypfer commented Jan 19, 2021

Looks good so far. 👍
I've added a few remarks where appropriate

@bensweet86
Copy link
Contributor Author

Had a similar thought process myself where to put this. The DND enabled is more a state attribute to me and parameters as you say should go elsewhere.

The state of DND is important if the Jobs functionality needs to understand whether the schedule should be overridden or not.

I was thinking the next iteration would see the DND not only relate to core OEM functionality but also extend to a Valetudo state where it understands for a schedule whether to even try and initiate and activity or not as there could be additional use cases outside of the original vacuum functionality such as:

  • Clean at night with no voice / volume at 0
  • Option to resume a clean even if this is within a DND period

Thoughts?

@Hypfer
Copy link
Owner

Hypfer commented Jan 19, 2021

At least for roborock it's (afaik) impossible to override the DND settings but even if it was possible that's far beyond the scope the scheduling in Valetudo should have.

In a perfect world, there would be no timers inside valetudo whatsoever since it would just provide the generic interface for more sophisticated home automation software to do the scheduling.
There are a few valid use-cases where having to install Home Assistant on a different host just to get a very basic schedule running would be overkill though. (e.g. the vacuum robot at your (grand-)parents house etc)
Therefore there doesn't seem to be a way around having a basic timer functionality to cater to that needs.
However everything beyond that very basic scope should be implemented elsewhere.

A while ago I've suggested that people could add their sophisticated automation configurations to the docs so that other users may simply copy-paste those into their own home automation setups.
That never happend though :(

It would be possible to either poll the DnD config via HTTP or publish it to another mqtt topic without having it as a state attribute, however I don't really see the point since there's nothing that could be done with that information due to the firmware handling it internally. You can tell it to continue but it will iirc do nothing or start a new cleanup which both isn't the intended action

@bensweet86
Copy link
Contributor Author

Don't disagree with those comments, from reading the Telegram thread I thought the intention was to add cron timer functionality within Valetudo to initiate schedules instead of relying on the vacuum from starting a schedule itself. Example would be in the Gen1's it can start a cleaning job on a schedule but not a zoned / goto / spot clean. In this instance there would be an opportunity to not only build on the scheduling logic but include DND, etc as enhancements too.

In any case will push though an updated PR to move DND away from the state attributes when I get a change later tonight.

Hypfer pushed a commit that referenced this pull request Jan 20, 2021
@Hypfer Hypfer mentioned this pull request Jan 20, 2021
@Hypfer
Copy link
Owner

Hypfer commented Jan 20, 2021

Unfortunately, I can't push my proposed changes to your branch. Therefore I've opened #659

@Hypfer Hypfer closed this Jan 20, 2021
@bensweet86 bensweet86 deleted the DoNotDisturb-Capability branch January 23, 2021 19:43
Hypfer pushed a commit that referenced this pull request Feb 8, 2021
* UI and API fix for comsumables

* Roborock/Capability Do Not Disturb

* Roborock/Capability Do Not Disturb
#656 - modifications

* Roborock/Capability Do Not Disturb

* CarpetMode-Capability

* Fixed ValetudoCarpetModeConfiguration to be serializableentity

* Roborock/Capability Carpet Mode #661

* Carpet mode capability updates

* Roborock/Capability Carpet Mode #661
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants