Incorrect output, length error when using a String object

[DougieLawson]

I've got these objects defined in the prolog for my code:

/* DCF 77 signal and MQTT */

define DEBUG

String MQTTbuffer;
char MQTTtext[400];
boolean DCF77signal[62];

I then build the string to send to Mosquitto using this:

void displayTime() {

MQTTbuffer = "** ";
for (i = 0; i < 63; i++) {
MQTTbuffer += DCF77signal[i];
}
MQTTbuffer += " **\r\n";
MQTTbuffer +="TZ:";
MQTTbuffer +=BCDtoDecimal(17,2);
MQTTbuffer +="\tmm:";
MQTTbuffer +=BCDtoDecimal(21,7);
MQTTbuffer +="\thh:";
MQTTbuffer +=BCDtoDecimal(29,6);
MQTTbuffer +="\tday:";
MQTTbuffer +=BCDtoDecimal(36,6);
MQTTbuffer +="\tdw:";
MQTTbuffer +=BCDtoDecimal(42,3);
MQTTbuffer +="\tmth:";
MQTTbuffer +=BCDtoDecimal(45,5);
MQTTbuffer +="\tyy:";
MQTTbuffer +=BCDtoDecimal(50,8);
MQTTbuffer +="\tParity:";
MQTTbuffer +=BCDtoDecimal(60,3);
MQTTbuffer.toCharArray(MQTTtext,400);

if DEBUG

for (int i = 0; i < 400; i ++) {
Serial.print(MQTTtext[i],HEX);
}

endif

if (client.connect("arduinoClient")) {
client.publish("arduino/DCF77",MQTTtext);
client.disconnect();
}
if (client.connect("arduinoClient")) {
client.publish("arduino/DCF77","DONE");
client.disconnect();
}
}

That gets the following printed on the Serial console:

2A2A2030313030313130303031313130313030303130313031313030303131313030303030
3130303031303030303131313030303031303031303030303030303130202A2ADA545A3A
3196D6D3A3633968683A323096461793A34964773A3696D74683A31979793A399506172
6974793A32
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

So as far as I can tell the char array MQTTtext has been built correctly (until someone proves otherwise).

But the send to Mosquitto is rejected with a length error.

Looking at a tcpdump (with Wireshark) I get the following:

CONNECT
00000000 10 .
00000001 1b .
00000002 00 06 4d 51 49 73 64 70 03 02 00 0f 00 0d 61 72 ..MQIsdp ......ar
00000012 64 75 69 6e 6f 43 6c 69 65 6e 74 duinoCli ent

CONNACK
00000000 20 02 00 00 ...

PUBLISH
0000001D 30 0
0000001E 83 .
0000001F 00 0d 61 72 64 75 69 6e 6f 2f 44 43 46 37 37 2a ..arduin o/DCF77*
0000002F 2a 20 30 30 30 30 30 30 30 30 30 30 30 30 30 30 * 000000 00000000
0000003F 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 00000000 00000000
0000004F 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 00000000 00000000
0000005F 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 00000000 00000000
0000006F 30 20 2a 2a 0d 0a 54 5a 3a 30 09 6d 6d 3a 30 09 0 **..TZ :0.mm:0.
0000007F 68 68 3a 30 09 64 61 79 3a 30 09 64 77 3a 30 09 hh:0.day :0.dw:0.
0000008F 6d 74 68 3a 30 09 79 79 3a 30 09 50 61 72 69 74 mth:0.yy :0.Parit
0000009F 79 3a 30 y:0

DISCONNECT
000000A2 e0 .
000000A3 00 .

That x'E000' on the tail end of TCP stream is from the DISCONNECT, if I remove the client.disconnect() call it makes no difference. I still don't get my message delivered.

[knolleary]

Hi,

the client library has a maximum packet size of 128 bytes - which your code is exceeding. This isn't a scenario the code guards against (it does for incoming messages, but not for outgoing).

The length value is written to the header and then that number of bytes are copied to the internal payload buffer. If length exceeds the internal buffer, it carries on writing beyond the end of the array, which could cause memory corruption.

I'll put a fix in that guards against this, but in the meantime, if you want to increase the max packet size, change MAX_PACKET_SIZE in PubSubClient.h - this will obviously increase the memory usage of the code.

Nick

[DougieLawson]

Thanks Nick. I'll change the length to 140 (you can guess why).

[knolleary]

Actually, looking at it after a cup of tea, the write method only supports a single byte of length - which limits you to a 128 byte message. So you cannot simply change the #define.

When I get a chance later today, I'll put a fix in that enables multi-byte length encoding.

Once that is in place you'll be able to increase the message size.

You'll need to allow for the message headers when determining your message size. The sum is:
1 byte fixed header
2 bytes length (for messages 128 < length <16383 )
2 bytes for the topic length
topic bytes
payload bytes

which gives you 5+topic+payload bytes.

This assumes a qos 0 message (which is all the arduino lib currently supports), otherwise you need another 2 bytes for message ID in there.

[DougieLawson]

I've found that forked copy of your library. I'll take a look at that.

[knolleary]

Which forked copy? I haven't kept track of the handful of forks others have done, but I'm not aware of anyone making these changes already.

[DougieLawson]

https://github.com/karlp/pubsubclient

Or I'll put my S/370 (aka zSeries now) Assembler knowledge back in the cupboard, sharpen my C/C++ coding pencil and hack at your code.

I've been doing messaging and queuing for 30 years (along with lots of DL/I and DB2 databases), This MQTT stuff is like a little IMS thing. It's lots of fun. Look me up in BluePages.

[knolleary]

Reviewing the open issues. The library now handles larger packets (when the MAX packet size constant is changed in the .h file). Nothing more to do here.