ZZ-Cat / CRSFforArduino

An Arduino Library for communicating with ExpressLRS and TBS Crossfire receivers.
GNU Affero General Public License v3.0
162 stars 27 forks source link

refactor(rc channels): Use packed RC channels #30

Closed ZZ-Cat closed 1 year ago

ZZ-Cat commented 1 year ago

Overview

This Pull Request re-factors the way RC channels data is handled.

What's changed

The Old Way

In the CRSFforArduino::update() function, this is how I used to unpack RC channels data...

// Read the RC channels.
// 11 bits per channel, 16 channels = 176 bits = 22 bytes.
_channels[0] = (uint16_t)((_crsfFrame.frame.payload[0] | _crsfFrame.frame.payload[1] << 8) & 0x07FF);
_channels[1] = (uint16_t)((_crsfFrame.frame.payload[1] >> 3 | _crsfFrame.frame.payload[2] << 5) & 0x07FF);
_channels[2] = (uint16_t)((_crsfFrame.frame.payload[2] >> 6 | _crsfFrame.frame.payload[3] << 2 | _crsfFrame.frame.payload[4] << 10) & 0x07FF);
_channels[3] = (uint16_t)((_crsfFrame.frame.payload[4] >> 1 | _crsfFrame.frame.payload[5] << 7) & 0x07FF);
_channels[4] = (uint16_t)((_crsfFrame.frame.payload[5] >> 4 | _crsfFrame.frame.payload[6] << 4) & 0x07FF);
_channels[5] = (uint16_t)((_crsfFrame.frame.payload[6] >> 7 | _crsfFrame.frame.payload[7] << 1 | _crsfFrame.frame.payload[8] << 9) & 0x07FF);
_channels[6] = (uint16_t)((_crsfFrame.frame.payload[8] >> 2 | _crsfFrame.frame.payload[9] << 6) & 0x07FF);
_channels[7] = (uint16_t)((_crsfFrame.frame.payload[9] >> 5 | _crsfFrame.frame.payload[10] << 3) & 0x07FF);
_channels[8] = (uint16_t)((_crsfFrame.frame.payload[11] | _crsfFrame.frame.payload[12] << 8) & 0x07FF);
_channels[9] = (uint16_t)((_crsfFrame.frame.payload[12] >> 3 | _crsfFrame.frame.payload[13] << 5) & 0x07FF);
_channels[10] = (uint16_t)((_crsfFrame.frame.payload[13] >> 6 | _crsfFrame.frame.payload[14] << 2 | _crsfFrame.frame.payload[15] << 10) & 0x07FF);
_channels[11] = (uint16_t)((_crsfFrame.frame.payload[15] >> 1 | _crsfFrame.frame.payload[16] << 7) & 0x07FF);
_channels[12] = (uint16_t)((_crsfFrame.frame.payload[16] >> 4 | _crsfFrame.frame.payload[17] << 4) & 0x07FF);
_channels[13] = (uint16_t)((_crsfFrame.frame.payload[17] >> 7 | _crsfFrame.frame.payload[18] << 1 | _crsfFrame.frame.payload[19] << 9) & 0x07FF);
_channels[14] = (uint16_t)((_crsfFrame.frame.payload[19] >> 2 | _crsfFrame.frame.payload[20] << 6) & 0x07FF);
_channels[15] = (uint16_t)((_crsfFrame.frame.payload[20] >> 5 | _crsfFrame.frame.payload[21] << 3) & 0x07FF);

This is a very inefficient way of doing it, plus visually speaking, it looks confusing af.

The New Way

In the CRSFforArduino::update() function, this is how I roll, now...

// Read the RC channels.
memcpy(&_crsfRcChannelsPackedFrame, &_crsfFrame, CRSF_FRAME_SIZE_MAX);

Notice how it's much more concise? Programmatically, it is more efficient because all I am doing here is double-buffering the incoming RC channel data. I have also moved the RC channel data unpacking out of the CRSFforArduino::update() function & put it into the CRSFforArduino::getChannel() function.

const __crsf_rcChannelsPacked_t *rcChannelsPacked = (__crsf_rcChannelsPacked_t *)&_crsfRcChannelsPackedFrame.frame.payload;

// Unpack the RC channels.
_channels[RC_CHANNEL_ROLL] = rcChannelsPacked->channel0;
_channels[RC_CHANNEL_PITCH] = rcChannelsPacked->channel1;
_channels[RC_CHANNEL_THROTTLE] = rcChannelsPacked->channel2;
_channels[RC_CHANNEL_YAW] = rcChannelsPacked->channel3;
_channels[RC_CHANNEL_AUX1] = rcChannelsPacked->channel4;
_channels[RC_CHANNEL_AUX2] = rcChannelsPacked->channel5;
_channels[RC_CHANNEL_AUX3] = rcChannelsPacked->channel6;
_channels[RC_CHANNEL_AUX4] = rcChannelsPacked->channel7;
_channels[RC_CHANNEL_AUX5] = rcChannelsPacked->channel8;
_channels[RC_CHANNEL_AUX6] = rcChannelsPacked->channel9;
_channels[RC_CHANNEL_AUX7] = rcChannelsPacked->channel10;
_channels[RC_CHANNEL_AUX8] = rcChannelsPacked->channel11;
_channels[RC_CHANNEL_AUX9] = rcChannelsPacked->channel12;
_channels[RC_CHANNEL_AUX10] = rcChannelsPacked->channel13;
_channels[RC_CHANNEL_AUX11] = rcChannelsPacked->channel14;
_channels[RC_CHANNEL_AUX12] = rcChannelsPacked->channel15;

Here you can see how straightforward this looks. I know what RC channel is going where.

Additional

I have removed the caveat regarding the SERCOM UART data order. Since using packed RC channels, there is no need for me to the low-level hardware UART driver to set its data order to Most Significant Bit First. This opens up the door for more hardware compatibility.

In the readme I have also clarified what CRSF for Arduino's minimum requirements are, & what you can do if your devboard meets the minimum requirements & yet, you're still having compatibility issues.