xHasKx / luamqtt

luamqtt - Pure-lua MQTT v3.1.1 and v5.0 client
https://xhaskx.github.io/luamqtt/
MIT License
154 stars 41 forks source link

adding a streaming interface #32

Closed Tieske closed 2 years ago

Tieske commented 2 years ago

Hi @xHasKx

Thx for this nice library, I've been eyeballing MQTT for a while now, and thought to give it a go actually.

The code looks nice and clean, but what I'm missing is a streaming interface. Looking at the code, currently client.lua mixers client functionality with io-loop functionality. Which isn't the right separation of concerns imho.

So would you be interested in a streaming interface? I've seen another issue about blocking IO (#23 ). So what I would propose to separate the client from the runloop is along the following lines;

This way the client and the runloop are cleanly decoupled.

wdyt?

as a start I sent a PR (#31 ) to expose the keepalive functionality to external runloops.

xHasKx commented 2 years ago

@Tieske, please take a look at the Connectors topic - https://github.com/xHasKx/luamqtt#connectors They're providing .send() and .receive() methods used by a MQTT protocol parsing code run in the coroutine like this: https://github.com/xHasKx/luamqtt/blob/master/mqtt/protocol5.lua#L811 That approach allows me to write the protocol parsing code much cleaner than with any callbacks.

read_func (recv_func) is created inside MQTT client code here: https://github.com/xHasKx/luamqtt/blob/master/mqtt/client.lua#L751 or here: https://github.com/xHasKx/luamqtt/blob/master/mqtt/client.lua#L1087

For now, that approach allows me to integrate several connectors from different runtime environments (luasocket, copas, nginx). So, the level of client and run-loop coupling is quite low IMO.

But of course you can suggest a better way to decouple network streams reading logic, if it leave the MQTT protocol parsing code in the same form.

Tieske commented 2 years ago

That approach allows me to write the protocol parsing code much cleaner than with any callbacks.

Yes, after spending quite some time reading the code, I noticed that.

But I also ran into some other issues, there are hardcoded error messages in the client.lua code, that are connector specific. That is not safe. For example here. Sometimes a "timeout" is an actual error, and sometimes, it is a notification that reading on the socket should be retried later. (not to mention that "wantwrite" is missing there).

So, the level of client and run-loop coupling is quite low IMO.

The code in client.lua has 2 implementations, a sync one and the ioloop one, that code belong in the repspective loops imo, not the client.

Tieske commented 2 years ago

So I've been updating code, but unfortunately the changes were bigger than I anticipated (sorry about that 😞 ).

I'll push the code to #31 shortly.

Tieske commented 2 years ago

closing this in favor of #31