Support custom areas - Githubissues

bwp91 / homebridge-deebot

Homebridge plugin to integrate ECOVACS Deebot devices into HomeKit.

MIT License

64 stars 4 forks source link

Support custom areas #91

Closed apfelnutzer closed 1 year ago

apfelnutzer commented 1 year ago

@bwp91, I'm not done yet, but you can already configure custom areas along with spot areas and trigger custom area cleanings.

Still to be done:

refactoring of (old) "custom area" to "predefined area" as a more generic term throughout the code
readme/howto adjustments
cleanup of TODOs

Please have a look, if you like!

Closes bwp91/homebridge-deebot#83

apfelnutzer commented 1 year ago

@bwp91, btw - is there any way to migrate the configuration? Because I renamed the "command" to "spotAreaCommand"...

apfelnutzer commented 1 year ago

@bwp91, I will externalize the string literal for the new log entries later on, too...

bwp91 commented 1 year ago

is there any way to migrate the configuration? Because I renamed the "command" to "spotAreaCommand"...

unfortunately not :)

a name change like this would be a breaking change for all users who use this. i would prefer it not to be if possible.

bwp91 commented 1 year ago

that said, if you can code to accommodate for both naming conventions, (ie making backward compatible) then this is okay

apfelnutzer commented 1 year ago

@bwp91, I think I've found a way to handle this: old commands always have priority over new commands (spot area/custom area). I included a hint in the configuration dialog asking the user to move those entries manually to the new section (as spot areas). At some point in the future (maybe the next big release, when there are needed configuration changes anyway) one could remove this code (see last commit and appropriate comments in the code).

What do you think?

apfelnutzer commented 1 year ago

@bwp91, I'm done so far. Only thing left is to adjust the documentation. I'd suggest the following:

I'd create a copy the current Custom Area wiki page and reframe and adjust it to Predefined Areas
I'd then adjust the link from the new configuration page to that new page
I'd moreover add a hint to the old Custom Area wiki page that it's related to plugin version <= 5.1.0

Would that be fine with you?

bwp91 commented 1 year ago

Hey @apfelnutzer I would just directly edit the wiki right now to how it should be as if this PR were accepted. Once you have done this and marked this PR as ready i will give it a look over and merge it in asap

apfelnutzer commented 1 year ago

@bwp91, ok - I will do so tomorrow!

apfelnutzer commented 1 year ago

@bwp91, just stumbled upon two more things, when updating the wiki...

First, I can't change the images in the wiki as it's your user content. How can I provide you the new screenshots?

Second, I renamed the "Custom Area" characteristic to the more generic term "Predefined Area" too, as it now comprises spot and actual custom areas. Maybe that's not an issue because the UUID did not change, but I'm not sure... In the worst case users would have to adjust their scenes, which we would like to avoid. Unfortunately I can't test this without removing and adding the characteristic again, which would break the scenes anyway. Do you know if changing only the name breaks the scenes?

apfelnutzer commented 1 year ago

@bwp91: Update re: the images: I managed to upload my images!

apfelnutzer commented 1 year ago

@bwp91, wiki is updated. Please note, that the custom area link on the README does not work right now. I've update the link on my branch.

Please review & merge (don't forget to consider my second question above, but I think we're fine)!

bwp91 commented 1 year ago

For the images I cheat a bit, and go to create an issue on this repo, upload the image into the textbox grab the url and use that in the wiki!

apfelnutzer commented 1 year ago

Well I figured out, that you can just drag and drop the images!