Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filled CRTP messages with mavlink packets to save space #9232

Closed
wants to merge 15 commits into from

Conversation

barzanisar
Copy link

@barzanisar barzanisar commented Apr 2, 2018

Instead of packing header, payload and checksum of a mavlink message in separate CRTP messages, I packed them into one to save space in buffers and send less bytes/second to the crazyradio. This helps decrease data traffic and hence data loss.

static bool init = 1;

if (init) {
_msg_to_send.header = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this block to the constructor (https://github.com/barzanisar/Firmware/blob/60e59ff357c1b111bbe261cb6671c7d8bd8bdd24/src/modules/syslink/syslink_bridge.cpp#L50), then you don't need the init flag here.

}

static int msg_rem = CRTP_MAX_DATA_SIZE -
1; //30 bytes data, so size of crtp message being sent will be = 30 bytes data + 1 byte header + 1 extra byte not filled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ 1 extra byte not filled
Is this really needed? If so, please explain why.

src/modules/syslink/syslink_bridge.cpp Outdated Show resolved Hide resolved
if (msg_rem == 0) {

if (_link->_writebuffer.force(&_msg_to_send, sizeof(crtp_message_t))) {
printf("write buffer overflow!!! \n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use: PX4_WARN("write buffer overflow")?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants