stianaske / pybotvac

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

Validiate responses #68

Closed Santobert closed 3 years ago

Santobert commented 3 years ago

There have been some errors recently that indicate that neatos API is not behaving according to their documentation. For this reason, we are now validating all responses. This is not disruptive for now, as all errors in robot.py are logged and then ignored. Only errors that occur in account.py will stop the further process as it makes no sense to continue with invalid data.

Relevant issues:

Closes: #18

Still a draft, as extensive testing is required I think we can merge this. At least to solve the issues listed above as soon as possible. @dshokouhi What do you think?

ahknight commented 3 years ago

As requested in https://github.com/home-assistant/core/issues/44709#issuecomment-758117765, here's the output on an account with a robot that has a bad response (note I added a pprint line to def state so it appears twice; that's a local change):

Robots:
Invalid response from https://nucleo.neatocloud.com/vendors/neato/robots/GPC3XXXX-XXXXXXXXXXXX/messages: required key not provided @ data['availableServices']. Got: {'version': 1, 'reqId': '1', 'result': 'ok', 'data': {}}
<Response [200]>
Your robot Upstairs is unsupported.
<Response [200]>
Name: Downstairs, Serial: OPS4XXXX-XXXXXXXXXXXX, Secret: A...0 Traits: ['maps', 'persistent_maps']

<Response [200]>
{'action': 13,
 'alert': 'dustbin_full',
 'availableCommands': {'goToBase': False,
                       'pause': False,
                       'resume': False,
                       'start': False,
                       'stop': False},
 'availableServices': {'IECTest': 'advanced-1',
                       'findMe': 'basic-1',
                       'generalInfo': 'basic-1',
                       'houseCleaning': 'basic-4',
                       'logCopy': 'basic-1',
                       'manualCleaning': 'basic-1',
                       'maps': 'basic-2',
                       'preferences': 'basic-2',
                       'schedule': 'basic-2',
                       'softwareUpdate': 'basic-1',
                       'spotCleaning': 'basic-3',
                       'wifi': 'basic-1'},
 'cleaning': {'category': 4,
              'mapId': '',
              'mode': 1,
              'modifier': 1,
              'navigationMode': 1,
              'spotHeight': 0,
              'spotWidth': 0},
 'data': {},
 'details': {'charge': 96,
             'dockHasBeenSeen': False,
             'isCharging': False,
             'isDocked': False,
             'isScheduleEnabled': True},
 'error': 'gen_picked_up',
 'meta': {'firmware': '4.5.3-189', 'modelName': 'BotVacD7Connected'},
 'reqId': '1',
 'result': 'ok',
 'state': 4,
 'version': 1}
State:
 {'version': 1, 'reqId': '1', 'result': 'ok', 'data': {}, 'error': 'gen_picked_up', 'alert': 'dustbin_full', 'state': 4, 'action': 13, 'cleaning': {'category': 4, 'mode': 1, 'modifier': 1, 'navigationMode': 1, 'mapId': '', 'spotWidth': 0, 'spotHeight': 0}, 'details': {'isCharging': False, 'isDocked': False, 'isScheduleEnabled': True, 'dockHasBeenSeen': False, 'charge': 96}, 'availableCommands': {'start': False, '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-2', 'schedule': 'basic-2', 'softwareUpdate': 'basic-1', 'spotCleaning': 'basic-3', 'wifi': 'basic-1'}, 'meta': {'modelName': 'BotVacD7Connected', 'firmware': '4.5.3-189'}}

<Response [200]>
Schedule enabled: True
Disabling schedule
<Response [200]>
Schedule enabled: False
Enabling schedule
<Response [200]>
Schedule enabled: True
Santobert commented 3 years ago

Thanks @ahknight. The behavior is as intended. First we try to fetch the robot state. As we can see, the invalid response was logged. Then we try to use this state. Since it' s invalid, we log that too that and skip that robot. The other one works as expected and remains usable.

Santobert commented 3 years ago

@stianaske I'll fix these conflicts right now. Just give me 2 minutes nvm :laughing: