Large diagnostic response payloads aren't sent properly in JSON

[zacnelson]

Initially created an issue in openxc-android here. Turns out I was able to recreate the same issue with the Python command line tools, so now I think it's an issue in the firmware.

In IsoTp and UDS-C we allow a max diagnostic payload size of 255 bytes. When trying to send many bytes through in the diagnostic response, part of the JSON string is overwritten by other information. Like this:

{"bus":1,"id":2016,"mode":17,"success":true,"pid":2,"payload":"0x030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f60616{"command_response":"diagnostic_request","status":true}

In this example case, the payload should have contained 128 bytes (256 ascii characters when converted to JSON string).

[peplin]

I'll bet it's hitting the MAX_OUTGOING_PAYLOAD_SIZE:

#define MAX_OUTGOING_PAYLOAD_SIZE 256

in pipeline.h, when calling serialize, it passes in a buffer of this size as the maximum allowed serialization output.

[peplin]

The payload in your example stop exactly at 256 bytes - I'll bet the rest is garbage from other regions of memory that we unintentionally read because the payload is not null-terminated.

[zacnelson]

Yeah, I was looking at that earlier. I did increase that size which helps. But also needed to increase the size of the Queue here:

vi-firmware/src/util/bytebuffer.h

Line 7 in 0ee0b5a

QUEUE_DECLARE(uint8_t, 320)

[zacnelson]

The queue availability is checked here: https://github.com/openxc/vi-firmware/blob/master/src/pipeline.cpp#L46. It's a bit tough to debug though because adding any debug statements cause a recursive function call to sendMessage. Figured that out the hard way...

[peplin]

Alternatively, you could fit a lot more data in the string if it uses base64 encoding instead of hex, but that would be a breaking API change.

[zacnelson]

Alright, so after further investigation and attempts to get 255 bytes of payload response out over JSON, I don't think we can do it... I tried increasing the queue size to 700 and the MAX_OUTGOING_PAYLOAD_SIZE but now I get an OOM error when trying to create the JSON object for the payload. Looks like going to the base64 string encoding type might be the only option. OR we could send out a large diagnostic payload response in multiple messages...

[zacnelson]

Pull request (#374) has been merged to limit the diagnostic payload response size to 127 bytes so this error shouldn't happen. Changed this from a bug fix to a feature enhancement for future development