fix: Add b01 q10 protocol encoding/decoding and tests#718
fix: Add b01 q10 protocol encoding/decoding and tests#718allenporter merged 2 commits intoPython-roborock:mainfrom
Conversation
| try: | ||
| common_dps_result = _convert_datapoints(common_result, message) | ||
| except ValueError as e: | ||
| raise RoborockException(f"Invalid dpCommon format: {e}") from e |
There was a problem hiding this comment.
i'm unconvinced we should error out here. i.e. a new data point is added, rather than erroring out, we should be made aware of it, but if there are other dps that we DO have, we should continue
There was a problem hiding this comment.
Done, updated to add logic into the enum parsing to handle logging unknown codes and to support optional parsing. Updated the tests to exercise this case with a mix of known and unknown DPS keys.
Lash-L
left a comment
Lash-L
left a comment
There was a problem hiding this comment.
Like it! Just have that one piece of feedback
| @classmethod | ||
| def from_code_optional(cls, code: int) -> RoborockModeEnum | None: | ||
| try: | ||
| return cls.from_code(code) | ||
| except ValueError: | ||
| return None |
There was a problem hiding this comment.
Fine with this - how we handled it on RoborockEnum though is that we had a missing attribute that would automatically be set when you give it a invalid value. Fine either direction though.
There was a problem hiding this comment.
I tried to add it that way but it doesn't work well with the enum constructor (it started saying code was required) and I couldn't really figure it out.
There was a problem hiding this comment.
This seems like it works based on the constructor but not sure if that works here given we're doing from_code. Happy to change it just not sure what to do exactly.
There was a problem hiding this comment.
Not sure either.
What we did before was override missing to handle if the value was missing. But to actually take advantage of it, every instance has to have a specific key added, which isn't ideal. As far as I am aware there is no way to do it from a parent class automatically as enums don't inherit members.
I have no issue with this approach, we will just have to play it by ear and see if we need to change how we handle anything.
2 participants
Pulled this out from #692 and #709 into a smaller PR.
This pushes some of the enum encoding into the protocol library since it is common across all b01 q10 requests. This handles the flattening of COMMON results into the base layer to make processing results easier in the channel / trait layer.