Create individual buffer sizes for sending and receiving buffers #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add individual buffer sizes for sending the receiving buffers
jpeterse
changed the title
…-and-receiving-buffers
|
I think this should be a major release, as API changes. |
|
See changes in header file. |
|
Agreed that is also what I advised for in the merged PR 12. Additionally I will also work on fixes in the ThingsBoard Client SDK to integrate the breaking changes of this API as soon as possible. |
|
@imbeacon Would be nice if the newest release |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After pull request #10 the memory need of the library doubled.
Not a big issue if default buffer size is used.
But in my case, I integrate with Home Assistant, which need some fairly large MQTT messages to provision a device into the platform. The messages returned by Home Assistant however, are small.
My sending buffer need to be > 1150 bytes, while I only need about 150 bytes for the receiving buffer.
So I suggest the following changes.
Add the ability to separate sizes for sending and receiving buffers.
My proposed implementation does introduce breaking changes.
setBufferSize method has an additional parameter, and function getBufferSize is renamed.
I propose the breaking changes, because while setting a default value to the second parameter of setBufferSize function, and make a new function to set the other buffer, could make the interface persistent with current implementation, it would make it unclear which buffer size is being set.
Equally, getBufferSize would no longer clearly articulate which buffer size it is returning.
Overall, it's my opinion that the overall benefit of better memory utilization justify the breaking changes.
I also noticed from #11 , that the receiving buffer is used to submit user credentials to the broker. I have submitted separate pull request for the connect function to use the send buffer instead. Thereby, making sure it use a buffer that are appropriately sized.