6 years later...

I was researching before making the exact same PR. Having read all of the comments on#186, I agree that users ofumqtt.robust are responsible for resubscribing, and this PR makes it easier to do so.

To that end, myafter_connect() is only called if the broker returns a clean session, so I have an extraif compared to the example above (line 6):

fromumqttimportrobustclassMQTTClient(robust.MQTTClient):defconnect(self,clean_session=True):res=super().connect(clean_session)ifnotres:self.after_connect()returnresdefafter_connect():#use for mqtt subscribereturn

@pfalcon and@dpgeorge: FYA - no extra code, no broadening of the scope of the module, behaviour identical to the currentumqtt.robust, just more easily extensible.

@andrewleech, fancy reviewing / merging this PR? Do you see any pitfalls?

I actually think it looks really strange to call self.connect when there is no actual self.connect function, it's confusing.

I can guarantee new users coming to this module won't understand the need for this, at least not without examples / docs.

I think if the end goal is to make it easier for users to make reconnect re-subscribe as needed, assuming this is a common need, scaffolding to do exactly that should be added instead.
Either your code with an empty after_connect function should be added to robust with a comment saying "subclass this add fill out the function", or provide a way for users to pass in a callback that will be run instead.

I appreciate the review, and still think the PR, as is, has merit. Addressing the general thrust of your critique...

Strange and confusing

Agreed, to the uninitiated, the reference toself.connect() may look strange and be confusing. That doesn't make it wrong however and, as you suggest, it could be explained in docs / examples which I'd be happy to add if you agree.

The end goal

Depending upon who you cite, the 'end goal' can differ quite a lot. Expanding robust's scope has been debated at length, particularly in#186, and I'm generally content with its present scope (my interpretation):

1. Minimum code size
2. Broker assumes all possible 'robustness' functions
3. An example to all of sub-classingumqtt.simple.MQTTClient
4. A convenient working client with some network fault tolerance out-of-the-box

A common enough wish is that robust would provide automated re-subscription when a clean session is returned fromconnect() withinreconnect(); the cost: code size (1 above). This can already be achieved through (2 above) so I agree with the argument not to change the module.

Outcomes

This PR makes implementing re-subscription more compact:without it, the user has to overridereconnect() thereby repeating the 8 lines of the same method in robust before adding code to re-subscribe - not very DRY.With this PR, the user has to callsuper().connect() at some point, but is otherwise free to focus on their application's logic.

I think it bears repeating that this change doesn't increase code size and doesn't alter existing built-in behaviour. An elegant solution that makes it easier for users to leverage robust whilst building upon it.

Either your code with an empty after_connect function should be added to robust with a comment saying "subclass this add fill out the function", or provide a way for users to pass in a callback that will be run instead.

This is a separate issue IMHO. It has merit and I think people would use it, but I think it would form part of a third umqtt module. I'd be happy to collaborate because that's effectively what I'm implementing - just think it should stay clear of robust.

When you mention a possible third version, well to be honest I've never used this robust module because any time I want to use mqtt I go straight tohttps://github.com/peterhinch/micropython-mqtt and in particular his async version.

Regardless, I appreciate the argument that this change here keeps it lean, and with an example and/or docs it has merit in terms of a cheap increase in flexibility.