feat: Implement L01 protocol #487
Conversation
| ping_message = RoborockMessage( | ||
| protocol=RoborockMessageProtocol.PING_REQUEST, | ||
| ) | ||
| await self._send_message( | ||
| roborock_message=_PING_REQUEST_MESSAGE, | ||
| request_id=_PING_REQUEST_MESSAGE.seq, | ||
| roborock_message=ping_message, | ||
| request_id=ping_message.seq, | ||
| response_protocol=RoborockMessageProtocol.PING_RESPONSE, | ||
| ) |
There was a problem hiding this comment.
We don't need to set ping request statically like we did. I know we talked about this before but I wasn't fully sure.
This works
| self._logger.error(e) | ||
| if response.version.decode() == "L01": | ||
| self._ack_nonce = response.random | ||
| self._set_l01_encoder_decoder() |
There was a problem hiding this comment.
So, up until this point the encoder was used without a connect nonce or an ack nonce. But then for all future messages beyond the hello, these must be used? Is that a requirement or can the connect nonce be set up front in the encoder even for the hello message?
Is response.random set to None in the non-L01 protocols? Does it hurt to set it on future requests for non-L01 protocols too?
There was a problem hiding this comment.
response.random is not None for non-L01 protocols. However, it is actually fine if they exist, that is a good call. It isn't used on encrypting the 1.0 payload, so those values can be anything.
As well, the hello message does not include any payload, so the value doesn't matter there as well.
The only potential issues to setting connect_nonce upfront is that if the user reconnects, it will be the same nonce, but the vac should handle that fine as it is a new transport thread.
| ): | ||
| if method in CLOUD_REQUIRED: | ||
| raise RoborockException(f"Method {method} is not supported over local connection") | ||
| if self._version == "L01": |
There was a problem hiding this comment.
This all seems like the responsibility of RequestMessage.encode_message and should live in protocol. Firstly because the code follow this pattern so its cleaner and unit testable, but also so that it can be reused by the other client.
There was a problem hiding this comment.
Great call, as it turns out all the logic was similar enough to just carry over with a small change.
|
Ready for another pass, all the logic is working, I've made a number of commits and I would say this is ready for primetime when we finish up reviews |
This is mainly working, but since we really need to get this done asap, I figured I would go ahead and put a PR up while the rest is being figured out.