Skip to content
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

enable long message&topic data #5

Closed
wants to merge 2 commits into from
Closed

enable long message&topic data #5

wants to merge 2 commits into from

Conversation

Sild
Copy link

@Sild Sild commented Jul 8, 2015

Hi, I have written to you email a few time ago, and ask you about the limitation on topic/message size. Now I find the reason of this behavior and fix it. Please make a new release to make it available to install by ArduinoIDE.

@@ -318,12 +318,17 @@ void MQTT::Client<Network, Timer, a, b>::freeQoS2msgid(unsigned short id)
template<class Network, class Timer, int a, int b>
int MQTT::Client<Network, Timer, a, b>::sendPacket(int length, Timer& timer)
{

int MAX_WRITE_SIZE = 90; //Did not investigate the reason.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that could be constant, like:

#define MAX_WRITE_SIZE 90

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@256dpi
Copy link
Owner

256dpi commented Jul 9, 2015

Thanks for your contribution!

Have you tested the patch extensively? And with what Hardware?

@256dpi
Copy link
Owner

256dpi commented Jul 9, 2015

I like to prevent changes to the underlying library as that gets updated automatically by a script.

I just pulled the latest library and saw that they have been also working on a fix, for I guess the same problem: https://github.com/256dpi/arduino-mqtt/pull/6/files

Maybe you could take a look and check if that would also help in your situation?

@Sild
Copy link
Author

Sild commented Jul 9, 2015

I have tested my patch for my particular use case with arduinoUNO and wifi shield, however there is no reason to break something on other devices.

About following:

rc = ipstack.write(&sendbuf[sent], length - sent, timer.left_ms());

This is not enough to clear work with my device. If we try to write more then 90 symbols, ipstack.write() will return 0, so we get infinity loop ( while !timer.expired() ). I have tried this way and it's fail.

@256dpi
Copy link
Owner

256dpi commented Jul 9, 2015

I'll try to loop in @icraggs as he is the maintainer of the underlying library.

@icraggs: Is this really a bug in the embedded C client, or are we missing something here? I would appreciate it, if you could take a look at the code briefly: https://github.com/256dpi/arduino-mqtt/pull/5/files

@Sild
Copy link
Author

Sild commented Jul 10, 2015

Also I found out that if length is more then 128, we can not publish anything. Moreover, we do not call sendPacket function. Is it expected behavior? Serial output:

sending row: 'series e:zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz m:millis=329.11IIIIIIIIIIIIIIIIII' ...
length: 127
sended.
sending row: 'series e:zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz m:millis=329.11IIIIIIIIIIIIIIIIIII' ...
length: 128
sended.
sending row: 'series e:zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz m:millis=329.11IIIIIIIIIIIIIIIIIIII' ...
sended.
sending row: 'series e:zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz m:millis=329.11IIIIIIIIIIIIIIIIIIIII' ...

I just have added debug message in MQTTClient.h:

...
int MQTT::Client<Network, Timer, a, b>::sendPacket(int length, Timer& timer)
{    
int writeSize = 0;
Serial.print("length: ");
Serial.println(length);  
...
}

part of sketch code:

if(client.connected()) {
String data = "series e:" + entityID + " m:millis=" + (String)getData();
    while(true) {
        Serial.println("sending row: '" + data + "' ...");
        client.publish(pubTopic,data);
        Serial.println("sended.");
        data+="I";
        delay(5000);
    }

Full code to reproduce this bug you can find here

@icraggs
Copy link

icraggs commented Jul 10, 2015

Hello all. In MQTTClient.h there is a MQTT_BUFFER_SIZE constant, which defaults to 128. I did this to void dynamic memory allocation, which maximizes portability and makes the memory use more predictable. If you want to send or receive packets longer than 128, you need to increase the size of that constant.

@Sild
Copy link
Author

Sild commented Jul 10, 2015

Thanks for explanation. And what about ipstack.write() function? Why does it return 0 if I try to call with length > 90?

@256dpi
Copy link
Owner

256dpi commented Jul 11, 2015

@Sild I guess Ian is right. You need to increase the constants here directly: https://github.com/256dpi/arduino-mqtt/blob/master/src/MQTTClient.h#L5

Unfortunately you can't set it in your sketch as the libraries are compiled separately.

@256dpi 256dpi closed this Jul 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants