ardupilot
ardupilot copied to clipboard
ArduPilot
SRV_Channel: Passthrough mapped RC channels
The intention of this is to create new servo functions that map the RC input to the servo min/trim/max. I think the function I am needing already exists to map between the 2. Just getting lost in calls right now. I'll take another read through the code to figure it out.
I think a bitmask to determine if the aux function 51-66 is a direct passthru like now, or a output scaled using the output min/max/trim, would be a better approach than new functions
Maybe I'm missing something, but doesn't setting the SERVOn_FUNCTION to an RCINxx do this? I take that back.
If I understand this correctly - with this change, you will now have the ability to set SERVOn_FUNCTiON to RCINxx_MAPPED for any given channel, where mapped will scale the RCIN to the SERVOn min/trim/max. If that's what it's doing then it's an excellent idea and definitely better than using a bitmask.
unneeded complexity....unneeded use of aux function places....a bit mask that designates if the passthru is scaled or not (current code) is all that is needed....you dont need to have any RC input assigned to any servo output...you can always arrange the ins and outs as needed currently, the only issue is that passthru does not scaling currently....
unneeded complexity....unneeded use of aux function places....a bit mask that designates if the passthru is scaled or not (current code) is all that is needed....you dont need to have any RC input assigned to any servo output...you can always arrange the ins and outs as needed currently, the only issue is that passthru does not scaling currently....
From this users perspective, it seems much simpler to just pick the RCINxx or RCINxx_MAPPED vs having to track down a separate parameter and figure out how to set it to match the SERVOn that I'm using. Are you talking code complexity? Because I'd suggest that the UX is much more complex with the bit mask.
yes code..we are removing functions weekly from the F4s (and some F7s)....code space for generic functions is precious....and using up limited aux functions....only 100 or so out of 255 are available for any future adds....and this can be accomplished without that while making it a tiny bit more user friendly (although confusion between current passthru and this would STILL require reading the wiki to understand the differences) , its low bang per buck in my opinion....
This approach is working on a Cube Orange now. Moved the RC min and max values around to test that the values were actually being mapped correctly.
Other approaches mentioned would probably be better than this one. I'll shelve the 16 new servo function approach in a different branch and try the bitmask approach.
EDIT: Branch with that method is stored here https://github.com/TunaLobster/ardupilot/tree/wip/rc-passthrough-srv-functions
Bitmask method is most of the way there. I need to figure out if the bitmask needs to be sent to the IOMCU. Current code problem is the bitmask parameter is not showing up in the GCS. I didn't think I was doing anything wildly different than the GPIO mask parameter.
EDIT: Bitmask method is stashed in this branch https://github.com/TunaLobster/ardupilot/tree/wip/rc-passthrough-mapped-bitmask-with-iomcu-failover
I prefer the new aux functions, the param is a int16 so were not going to run out. There maybe some transport cases where were using uint8, but were still a long way from 255 and we can fix that when we hit it. Better to only have one param, the servo function rather than to have to also go and set a bitmask.
I prefer the new aux functions, the param is a
int16so were not going to run out. There maybe some transport cases where were usinguint8, but were still a long way from 255 and we can fix that when we hit it. Better to only have one param, the servo function rather than to have to also go and set a bitmask.
I'd strongly agree, and it's also just much more obvious what is happening as a user that way. It also helps limit the number of parameters we have to track and manage, which is valuable in it's own right.
Branch is back to servo aux functions. Cleaned up and added the parameter metadata. This is going to need a wiki with a decent figure to help cover all the bases of information types. Marking for devcall as is and we'll talk about it then.
I would vote to not bother with the IOMCU mixer stuff at all. Its increasingly unlikely that that functionality will ever save a vehicle.
From someone who is still reasonably new to this, I have generally understood that the message is "don't do anything in your transmitter - let ArduPilot manage everything". Not having this scaling makes passthroughs an exception - "except for passthroughs, for passthroughs you need to set trims and weights and offsets in your transmitter". Is it a good idea to have an exception?
As discussed on the dev call I agree with @WickedShell (and others) that just adding to the Enum (as is done in this PR) is a better approach than adding a new parameter or SERVOx_OPTION bit.
Tested reversal, dead zone dramatically high, dead zone at zero, and trims at min. Log of the test is attached. Got to do a rebase with drops, fixup, and reword all at the same time!
Some unexpected size changes, annoyingly for stuff that we don't save the elf diff for.
Binary Name Text [B] Data [B] BSS (B) Total Flash Change [B] (%) Flash Free After PR (B)
--------------- -------------- ----------- ------------- ---------------------------- -------------------------
antennatracker 220 (+0.0293%) 0 (0.0000%) 4 (+0.0020%) 220 (+0.0293%) 280088
arducopter-heli 76 (+0.0075%) 0 (0.0000%) 4 (+0.0020%) 76 (+0.0075%) 18588
ardurover 76 (+0.0083%) 0 (0.0000%) -4 (-0.0020%) 76 (+0.0083%) 112532
arducopter 76 (+0.0076%) 0 (0.0000%) 4 (+0.0020%) 76 (+0.0076%) 30612
ardusub 220 (+0.0239%) 0 (0.0000%) 4 (+0.0020%) 220 (+0.0239%) 111184
blimp 220 (+0.0311%) 0 (0.0000%) -4 (-0.0020%) 220 (+0.0310%) 322572
arduplane 76 (+0.0074%) 0 (0.0000%) -4 (-0.0020%) 76 (+0.0074%) 268
I think I figured it out. The symbol for RC_Channel::norm_input_dz() const appeared in AntennaTracker and is 144 bytes. The other vehicles seem to already be using that method. So that accounts for the 144 byte difference. The board to board difference between vehicles is the same size but I don't know why that method was not already used.