Reduce duplicated code + improve structure
Hi,
First of all, thanks for creating and maintaining this repository. I do see some things that I think can be improved, and would like to contribute here. These are not small changes though, so I'll lead with the main question I have: I read in other issues that you are working in an internal repo and will later add changes here. That makes it a bit hard for external contributors to make larger changes. Are you interested and willing to accept large changes, and if so, how would that process look like?
I'll now detail a bit more what changes I would propose.
The main issue I'd like to tackle is the duplicated code.
There's a lot of copy-paste between the MQTT 3.1.1 and MQTT 5.0 implementation for the client.
I don't think this is necessary, and makes maintenance a lot harder.
Additionally, the protocol implementation could be shared between rumqttc and rumqttd.
I've already worked on an implementation in my fork, but you'll see that the diff is large. What I've done:
- extract the protocol (the two
mqttbytesfolders forrumqttc) into a newrumqtt-bytescrate This implements version 4 and 5 of the protocol in a similar way as it's done inrumqttd/src/protocol. This crate then used inrumqttcand themqttbytesfolders are removed. I haven't used it inrumqttdyet, but that should be pretty straightforward. - unified the client, eventloop, state, ... implementations in
rumqttcTo come to a single implementation that works for MQTT 3.1.1 and 5.0. - restructured some of the code for better readability
Created new modules for the
TransportandMqttOptionstypes. - moved to a common
Propertiesstructure that can be used for all packets Instead of having packet specific properties (ConnectProperties,SubscribeProperties,PublishProperties, ...) I think it makes sense to have a commonPropertytype and store the properties as aVec<Property>. That does incur a heap allocation, but since everyStringorVecproperty already allocates, I don't think this has a big impact. There are many advantages though:- the packet becomes smaller, since the properties are now on the heap instead of in the packet itself
- you only need space for the properties that exist, not all possible properties for a packet
- common implementation of property handling instead of having to duplicate it for each packet type
- reworked the eventloop implementation to be easier to understand and maintain
These changes touch just about everything in rumqttc, so the diff is very large, as I already said.
I'm interested to hear what you think about it, and what changes you've already made in your internal repo.
@jspngh we are currently working on the new architecture for rumqttc. we shall publish new architecture ASAP.
Hey @giridher-art,
What's the current status on your end? I'd like to contribute improvements here, but it seems you don't currently have the bandwidth for the open-source maintenance of this library.
Could you tell me if either:
- the situation will change in the near future and you will have more bandwidth
- you are willing to accept outside help for the maintenance
If neither is true (which is totally fine, it's just a matter of setting the right expectations) I know that I don't have to wait and need to look somewhere else.
hey @jspngh we are working on new architecture of rumqttc as well as new mqtt broker . we shall release all the further details ASAP and also , we appreciate any outside contributions as well.
Thanks for the update, I'll stay patient then.