Multiple changes

[kevinkk525]

This pull request is merely to give you an overview of the changes I made, so you can have them in mind when looking at the code. I know you'll only start working on this once you check the module for the new hardware and retest the reliability. Thanks for your efforts with this module.

Changes:

1. Sorted files and made a structure that made sense so you know which file belongs where without reading the documentation every time
2. Made the repo a module to be used like from micropython_mqtt_as.mqtt_as import MQTTClient,
   making it possible to just clone the repo and copy it to espXXXX/modules also reducing file clutter in this directory.
3. Adapted code to ESP32 loboris fork as sys.platform returns a different string as the official port and all workarounds for the official port are not needed in this fork except the Wifi credentials. [change already implemented in your repo by now]
4. Changed MQTTClient constructor initialization from using a dictionary to using keywords with default parameters.
5. Made a minimal version of mqtt_as for the ESP8266 [can redo this once you get a new version released]
6. Added support for "unsubscribe" command
7. Added support for recognizing a retained publication. This is also a breaking change as the "subs_cb" now has to accept 3 args: topic, msg, retained
8. Improved reliability when (un-)subscribing to many topics in a short time (resulted in self.pkt being overwritten before response was received/processed, resulting in endless reconnects)
9. Added Lock.release() as a workaround until bugfix is released in the next micropython update. Also added Lock.locked() to make it more generally compatible and useable.
10. Updated all other files to work correctly with these changes
11. Updated documentation to reflect all changes

[kevinkk525]

The pyboard D commit looks good.
Will you take a look at these changes too?
Notably 1,2,4,6,7,8 and maybe the pause/resume feature I implemented on a different branch?

[peterhinch]

I haven't forgotten these, but they do impose substantial API changes on existing users. By contrast my latest commit has zero API changes :)

Re 1,2,4 I'm still unsure whether the gains, particularly from using a package and replacing the config dictionary, justify the API disruption. I originally started out with lots of constructor args and changed to the dict. This does have some advantages. For example it is initialised with default args in the library but you can modify it with project-level defaults in config.py. So, in a project where (say) the server IP or WiFi credentials change, you just distribute a new config.py. You can, of course, pass **config to the multi-parameter constructor, but if you're going to do that you might as well stick with the dict.

7 (retained messages) is definitely worth doing.
Re 6, 8 I'm not convinced about unsubscribing: are embedded applications likely to unsubscribe?

Right now I'm busy trying to understand some weird behaviour of Pyboard D applications and am trying to produce a decent test case.

[kevinkk525]

Thanks for your answer.
I can understand that you are worried about API changes to existing users. This however always slows down progress. Maybe an updated version with the current API can be kept on a separate branch that'll only receive bugfixes once stable?

1,2,4 would be nice to have if a new version already changes the API.
The advantages you see in using a dict are not what I can see. With normal constructor args you can have all those default values too and you will have a project-level config anyway that will store the needed values that will overwrite the default ones and can therefore still be easily swapped by distributing a new config.py.

7 (retained messages) is where the biggest API change lays because the function that is being called when a new message is received now takes 3 args instead of 2.

6 unsubscribe: I use it often. On topic "home/device/state" I keep the retained status that the device has. On topic "home/device/state/set" the device receives commands to change that state to a new state.
So on reboot my devices subscribes to the state topic ("home/device/state") to get the retained value and restores the state it had before its reboot. Then it unsubscribes that topic because it does not need that anymore to free resources (as you need a higher level subscription handler to know what function to call on which topic). Then it subscribes to the command topic ("home/device/state/set") and listens to changes that might even have occured during its outage.
I don't know if that's the best way for restoring device states but I did not find anything better yet.
Therefore I don't know if embedded applications are likely to use unsubscribe but mine do. It is however not a big code change to implement unsubscribe and does only extend the API, not break compatibility.

8 is also useful with only subscribe as I had problems subscribing from different uasyncio coros in short succession.

Pause/Resume might be helpful for battery powered devices to properly disconnect from the server to save power.

Once your new version is stable and ready, I can pull those changes and create pull requests only with the changes you want to implement. That will hopefully make it easier for you.

Good luck hunting down the problems with the Pyboard D! Hope you can find it soon.

[kevinkk525]

Just as an update as time has passed:

• Pulled your changes
• Added support for the unix port
• optional feature in a different branch: pause and resume to cleanly disconnect before shutting down wifi to save power and resume when starting wifi again (as requested by Mike, don't know about anyone else who wanted that feature)

[peterhinch]

This looks great, thank you.

I think the changes for the new uasyncio should be a separate PR rather than an addendum to this very large one. Then I could simply do a merge.

[kevinkk525]

Ah yes, I forgot that this PR pulls all the changes from my master repository. I guess this can be closed since you won't be merging the remaining changes anyway.