Simpit-team / KerbalSimpitRevamped

A revamp of the Kerbal Simpit Mod for KSP
BSD 2-Clause "Simplified" License
26 stars 15 forks source link

[Feature]: Rewrite serial protocol: COBS encoding, smaller packets. #65

Closed phardy closed 2 years ago

phardy commented 3 years ago

Is your feature request related to a problem? Please describe.

No.

Describe the solution you'd like

A clear and concise description of what you want to happen.

The last feature that I started working on before dropping this project entirely was a total rewrite of the serial protocol. The intention was to make packets smaller, with lightweight error detection.

Right now, a data packet is serialised as-is, with a four byte header. A typical packet looks like this:

leader byte 1 | leader byte 2 | msgSize   | msgType | payload
 0xAA         |  0x50         |  0-32     |  0-255  | Any valid data
 Alternating  | High is 0101  | higher is |         |
 0 and 1 bits | Low is version| an error  |         |

I'd like to suggest, instead of pushing data across the line as-is, the plugin switches to encoding data with COBS. COBS provides unambiguous framing, and can be done in a stream in the arduino library with trivial overhead. Using COBS encoding, I'd propose a packet that looks like:

msgType | payload        | checksum
 1 byte | Any valid data | 1 byte XOR of entire packet

COBS encoding adds at most a single byte to a packet. But it saves having to transmit those leader bytes of useless data with every packet. And replacing the msgSize in the header with an XOR checksum in the last byte of the packet helps streamline packet serialisation for tiny 8 bit uCs.

Along with a new protocol, I'd like to suggest these changes to the handshaking:

How would it be best that the feature is implemented? What sort of data may need to be sent by the mod to controllers? Would it require a digital signal, or an analogue one? Maybe it needs a terminal command to help use it, or a GUI button?

Changing the encoding without hurting users of older Arduino libraries too much would probably involve some code in the plugin that can detect version 1 headers, and appropriate error logging.

PBechon commented 3 years ago

This seems like a good feature to add. In this case the idea is to encode each packet as an independent COBS packet right ? Do you think it could be useful to bundle several packets together if they are all to be sent together ?

I also agree with you that we need to handle the case of an old Arduino lib that does not encode its packet for backward compatibility.

PBechon commented 3 years ago

Some links that could prove useful.

Wikipedia has a C implementation of it. Alternatively, there is Arduino libs that already support COBS.

On C# side, there seem to be also library available but licensing has to be checked : https://github.com/rockthethird/Consistent-Overhead-Byte-Stuffing

PBechon commented 2 years ago

Feature added in the commit fe99f92d8dbce48fe9dd4b3452930a38c3a1be88