Atomic channel data updates#3363
Conversation
We do this because the generated code for serial output may access each channel multiple times and the global buffer may be modified in-between accesses
547658e to
2d163ce
Compare
|
I've run into another problem with the direct ChannelData access used everywhere When the data comes in and gets loaded into ChannelData If I just hack into the source that the I'm testing with the headtrack data being fed in from the main tx_main loop to take the backpack out of the equation, putting the headtrack data in CH0 so it is sent every packet, and then just having the RX print a debug line if CH0 isn't the headtrack value. At 500Hz or 100Full I very rarely see one but at 50Hz I see one maybe every 10 seconds on average. This needs to be addressed as well, but the maybe bigger issue is why the TOCK is going off right as the channel data is coming in from the handset. The sync is usually ~100us before transmit time but it seems there's enough jitter in either the handset or us that we're managing to transmit right as the data is arriving so our actual end-to-end latency may be fairly poor if our ETX sync is so bad. |
|
I've added 29fe04f which has a new callback in Handset to allow overrides of the channel values before they are published. PPM and CRSF handsets are updated to operate on a local copy of the channels, which are then overridden, and then the RCDataReceived() method is called to "publish" them. Also note that PPM handset would update the arm flag before publishing and CRSF would update the arm flag after publishing, so I've made this consistent so they both update the arm flag then publish. I'm not sure which way is the most correct (perhaps the other way?) but it should be one or the other. The changes to devBackpack.cpp are pretty rough, but this is rewritten in #3373 so I didn't want to spend much time on it only to cut it back out again after this is merged. Let me know if you'd prefer I revert my changes and submit them another way. I didn't want to have to paste such a large diff here. |
All good. I'm ok with this since it's effectively temporary till #3373 is merged. |
CapnBry
left a comment
There was a problem hiding this comment.
Code looks good and tested on Nomad TX / BAYCK Dual RX at 1000Hz (both dual core MCU). I don't have an SBUS tester I trust enough to see if one frame has a corrupted value, but for sure the old code had possible concurrency problems.
Investigation
Investigating #3347, in Full resolution modes the channel data each channel is updated atomically as they are 32bit values aligned in an array. There is a multi-update problem with hybrid mode, which the PR addresses (see 1 below).
The code that takes the channels and converts them to SBUS output operates on each channel individually and each channel is read only one time in the code that builds the SBUS frame. So each channel is read atomically. The SBUS frame is only sent to the serial port once it is fully built.
Unless of course the compiler is generating multiple reads on the channelData array while forming the SBUS frame! Which is what it's doing.
Solution(s)
ChannelDataand pass the pointer to the local copy down to the serial protocol drivers which may/do access each channel slot multiple times.Fixes SBUS Periodic glitch every15-20 seconds #3347