stianaske / pybotvac

Python module for interacting with Neato Botvac Connected vacuum robots.
MIT License
85 stars 44 forks source link

Use the category provided for start_cleaning #20

Closed RomRider closed 5 years ago

dshokouhi commented 6 years ago

Just an FYI but we discussed this in https://github.com/stianaske/pybotvac/pull/14 and it was discovered that if a No Go map was not created the botvac will not clean and just error out. I think we should consider an additional service to adjust cleaning and force start_cleaning to behave as if the user walked up to the robot and pressed the button to start cleaning.

stianaske commented 6 years ago

@dshokouhi what error message were you getting when starting in category 4 without a persistant map? Could a potential solution be to default to category 4, but catch the exception and fall back to category 2?

dshokouhi commented 6 years ago

@stianaske to be honest I did not look at the error message. I just noticed that the BotVac just make this beeping sound. I just checked Neato's site and it does not seem to be an error message but an alert that should get sent.

nav_floorplan_not_created

dshokouhi commented 6 years ago

@stianaske so I took a different approach about this today. @RomRider had asked me about the custom cleaning service so I had set somethings up on my own repo that I plan to merge here. What I did was I kept the start_cleaning method as is so it behaves as if a user walked up to the botvac and pressed the button. I then created a custom_cleaning command that allows the user to set whichever values they prefer (depending on botvac version). I am not very well versed so trying to do the best I can with my limited knowledge :)

Here is what I have:

https://github.com/dshokouhi/pybotvac/commit/dc58abc9e20c90d9d176f2af44bccbfaca4f41d9

Here is the home assistant code to go along with it:

https://github.com/dshokouhi/hass-neato-custom-component/tree/custom_cleaning

Let me know if you think this is a good approach and I can submit a PR if not open to suggestions :)

stianaske commented 6 years ago

@dshokouhi I believe the same could be achieved by using the existing version of start_cleaning and passing in non-default arguments for category? Or am I missing something? The hard-coded 'catecory': 2 in the current version of start_cleaning is a temporary work-around that we should change any way.

Adhering to the DRY principle, I would prefer that we keep one version of start_cleaning and either catch the exception occurring when no persistent map is created, or by first checking if a persistent map has been created and then default to use persistent map or non-persistent map based on the result.

As I don't have access to a D7, it's hard for me to test the code for this, but I'll see what I can do.

dshokouhi commented 6 years ago

@stianaske I think that would be better. My other thought was to allow the user to set whichever values they want and if no values are provided we stick to the defaults that will always work. If you need any help in testing let me know :)

mike9011 commented 5 years ago

Hi guys!

Is this still in development? I would like to change it in Hassio, but on HassOs I'm not able to edit the local library. Otherwise I need to find another solution.

Thank you! :)

stianaske commented 5 years ago

I merged this into the dev branch and added some logic to retry in case the category 4 cleaning throws an exception. Could you guys with D7s see if this seems to work/make sense?

dshokouhi commented 5 years ago

@stianaske I just tested it but it is not working. There is no exception thrown I just get an alert. This is the alert given 'nav_floorplan_load_fail'

stianaske commented 5 years ago

@dshokouhi I see. And this is when your robot makes the annoying beeping sound as well? And the error is in state['error']?

dshokouhi commented 5 years ago

@stianaske yes it does but it is not in state['error'] its in state['alert']

{'version': 1, 'reqId': '1', 'result': 'ok', 'data': {}, 'error': None, 'alert': 'nav_floorplan_load_fail', 'state': 1, 'action': 0, 'cleaning': {'category': 4, 'mode': 2, 'modifier': 1, 'navigationMode': 1, 'mapId': '', 'spotWidth': 0, 'spotHeight': 0}, 'details': {'isCharging': True, 'isDocked': True, 'isScheduleEnabled': False, 'dockHasBeenSeen': False, 'charge': 98}, 'availableCommands': {'start': True, 'stop': False, 'pause': False, 'resume': False, 'goToBase': False}, 'availableServices': {'findMe': 'basic-1', 'generalInfo': 'basic-1', 'houseCleaning': 'basic-4', 'IECTest': 'advanced-1', 'logCopy': 'basic-1', 'manualCleaning': 'basic-1', 'maps': 'basic-2', 'preferences': 'basic-1', 'schedule': 'basic-2', 'softwareUpdate': 'basic-1', 'spotCleaning': 'basic-3', 'wifi': 'basic-1'}, 'meta': {'modelName': 'BotVacD7Connected', 'firmware': '4.3.1-180'}}
stianaske commented 5 years ago

@dshokouhi What about the last commit?

Edit: I think I need to go buy a D7 :)

dshokouhi commented 5 years ago

@stianaske it looks like it works :) it is hard to tell since sometimes commands are delayed from Neato but once I delete a floorplan the botvac gets the alert then a few seconds later it starts house cleaning. The other botvac that has a floorplan is not impacted by this.

dshokouhi commented 5 years ago

@stianaske I found one more alert to add to the list, if the botvac returns the following then it will not clean nav_floorplan_localization_fail this happens if the floorplan changed (i.e. you are standing in front of it or not everything is picked up off the floor around the botvac). The botvac does an initial scan of the room and before it cleans it does this.

Here is the full state output:

{'version': 1, 'reqId': '1', 'result': 'ok', 'data': {}, 'error': None, 'alert': 'nav_floorplan_localization_fail', 'state': 2, 'action': 4, 'cleaning': {'category': 4, 'mode': 2, 'modifier': 1, 'navigationMode': 1, 'mapId': '', 'spotWidth': 0, 'spotHeight': 0}, 'details': {'isCharging': False, 'isDocked': False, 'isScheduleEnabled': False, 'dockHasBeenSeen': True, 'charge': 98}, 'availableCommands': {'start': False, 'stop': False, 'pause': True, 'resume': False, 'goToBase': False}, 'availableServices': {'findMe': 'basic-1', 'generalInfo': 'basic-1', 'houseCleaning': 'basic-4', 'IECTest': 'advanced-1', 'logCopy': 'basic-1', 'manualCleaning': 'basic-1', 'maps': 'basic-2', 'preferences': 'basic-1', 'schedule': 'basic-2', 'softwareUpdate': 'basic-1', 'spotCleaning': 'basic-3', 'wifi': 'basic-1'}, 'meta': {'modelName': 'BotVacD7Connected', 'firmware': '4.3.1-180'}}
dshokouhi commented 5 years ago

It looks like this alert behaves in a different fashion because it is able to load the floorplan but a few seconds later it fails and does not start back up. It actually does begin spinning up the brush so its attempting :)

stianaske commented 5 years ago

@dshokouhi I take it that nav_floorplan_localization_fail should not be handled the same way as nav_floorplan_load_fail then (as cleaning fails, but for a different reason)?

dshokouhi commented 5 years ago

@stianaske yea it looks like nav_floorplan_load_fail will show up with the response code as tries it tries to verify it can clean the map but the map does not exist. nav_floorplan_localization_fail will only show up after the map was loaded but an object is out of place. So the alert shows up later on. I have actually submitted a PR to home assistant so that this alert can be more visible to the user.

https://github.com/home-assistant/home-assistant/pull/19238