micropython-lib icon indicating copy to clipboard operation
micropython-lib copied to clipboard

umqtt.simple: refactor packet de/encoding and fix remaining length encoding (fixes #284)

Open SpotlightKid opened this issue 7 years ago • 3 comments

As stated in #284, the MQTTClient.subscribe method doesn't handle topic lengths >= 122 correctly, since it only allocates one byte for the remaining length portion of the SUBSCRIPTION packet. I've refactored the variable length encoding of the remaining length bytes to use the new _varlen_encode method consistently and also got rid of the usage of the ustruct module.

  • Add _varlen_encode method to write variable length encoded remaining length bytes to packet buffer and use it consistently.
  • Use int.from_bytes/to_bytes to de/encode short integers and replace ustruct usage with it.
  • Correctly separate bytes of fixed and variable header in connect method.

In a separate commit I've also removed all (commented ou) debug printing calls in simple.py and added a debug wrapper for MQTTClient (umqtt_debug.py) to the examples, which can optionally be used with examples, and I've also expanded example_pub.py and example_sub.py so that they can be parametrized form the command line. If desired, I can separate this commit into an extra PR.

This PR is based on the branch for PR #302. If the latter is merged, I will rebase this on master.

SpotlightKid avatar Aug 27 '18 15:08 SpotlightKid

@pfalcon this looks like an important bugfix - any chance you could review this?

Kriechi avatar Dec 29 '18 21:12 Kriechi

@Kriechi It seems that only the fork (or original, depending on how you look at it) of micropython-libat https://github.com/pfalcon/micropython-lib is maintained and developed any further, ATM.

I could re-submit my PR there, but I have basically given up on MicroPython, and I probably won't put any more effort into it. Feel free to take this PR (and any others I have made in this repo) and re-submit it (them) to pfalcon's repo.

SpotlightKid avatar Dec 29 '18 21:12 SpotlightKid

Thank you @SpotlightKid! Without your refactor bug fix I wasn't able to subscribe to my AWS topic, although it was only 63 bytes in length. Now it is working!

dl2080 avatar Feb 09 '21 17:02 dl2080

Closing this since I'm cleaning my unmerged PRs and this hasn't been acted upon in years.

SpotlightKid avatar Aug 24 '23 11:08 SpotlightKid