-
Notifications
You must be signed in to change notification settings - Fork 791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatically set mqtt buffer size #637
Conversation
Current versions of pubsubclient allows the buffer size to be set dynamically: knolleary/pubsubclient#282 We can take advantage of this to avoid having to modify one of the dependencies and instead automatically up the buffer size when a powerful board is detected.
Thanks for the feedback!
Well seen, this will avoid confusions regarding the MQTT MAX PACKET SIZE modifications.
It will be changed when we release a new version, and done automaticaly by the github actions setup made by @Legion2 I begin the review, thanks for your participation! |
@@ -401,6 +401,7 @@ void connectMQTT() { | |||
char topic[mqtt_topic_max_size]; | |||
strcpy(topic, mqtt_topic); | |||
strcat(topic, will_Topic); | |||
client.setBufferSize(mqtt_max_packet_size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok for this change,
below you can find also a warning
#if defined(ESP8266) || defined(ESP32) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega1280__)
if (MQTT_MAX_PACKET_SIZE == 128)
Log.error(F("WRONG PUBSUBCLIENT LIBRARY USED PLEASE INSTALL THE ONE FROM RELEASE PAGE" CR));
#endif
I think it could be removed and the trace just before log mqtt_max_packet_size
instead of MQTT_MAX_PACKET_SIZE
main/User_config.h
Outdated
# define mqtt_max_packet_size 1024 | ||
#else | ||
# define parameters_size 15 | ||
# define mqtt_topic_max_size 50 | ||
# define mqtt_max_packet_size 128 | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should apply clang format to the files, (we didn't updated the dev docs yet sorry).
Install the Clang-format extension to PIO and do a right click inside the main window of the file and click "format document"
Thank you @1technophile! I actually discovered cd3a2a0 which seemed to suggest the feature was already there at some point, but then later removed? I therefore started having doubts whether maybe there was a problem with it I'm overlooking...if you think it looks ok however, I will take a look at the comments above and apply the changes to this PR 👍 |
Yes later removed due to the fact it was not working, the
No doubt your PR is still relevant ;-) |
@1technophile I found these as well 🙇 : OpenMQTTGateway/docs/upload/troubleshoot.md Lines 48 to 52 in e398dfb
OpenMQTTGateway/docs/use/ir.md Lines 111 to 114 in e398dfb
OpenMQTTGateway/platformio.ini Line 121 in e398dfb
OpenMQTTGateway/platformio.ini Line 133 in e398dfb
I suppose we should do something about those as well? I assume they will get ignored if we begin using |
Well seen ! 😉 |
@1technophile I had a crack at updating the docs in 50819d3, seemed that some of the tips in there were still relevant, so I simply updated the place to make changes, but let me know if I'm off track! |
No, you are on tracks ;-). Thanks |
Thank you for creating (and maintaining!) this fantastic project!
I happened to notice that current versions of
pubsubclient
allow the buffer size to be setdynamically: knolleary/pubsubclient#282
We can take advantage of this to avoid having to modify one of the
dependencies and instead automatically increase the buffer size when a
powerful board is detected.
It also reduces a step in the documentation.
My first PR to an Arduino project so let me know if I'm doing something
wrong! 😅 🙏
I suppose this would mean changing the version of the library that is bundled with the
project (
PubSubClient_ID89
?) as well, but I'm not sure how to go about doing so...? 🙇