RespiraWorks / Ventilator

Fully-featured ICU ventilator design, optimized for manufacture using commonly available components and free to license. Repository tracks all mechanical, electrical and systems design, software, requirements and regulatory documentation.
Apache License 2.0
130 stars 38 forks source link

Implement framing and CRC check on data being transfered over serial #159

Closed Miceuz closed 2 years ago

Miceuz commented 4 years ago

Summary: Now data is not properly framed, frame markers and CRC have to be added.

Details:

Relevant links: https://stm32f4-discovery.net/2017/07/stm32-tutorial-efficiently-receive-uart-data-using-dma/

sglow commented 4 years ago

One method of framing binary communications that has worked well for me in the past is using one byte value as a termination byte which signifies the end of a message, and another byte value as an escape which can be used to allow those two special characters to be passed as part of the data.

For example, lets say the termination byte is 0x12 and the escape byte is 0x34. Those numbers are arbitrary, anything could be used.

To send a message with the following data: 0x00 0x01 0x02 0x03 you would send: 0x00 0x01 0x02 0x03 0x12

That's just the message with the termination byte appended.

To send something that includes special characters: 0x12 0x34 0x56 0x78 you would send: 0x34 0x12 0x34 0x34 0x56 0x78 0x12

The two bold 0x34 characters are stripped out of the message and allow the following character to pass unmolested.

Advantage of this method of framing is it's simplicity. You can keep track of the special characters in the receiving ISR and alert the higher level code when the message terminates.

sglow commented 4 years ago

Darn, I've been getting used to hitting ctrl enter in slack to avoid posting something half way through, and it turns out that in git hitting that will immediately post. That's unfortunate!

The disadvantage is that you will need to manually process each character in the ISR, you can't really use DMA effectively. I don't think that's an issue though because the STM32 is pretty fast and the receive data is coming in fairly slowly (86 us between characters at 115.2k), so this won't have a noticeable impact on system timing.

That's just framing of course, we should also add a CRC to the end of the message. That can either be calculated using a simple lookup table or by using the CRC hardware on the chip. Using a table lookup might be better because it avoids any possible contention with other parts of the system that also want to use the CRC hardware and it allows us to share the code between the STM32 and the UI computer. Either way is pretty dead simple though.

Miceuz commented 4 years ago

Yes, I have solved this issue once the same way also, https://github.com/greghaynes/Afproto/ has been suggested by Daniel - it does exactly that.

I'm looking at other serial protocols like dmx, modbus and linbus - all of them use break condition and idle line as termination characters. For example modbus has 3.5 character length idle state at the beginning of the package, linbus - 13 bits of break condition. Both of these are detectable via STM32 hardware. That could be an elegant solution - trigger DMA transfer on BREAK event and interrupt on IDLE to copy or process the data.

Miceuz commented 4 years ago

Regarding the CRC, I am sure we can find some code for UI side. No other part of the system uses CRC so far, so I guess we can grab it.

One benefit I see from using hardware CRC is that it allows us to have less code in the controller codebase. The same goes for Afproto vs break/idle.

Miceuz commented 4 years ago

Note: I did not check the errata of STM32L452RE, worth looking at before diving in.

sglow commented 4 years ago

I would caution against using timing for message framing. Certainly it can work, but you open yourself up to subtle problems in other parts of the system causing communication errors. For example, what if someone accidentally disables interrupts for longer then they should causing a random and infrequent delay in handling interrupts from the UART. Could that cause a false message termination? That sort of problem would be difficult to catch in testing.

sglow commented 4 years ago

I guess I'm just saying, keep it simple.

jkff commented 4 years ago

302 is also implementing a kind of framing. We should make sure that we don't end up with two framing protocols in two places - let's either use the one from #302 everywhere, or the one from #322 everywhere.