RS485 Data Dropping

[maciekish]

trafficstars

RS485 Data Dropping

This PR addresses the data dropping issues encountered when RS485 slaves are too busy to process incoming data.

The problem

RS485 slaves drop incoming export data if the user sketch runs too slow. It happens when drawing to a slow display, but also in cases where it should work a lot faster, such as writing to a Nextion display (UART2, 250000 baud). Additionally, if you map 48LEDs on an Arduino Mega, even with direct register writes, it triggers the same issue.

Videos

These examples run on the exact same hardware, code and data. The only difference are the #defines for USB or RS485 mode. I have made a dev-board for tracking down this particular issue, and it includes the same amount of LED as the A-10C Caution Panel, and a Nextion header for easy testing.

Before

Expand

https://github.com/DCS-Skunkworks/dcs-bios-arduino-library/assets/442007/aa52dc19-75ec-43b5-9923-50d1c205e953

https://github.com/DCS-Skunkworks/dcs-bios-arduino-library/assets/442007/8b70e117-7645-43d3-9224-e8cfed37ffae

After

Expand

https://github.com/DCS-Skunkworks/dcs-bios-arduino-library/assets/442007/da5d1ab8-925a-4ef8-aaff-95fb7868c803

https://github.com/DCS-Skunkworks/dcs-bios-arduino-library/assets/442007/a93be506-1f3b-4d3d-a5da-2a97d581068e

Changes

I have done extensive testing and found the following changes fix the issue. Each change improves the situation, and together, RS485 slaves behave like USB devices.

1. Process incoming data outside of the ISR. processCharISR() has been modified to only store the received byte if DCSBIOS_RS485_SLAVE is defined. processChar() has been moved to loop().
2. unit8_t availableForWrite() added to RingBuffer.
3. When receiving RS485 data, the incomingDataBuffer is checked for space to avoid a buffer overflow.
4. The size of incomingDataBuffer is user-configurable with #define DCSBIOS_INCOMING_DATA_BUFFER_SIZE 512.

Rationale

1. Currently, all RS485 data processing happens in ISR context, in processCharISR(). This function directly calls out to individual ESLs, which in turn run all user code in any DCS-BIOS callbacks. In practice, pretty much the whole sketch runs in ISR context, with nested interrupts. There is barely any prioritization because everything runs from the ISR. So when new data arrives, it has to contest for resources. While it may seem like calling processChar() is important, the full picture must be considered. If processChar() takes a bit longer because it´s now called in loop() and may be interrupted, the only downside is that the external update (LED, display, servo etc) will happen slightly later. But if processCharISR() doesn't run fast enough, we start dropping data randomly, sync is invalidated because we don't know what, or how much was dropped, and the whole incomingDataBuffer becomes a mess.
2. If the user sketch is too slow, data will have to be dropped somewhere. With the current code, this happens inside the incomingDataBuffer which is a RingBuffer. What this means in practice, is that if an ESL is busy reading data from the RingBuffer, and an interrupt causes a write longer than available space, the new data will overwrite old data that hasn't been processed yet. This causes all sorts of weird issues, like the sync sequence UUUU appearing as text on displays, or the wrong LEDs lighting up/extinguishing on a caution panel. So, if the buffer cannot fit the new data, it has to be discarded.
3. Same as above.
4. The default size of 64 bytes may be enough for a couple LEDs, but when attempting to draw a CDU page, the amount of data is 10 lines x 24 characters = 240 bytes of raw data. On top of that, space for the address and sync sequence etc must be considered. With a 512 byte buffer and controlling a Nextion display over UART2 with HardwareSerial, only 20% RAM on an Arduino Mega is used. If not defined by the user, the default 64b buffer is retained.

User impact

All changes only affect AVR MCUs in RS485 mode. USB mode is not affected. To enable the new processing mode, use DCSBIOS_DEFER_RS485_PROCESSING:

#define DCSBIOS_DEFER_RS485_PROCESSING

[maciekish]

Great job diagnosing this. I like your approach.

The only thing I'd ask it that your excellent PR description be made available to all for after the PR closes. You can decide where. Could be in readme. Could be on the wiki. I often find it best to make a sketch in the example and comment it well.

Hi, sorry for taking forever. I was cleaning up my branches and realized this was never merged. I have pushed an example called RS485Deferred. Please let me know if it looks ok.

How do i make changes to the wiki? Can't find any fork/edit function there.

[charliefoxtwo]

@maciekish to make changes to the wiki you should just be able to click the edit or new page buttons on any page in the top left. I've just opened up edit access to the wiki - let me know if you have any issues

[maciekish]

@maciekish to make changes to the wiki you should just be able to click the edit or new page buttons on any page in the top left. I've just opened up edit access to the wiki - let me know if you have any issues

I have added a page explaining the alternate processing mode but haven't touched the menu because i'm not sure how you want to organize it as there is no RS485 page or menu. https://github.com/DCS-Skunkworks/dcs-bios-arduino-library/wiki/RS485-Processing-Modes.

There is also an example i added before: https://github.com/DCSBIOSKit/dcs-bios-arduino-library/blob/rs485/examples/RS485Deferred/RS485Deferred.ino

Please let me know how you would like to proceed

[charliefoxtwo]

I have no strong opinions about the wiki structure (I'm not too involved in this project) so I'd say take whatever action you see fit. It's a community wiki and things can always be moved in the future - the important part is just getting the data there in the first place