KeyError('token') in EncryptionAdapter

[gunjambi]

Describe the bug
Library prints Unable to decrypt, returning raw bytes message for Hello packets due to missing context. This scary text confuses people when they debug unrelated issues. (yes, the issue title is a clickbait)

Details

• In MiIOProtocol::discover at

  python-miio/miio/miioprotocol.py

  Line 121 in f7b554f

  m: Message = Message.parse(data)

  we parse the Hello Message without giving context token
• In EncryptionAdapter::_decode at

  python-miio/miio/protocol.py

  Line 165 in f7b554f

  decrypted = Utils.decrypt(obj, context["_"]["token"])

  token is accessed from the context. It doesn't exists in the context and this throws KeyError('token')
• The error handling thinks this is a decoding error and prints a scary warning at

  python-miio/miio/protocol.py

  Line 168 in f7b554f

  _LOGGER.debug("Unable to decrypt, returning raw bytes: %s", obj)

Proposed fix
Since the hello message is parsed differently in any case (see Utils.is_hello), we could separate the message types as:

Message = Struct(
    # for building we need data before anything else.
    "data" / Pointer(32, RawCopy(EncryptionAdapter(GreedyBytes))),
    "header"
    / RawCopy(
        Struct(
            Const(0x2131, Int16ub),
            "length" / Rebuild(Int16ub, Utils.get_length),
            "unknown" / Default(Int32ub, 0x00000000),
            "device_id" / Hex(Bytes(4)),
            "ts"
            / TimeAdapter(
                Default(
                    Int32ub,
                    datetime.datetime.now(tz=datetime.timezone.utc),
                )
            ),
        )
    ),
    "checksum" / Checksum(Bytes(16), Utils.md5, Utils.checksum_field_bytes),
)

DiscoverMessage = Struct(
    "header"
    / RawCopy(
        Struct(
            Const(0x2131, Int16ub),
            "length" / Const(0x20, Int16ub),
            "unknown" / Default(Int32ub, 0x00000000),
            "device_id" / Hex(Bytes(4)),
            "ts"
            / TimeAdapter(
                Default(
                    Int32ub,
                    datetime.datetime.now(tz=datetime.timezone.utc),
                )
            ),
        )
    ),
    "token" / Bytes(16),
)

Additionally.

• In _discover , we'd then use DiscoverMessage instad of Message
• We remove Utils.is_hello
• We replace m.checksum when accessing the token with m.token in miioprotocol.py

Other
I can also do a PR if that's prefereable. It'll take me a few moments to set up the environment first though.

[rytilahti]

Hi Jarkko, that piece of code is one of the oldest in this project and it's indeed confusing for developers and end-users alike... Feel free to create a PR to separate the discovery from the handling of payload messages! The instructions for setting up a development env are documented in https://python-miio.readthedocs.io/en/latest/contributing.html .

A cleaner separation will likely cleanup the code a bit, allow better error reporting towards users (there are plenty of hard-to-debug reports like home-assistant/core#92774 home-assistant/core#59215 home-assistant/core#67926 to name some from homeassistant repo, there are plenty in issues of this one, too where having a clearer separation in errors would be very useful to have), and open a way for cleaner implementation of #1683 in the future.