Separate network hardware interface from mqtt client

[kevinkk525]

Goal:

• Easier maintainability of the repository (network interfaces and hardware keep growing)
• mqtt client easier usable with a variety of network interfaces (none for unix, wlan, ethernet, custom interfaces)
• users/devs can write their own network interface integration
• stay backwards compatible. If no hardware interface is given, a WLAN interface will be created.

This is a first draft that hasn't been tested yet. Looking for feedback on the implementation.

(Reminder for later: Changed mqtt_as folder to package due to relative imports. Have to test how that works with backwards compatibility. However, users will have to install this correctly anyway and might then need to change their import line, but nothing else.)

Note about the interface change callbacks: mqtt_as originally called the wifi callbacks after the connection to the broker was established, which looking back was actually a bug imho (line 527). Now it is called directly after the successful wlan connection (which is actually at least 5 seconds later due to the code waiting for wifi stability).

Future thoughts:

• Use of disconnect when trying to reconnect the interface is problematic because the interface can't differntiate between a legit disconnect request (with interface being down afterwards) and a simple reconnect attempt from the mqtt client to ensure a stable WLAN link (which might not be needed for other interfaces and might cause problems with multiple parallel projects using the interface).

[kevinkk525]

Anyone got any thoughts about the changes?

[kevinkk525]

However - I still think you should implement two callbacks:

* one for connected

* one for disconnected

It's a tiny overhead in the base library but it makes things much more pleasant to work with, especially if you have multiple code parts that interact with the interface.

Hmm yes this would be debatable, however, the original mqtt_as only uses one callback, so to have compatibility to existing projects, I can't switch to two separate callbacks, even though the change in code is tiny.

[spacemanspiff2007]

Hmm yes this would be debatable, however, the original mqtt_as only uses one callback, so to have compatibility to existing projects, I can't switch to two separate callbacks, even though the change in code is tiny.

Couldn't we just use the original callback and subscribe it for both callbacks? That way we could have two different callback possibilities for the future while the existing implementation still works.

[kevinkk525]

Having 2 callbacks also raises the question of which arg should be passed. Might still be ok with the wifi state as boolean. However, that workaround will work for your custom interface too, there's honestly no need to make such a workaround in the base interface. You can just put another callback in between in your handler or subclass it and change the callback function. Looks like you'll want a more complex custom interface anyway. But that's the beauty of this new approach, there's no need to add every option or fight over features. Everyone can just write an interface themselves with the features they need and other users can choose from all existing implementations.

[spacemanspiff2007]

Having 2 callbacks also raises the question of which arg should be passed.

No args at all. Just a possible callback for connected and disconnected. It's clear from the register function what you are subscribing to.

It actually boils down to "when do you want to do something specific". And the answer is "If the interface is up" and/or "if the interface is down". If this is not provided by the baseinterface the first line will always be:

async def my_connected_func(state):
    if not state:
        return None
    ...

async def my_disonnected_func(state):
    if state:
        return None
    ...

Looks like you'll want a more complex custom interface anyway.

My implementation is actually already done - I'm just trying to contribute something, too.

Could we make a Wifi a base class which holds only the credentials and then inherit the device specific implementation from there? e.g.

BaseInterface -> WifiInterace -> Esp32Wifi

[kevinkk525]

If this is not provided by the baseinterface the first line will always be:

Seems like an acceptable workaround.

No args at all. Just a possible callback for connected and disconnected. It's clear from the register function what you are subscribing to.

It is, but building a workaround for backwards compatiblity with one callback would be rather big. Current goal is to stay as small as possible and as compatible as possible. That said, I do look forward to creating a better interface handler but people who only use mqtt_as on their device (especially on esp8266) won't need more features and want to save as much RAM as possible.

Could we make a Wifi a base class which holds only the credentials and then inherit the device specific implementation from there? e.g. BaseInterface -> WifiInterace -> Esp32Wifi

Yes I might do that, there is some common code for checking wifi stability and the wifi credentials and other ports might get added later. better that they don't need to copy that part too.

[spacemanspiff2007]

It is, but building a workaround for backwards compatiblity with one callback would be rather big. Current goal is to stay as small as possible and as compatible as possible.

I'm afraid I don't understand your point. In the mqtt_as you can just subscribe to both - it's one line of code more.

Seems like an acceptable workaround.

But if you start scheduling coros it gets bloated quickly. Two list objects to, two task objects for cancelation, checking if the task is already running, etc.

[Molaire]

What is the state of this PR? Is help needed?

[kevinkk525]

Due to my current health situation I can't even use a PC or phone anymore and it might not change. So I wouldn't mind someone taking over. On 11/22/21, 18:05 Vincent Thibault @.***> wrote:

What is the state of this PR? Is help needed? —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.

[ebolisa]

Sorry to hear that. Thanks for your contribution and take care of yourself.

[spacemanspiff2007]

@kevinkk525 Shocked to hear that! I really hope that you get well again! Best wishes!

[mertozal]

Hi Kevin, I hope you are in good health.

Is there any chance of completion of this pull request?

If this is still not possible, is it possible for you to write what needs to be done?

If Kevin can't help with this, is there any chance for you to help @peterhinch ?

[peterhinch]

I have no plans to work on this. Recent aims have been supporting and testing new WiFi platforms and helping support a low power application which takes down the interface programmatically - so this PR has probably become rather stale.