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

MAVLink: Generate correct RC channel count #11219

Merged
merged 3 commits into from
Jan 25, 2019
Merged

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jan 15, 2019

Introduction
According to the message definition https://mavlink.io/en/messages/common.html#RC_CHANNELS_OVERRIDE
channels 1-8 should be ignored when they contain the value UINT16_MAX and the MAVLink 2 extension channels 9-18 should be ignored if they are either UINT16_MAX or zero.

Problem
Currently, the channel count is always 18 which doesn't reflect this definition. Hence I'm implementing the definition.

Solution
We loop from the last to the first channel and if one of them doesn't satisfy the conditions to be ignored we set the channel count to that number. Hence if the message is full of UINT16_MAX's the channel count stays 0, if the message is full of zeroes the count will be 8 because of the different definition in the extension and everything in between gets checked if it's ignored.

Test data
We use this implementation for half a year now on the H520.

Additional Context
mavlink/mavlink#855
#10535

@MaEtUgR MaEtUgR self-assigned this Jan 15, 2019
@MaEtUgR MaEtUgR requested review from julianoes and potaito January 15, 2019 08:46
@MaEtUgR MaEtUgR changed the title Mavlink: Generate correct RC channel count MAVLink: Generate correct RC channel count Jan 15, 2019
potaito
potaito previously approved these changes Jan 15, 2019
Copy link
Contributor

@potaito potaito left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@LorenzMeier
Copy link
Member

CI fail:

+ px4-px4_sitl_default-v1.9.0-alpha-226-gcb1014e/px4/Tools/ecl_ekf/process_logdata_ekf.py .ros/log/2019-01-15/08_42_44.ulg
<type 'exceptions.IndexError'> (estimator_status): list index out of range
<type 'exceptions.IndexError'> (ekf2_innovations): list index out of range
Using test criteria loaded from /tmp/jenkins/workspace/L_tests_mavlink-rc-channel-count/px4-px4_sitl_default-v1.9.0-alpha-226-gcb1014e/px4/Tools/ecl_ekf/check_level_dict.csv
Traceback (most recent call last):
  File "px4-px4_sitl_default-v1.9.0-alpha-226-gcb1014e/px4/Tools/ecl_ekf/process_logdata_ekf.py", line 83, in <module>
    late_start_early_ending=not args.no_sensor_safety_margin)
  File "/tmp/jenkins/workspace/L_tests_mavlink-rc-channel-count/px4-px4_sitl_default-v1.9.0-alpha-226-gcb1014e/px4/Tools/ecl_ekf/analyse_logdata_ekf.py", line 35, in analyse_ekf
    innov_time = 1e-6 * ekf2_innovations['timestamp']
KeyError: 'timestamp'

@priseborough Do we need to extend the EKF check tooling?

@LorenzMeier
Copy link
Member

@MaEtUgR Could you please rebase? Thanks!

@MaEtUgR MaEtUgR force-pushed the mavlink-rc-channel-count branch from f42ef02 to 9854166 Compare January 24, 2019 04:11
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Jan 24, 2019

I rebased, addressed review comments and improved readability.

@@ -1862,6 +1861,22 @@ MavlinkReceiver::handle_message_rc_channels_override(mavlink_message_t *msg)
rc.values[16] = man.chan17_raw;
rc.values[17] = man.chan18_raw;

// check how many channels are valid
for (int i = 17; i >= 0; i--) {
Copy link
Member

Choose a reason for hiding this comment

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

I still don't care for the magic number, but I don't see anything from mavlink we can use directly.

@dagar dagar merged commit 504372f into master Jan 25, 2019
@dagar dagar deleted the mavlink-rc-channel-count branch January 25, 2019 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants