Improvement of RC_Channel::set_control_in()

[kd0aij]

The existence and use of RC_Channel::set_control_in() is a potential source of confusion and bugs.

It is not possible to eliminate set_control_in without providing the equivalent functionality elsewhere. In general, it provides a mixing function mapping radio inputs to control inputs. Copter's "supersimple mode" rotates the roll and pitch axes so that the pitch axis always points "away from home" by using set_control_in to alter the control_in values for the roll and pitch radio inputs. tailsitter_check_input rotates the roll and yaw axes by 90 degrees around the body-frame pitch axis if "plane" type is selected. The current implementation with set_control_in() provides this capability by changing control_in values in place, with the downside being that these alterations to radio inputs are not clearly identified or logged.

One way to improve this would be to explicitly define a mixer to map radio_in to control_in values for every channel. This would change the update function to first get radio_in for all channels, then run a dynamic mixing function to generate control_in for all channels. In this scenario, set_control_in() would alter the mixing function instead of being invoked on a per-channel basis. We would log the control_in values in addition to (or instead of) radio_in values.

The remaining ambiguity is control_in vs. norm_in: these values differ in behaviour for TYPE and RANGE channels; unfortunately, channel type is not visible to users and varies on a per-vehicle basis.

Platform [ x ] All

[rmackay9]

@kd0aij,

I'm happy with us changing how Copter's simple mode works so that it doesn't overwrite the values in RC_Channel. I've always thought this is bad behaviour. We could very easily modify copter so it doesn't do this and instead just makes a local copy.

[kd0aij]

If you make a local copy of control_in somewhere, would you no longer use RC_Channel::get_control_in() when in Simple Mode? I guess the logged radio_in values would remain consistent with the actual vehicle behaviour, taking into account the current Mode. But if someone added code that used control_in or norm_in from RC_Channel, they might be surprised at the results. That was the cause of a recent bug in the tailsitter code.

[rmackay9]

@kd0aij,

I think we would still use RC_Channel::get_control_in() but then create local variables with the results rotated around. This would all be done in the Copter. I think it is bad behaviour at the moment that the RC values logged do now match the pilot's actual physical inputs.

[kd0aij]

This is what adding an RC input mixer might look like: https://github.com/kd0aij/ardupilot/pull/52 Probably overkill given that there are only 2 places where it is currently needed.

[rmackay9]

@kd0aij, ok.. looks fancy. As you say it is perhaps a bit of overkill.. at least for copter we could just have a local method to rotate it which might be easier for future developers to understand.