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

Validiate responses #68

Merged
merged 2 commits into from
Jan 18, 2021
Merged

Conversation

Santobert
Copy link
Contributor

@Santobert Santobert commented Jan 7, 2021

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?

@Santobert Santobert force-pushed the validiate_responses branch 2 times, most recently from 015d082 to 8bd3d11 Compare January 10, 2021 00:30
@Santobert Santobert force-pushed the validiate_responses branch from e492bb8 to 529ae2b Compare January 10, 2021 10:39
@Santobert Santobert marked this pull request as ready for review January 10, 2021 10:40
@ahknight
Copy link

ahknight commented Jan 11, 2021

As requested in home-assistant/core#44709 (comment), 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

Copy link
Contributor

@dshokouhi dshokouhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, tested in HA dev branch

@Santobert
Copy link
Contributor Author

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
Copy link
Contributor Author

Santobert commented Jan 18, 2021

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

@stianaske stianaske merged commit b4eda2a into stianaske:master Jan 18, 2021
@Santobert Santobert deleted the validiate_responses branch January 18, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when listing robots
4 participants