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

Check roll direction for VTOL #4797

Closed
LorenzMeier opened this issue Jun 11, 2016 · 19 comments · Fixed by #11382
Closed

Check roll direction for VTOL #4797

LorenzMeier opened this issue Jun 11, 2016 · 19 comments · Fixed by #11382
Labels
Hybrid VTOL 🛩️🚁 Multirotor + Fixedwing!

Comments

@LorenzMeier
Copy link
Member

LorenzMeier commented Jun 11, 2016

I have the odd symptom that the ailerons signs are inverted for VTOL in SITL.

@LorenzMeier
Copy link
Member Author

Will fix this in v1.5

@LorenzMeier LorenzMeier modified the milestones: Release v1.5.0, Release v1.4.0 Jul 29, 2016
@dagar dagar added the Hybrid VTOL 🛩️🚁 Multirotor + Fixedwing! label Nov 8, 2016
@LorenzMeier LorenzMeier modified the milestones: Release v1.6.0, Release v1.5.0 Nov 19, 2016
@dagar
Copy link
Member

dagar commented Feb 18, 2017

@LorenzMeier
Copy link
Member Author

I think we have no chance but to bundle this into a major release with big red warnings.

@sanderux
Copy link
Member

sanderux commented Apr 2, 2017

Following up here on the inversion issue of the roll on vtol's:
https://github.com/PX4/Firmware/blob/master/src/modules/vtol_att_control/standard.cpp#L456

@dagar commented:

This could be a good excuse to unify the VTOL mixers with regular FW & MC. Some of them are special, but many could just reuse a regular MC mixer on MAIN, regular FW mixer on AUX.

As far as the DeltaQuads go i would actually prefer using only MAIN outputs
This will make the setup compatible with FC's that do not provide an AUX rail

@dagar
Copy link
Member

dagar commented Apr 2, 2017

Nevermind, I just realized I don't know what I'm talking about. The current VTOL FW mixers are control group 1.

@sanderux sanderux assigned dagar and unassigned RomanBapst Apr 11, 2017
@mhkabir
Copy link
Member

mhkabir commented Apr 23, 2017

@bresch Can you please have a look at this.

@LorenzMeier
Copy link
Member Author

Changing this for the next release as a breaking change - not for this one.

@LorenzMeier LorenzMeier modified the milestones: Release v2.1.0, Release v1.6.0 Apr 23, 2017
@sanderux sanderux added devcall and removed devcall labels Apr 23, 2017
@LorenzMeier
Copy link
Member Author

Needs an API compatibility system that checks versions in params, compares it with the version in code and locks down arming if they mismatch or are smaller. Also should have a proper UI representation that includes instructions and a link to a more in-depth explanation on the user guide.

@mrpollo mrpollo removed this from the Release v1.7.0 milestone Oct 18, 2017
@bresch
Copy link
Member

bresch commented Feb 21, 2018

Still have to fix that. Maybe with the refactoring of VTOL @RomanBapst

@dagar
Copy link
Member

dagar commented Feb 21, 2018

@RomanBapst we should definitely fix this.

@RomanBapst
Copy link
Contributor

@bkueng and I have decided to finally tackle this one.
Here is our suggestion so far:

  • we add a line of code into all mixer files supported by PX4 which can be parsed by the mixer, e.g.
    V: 2
  • if the mixer does not find this line it returns an error which can be handled accordingly by the system
  • handling of the error message could include refusing to arm and sending a message to QGC (this is where input from others would be appreciated!)

This approach makes sure that no user can fly with a custom mixer without adding the magic line and thereby understanding that he/she requires to update their custom mixer files.

Please provide feedback!

@dagar
Copy link
Member

dagar commented Feb 27, 2018

Yes I think treating it like adding simple schema versioning would help us solve this annoyance as well as handle other changes in the future.

The other idea I had was to introduce a series of interactive preflight check walkthroughs, but that would need a lot more thought (and effort). On a newly configured airframe or major firmware upgrade we could force the checks to be rerun (unless the user explicitly opted out).

@stale
Copy link

stale bot commented Jan 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@jlecoeur
Copy link
Contributor

jlecoeur commented Feb 1, 2019

This is still an issue.

It looks like the minus sign has been in the codebase since the beginning of VTOL in 2014 (887ed69#diff-cd9a46a1bbc9010e5e50fd282544a874R405) and has then been copied into standard_vtol, tailsitter and tiltrotor files. I think it is time to let it go.

Here is my suggestion:

  • In next version:

    • add a parameter TOFIX_VT_ROLL_SIGN, default value 1
    • in all existing VTOL airframes scripts, set this param to 0
    • in all existing VTOL mixers, invert roll
    • remove the minus signs :-)
    • at runtime, if the parameter is equal to 1, display a big red warning and refuse to arm (implement this in commander?), request the user to check aileron deflection and fix his mixer if necessary
  • In next next version:

    • remove parameter

ping @LorenzMeier @dagar @RomanBapst @bresch

@LorenzMeier
Copy link
Member Author

@RomanBapst I've assigned this to you for triage. My preference would be to fix it now before even more people fly VTOL and it will become impossible to change.

@jlecoeur
Copy link
Contributor

jlecoeur commented Feb 3, 2019

Thanks @LorenzMeier!

@RomanBapst I think that adding mixer versioning, as you suggest above, is not semantically correct here. Indeed the mixer format will be unchanged between mixer version V1 and V2.
That said, I would be happy with either your solution, or mine above, or another. Any solution will be better than the current statu quo.

@dagar
Copy link
Member

dagar commented Feb 3, 2019

@jlecoeur that sounds good if you want to throw that in as a VTOL parameter. The nice part about that solution is we can have it set appropriately for all the builtin airframes + mixers.

@jlecoeur
Copy link
Contributor

jlecoeur commented Feb 3, 2019

@dagar OK I can give it a go. I just need your help to know where to best implement the arm/preflight check.

@dagar dagar moved this from To Do to In progress in Release 1.11 Blockers Mar 9, 2019
Release 1.11 Blockers automation moved this from In progress to Done Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hybrid VTOL 🛩️🚁 Multirotor + Fixedwing!
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

10 participants