stianaske / pybotvac

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

No robots listed when schema verification fails on non-required fields #70

Closed stianaske closed 3 years ago

stianaske commented 3 years ago

Issue

Running the latest commit (34b5941), I am unable to list any robots because the response verification fails.

Steps to recreate

Running

account = pybotvac.Account(pybotvac.PasswordSession(email=<email>, password=<password>))
for robot in list(account.robots):
  ...

on my account prints the following

Bad response from robots endpoint: expected str for dictionary value @ data['timezone']. Got: {'serial': 'OPSblabla', ... 'timezone': None, ...}

and the for loop returns.

Explanation

The reason for this appears to be that self._robots.add(robot_object) is being skipped if response verification throws in refresh_robots() in account.py, regardless of whether the exception is due to a required or non-required field missmatch:

    def refresh_robots(self):
        """
        Get information about robots connected to account.

        :return:
        """

        resp = self._session.get("users/me/robots")

        for robot in resp.json():
            _LOGGER.debug("Create Robot: %s", robot)
            try:
                ROBOT_SCHEMA(robot)
                robot_object = Robot(
                    name=robot["name"],
                    vendor=self._session.vendor,
                    serial=robot["serial"],
                    secret=robot["secret_key"],
                    traits=robot["traits"],
                    endpoint=robot["nucleo_url"],
                )
                self._robots.add(robot_object)
            except MultipleInvalid as ex:
                # Robot was not described accordingly by neato
                _LOGGER.warning(
                    "Bad response from robots endpoint: %s. Got: %s", ex, robot
                )
                continue
...

Proposed fixes

  1. ROBOT_SCHEMA should allow None as timezone entry
  2. Verification of a non-required field should create a log entry, but the rest of the code should execute as normal. If this is not possible, non-required fields should not be verified.
stianaske commented 3 years ago

@Santobert Do you agree with proposed fixes?

Santobert commented 3 years ago
  1. Yea this may be correct. Since timezone is not documented at all we shouldn't be too restrictive here. Let's use the following ROBOT_SCHEMA
ROBOT_SCHEMA = Schema(
    {
        Required("serial"): str,
        "prefix": Any(str, None),
        Required("name"): str,
        "model": str,
        Required("secret_key"): str,
        "purchased_at": Any(str, None),
        "linked_at": Any(str, None),
        Required("traits"): list,
        # Everything below this line is not documented, but still present
        "firmware": Any(str, None),
        "timezone": Any(str, None),
        Required("nucleo_url"): Url,
        "mac_address": Any(str, None),
        "created_at": Any(str, None),
    },
    extra=ALLOW_EXTRA,
)

We could do same with MAPS_SCHEMA and PERSISTENT_MAP_SCHEMA and allow None for non required fields as well.

  1. I am not 100% sure how to achieve this. Voluptuous checks if the Required() fields are present at all and then if the field has the expected typing. So we can't distinguish between critical and less critical errors this way. Let's stay binary here. Either we can work with the given response or not.

I can file a PR if you agree