search

PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.21k stars 13.38k forks source link

Cross check all RC mappings and signs #917

Closed LorenzMeier closed 10 years ago

LorenzMeier commented 10 years ago

We should check that all sign changes and mapping changes are indeed correct - I did so multiple times, but I want to raise general awareness @thomasgubler @DrTon .

LorenzMeier commented 10 years ago

@TickTock- Just found the first mismapping here:

if ((r_status_flags & PX4IO_P_STATUS_FLAGS_RC_OK) && (REG_TO_SIGNED(r_rc_values[4]) < RC_CHANNEL_LOW_THRESH))
            override = true;

This will be fine as long as one is not using single channel mapping. This line: https://github.com/PX4/Firmware/blob/master/src/modules/px4iofirmware/controls.c#L370

Needs to correspond to the logic here: https://github.com/PX4/Firmware/blob/master/src/modules/sensors/sensors.cpp#L1480

TickTock- commented 10 years ago

The hack is to just change the threshold to the lower 10% of the swing which should work for everyone as long as they keep MANUAL at the lowest setting (which they should) but I will work on a cleaner mechanism to pass the programmed thresholds. Testing this is tricky - I guess I need to temporarily disable the sensor.cpp code to ensure the px4iofirmware is doing it's thing. Or is there an establish debug hook?

LorenzMeier commented 10 years ago

@TickTock- This suggestion doesn't seem terrible. Note that I just realised that the px4io status command should output OVERRIDE if the unit is in override mode. To allow it to be, you need to configure it as fixed wing. It won't do this on multicopters.

LorenzMeier commented 10 years ago

@TickTock- If you don't mind I would appreciate a quick PR with that change. Its ok if the "right" fix takes longer, but I would sleep better if an intermediate safe fix is in (even if the default setup is still not affected right now).

TickTock- commented 10 years ago

I can, but I need to explore the code some more. I am not sure what the range of "REG_TO_SIGNED(r_rc_values[4])" is. Does full range go from -32768 to 32767? or some subset. I don't want to risk breaking it. I am away from my bench but will look into it and test it a little later today.

LorenzMeier commented 10 years ago

That particular call is just a call, so the value will remain unchanged. The convention is -10'000..+10'000 - I guess we should write this up in the wiki somewhere.