Don't log error message when decoding valid discovery packets #1832
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1832 +/- ##
==========================================
+ Coverage 80.80% 80.87% +0.07%
==========================================
Files 192 193 +1
Lines 18546 18599 +53
Branches 4012 4021 +9
==========================================
+ Hits 14986 15042 +56
+ Misses 3282 3279 -3
Partials 278 278
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
rytilahti
requested changes
Oct 4, 2023
miio/protocol.py
Outdated
Comment on lines
167
to
168
| if obj == b"": | ||
| return b"" |
There was a problem hiding this comment.
Suggested change
| if obj == b"": | |
| return b"" | |
| if not obj: | |
| return obj |
miio/protocol.py
Outdated
Comment on lines
164
to
166
| # if there is no payload, decode to 0 bytes. Missing payload is expected for discovery messages. | ||
| # note that we don't have "token" in the context for discovery replies so we couldn't decode it | ||
| # anyway. |
There was a problem hiding this comment.
Suggested change
| # if there is no payload, decode to 0 bytes. Missing payload is expected for discovery messages. | |
| # note that we don't have "token" in the context for discovery replies so we couldn't decode it | |
| # anyway. | |
| # Missing payload is expected for discovery messages. |
let's keep it simple here, also, there are some (very) old devices that actually respond with a valid token :-)
rytilahti
changed the title
|
Proposed changes applied. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix
Unable to decrypt, returning raw byteswarning message when decoding Discovery messages. Fixes #1824I opted for a quick surgical fix here rather than rework the whole message parsing. While separating the Discovery messages from normal data messages is quite easy in the context of miioprotocol, there's a quite bit of uses of the
Messagein other places too. I didn't feel confident in changing those.Additionally, it doesn't even seem desirable to separate Discovery-Ack and Data messages. It seems useful in some cases to be able to parse any message without knowing ahead of time what kind of data it might contain. (I doesn't really matter right now as the token, i.e. decryption key, must be supplied ahead of time, but it is conceivable that a parser might first match the ID of the reply to the request and then parse the contents. )
Fixes #1824