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

[autopilot] compiling generated AP for fixedwing #2055

Merged
merged 8 commits into from
May 6, 2017
Merged

Conversation

gautierhattenberger
Copy link
Member

Config file is updated to compile and is tested in simulation. At the moment, the same mode names are used and it may not behave correctly otherwise due to some hard-coded tests. Adding new modes might works thought.

One of the main changes is the modification of the AP mode name to match the rotorcraft firmware. It makes the code generation easier.

@podhrmic
Copy link
Member

@gautierhattenberger so does the generated code works the same way as the static autopilot? Or is this more like a partial PR with full functionality coming later?

Looks like most of the PR is about renaming the autopilot modes and some refactoring, which looks good.
Do you have some documentation about the generated code on wiki? I would like to see some docs/examples so it is easier to get started with the generated code.

@gautierhattenberger
Copy link
Member Author

I couldn't test it in real flight yet, but this description of FW autopilot should exactly match the behavior of the static version. Maybe the refactoring can be improved, but the general idea is here.
Also I have not done the wiki page describing this properly for both FW and rotorcraft. I'll do that when I have some time.
The comment about display only is about calling the navigation loop in order to move the carrot on the GCS even if not using it (MAN, AUTO1). It is very useful to know what the aircraft will do when switching to navigation mode.

@flixr
Copy link
Member

flixr commented Apr 19, 2017

I think parse_rc_mode in gen_settings.ml needs to be adjusted for the renamed PPRZ_MODE_x

Also some airframe files probably need to be updated for this change of name (when RC_LOST_MODE is defined).

Question is if we want to make backward compatible defines for that or announce it on the mailing list..

Otherwise it looks good to me, although I have no time to test right now...

@gautierhattenberger
Copy link
Member Author

I need a bit more time to finish tests with real aircraft, but it should be ready soon. Better do some more things even if it shouldn't break the standard configuration.

@podhrmic
Copy link
Member

@gautierhattenberger A few use-case comments:

If I add to my airframe config:

<autopilot name="fixedwing_autopilot.xml"/>

and

<configure name="USE_GENERATED_AUTOPILOT" value="FALSE"/>

the build will fail with:

In file included from autopilot.c:46:0:
/home/podhrad/Documents/Paparazzi/paparazzi/var/aircrafts/Minion_Lia/ap/generated/settings.h: In function 'settings_get_value':
/home/podhrad/Documents/Paparazzi/paparazzi/var/aircrafts/Minion_Lia/ap/generated/settings.h:307:20: error: 'autopilot_mode_ap' undeclared (first use in this function)
     case 0: return autopilot_mode_ap;

in both AP and NPS targets.

I have to have

<autopilot name="fixedwing_autopilot.xml"/>
    <configure name="USE_GENERATED_AUTOPILOT" value="TRUE"/>

but it is cumbersome because autopilot has to be specified under <airframe> tag, while configure is firmware specific so it is in <firmware> tag.

Tested with Minion Lia (fixedwing) and works fine in simulation, as well as in HITL.

It doesn't compile for rotocraft (tested with Ark_Quad under AggieAir conf), fails with:

/home/podhrad/Documents/Paparazzi/paparazzi/var/aircrafts/Ark_Quad/nps/generated/autopilot_core_ap.h:77:25: warning: the address of 'autopilot_in_flight' will always evaluate as 'true' [-Waddress]
         if (GpsIsLost() && autopilot_in_flight) { return AP_MODE_FAILSAFE; }

which I believe is something from the flight plan.

And again, I have to both specify

<autopilot name="rotorcraft_autopilot.xml"/>

and

    <configure name="USE_GENERATED_AUTOPILOT" value="TRUE"/>

Copy link
Member

@podhrmic podhrmic left a comment

Choose a reason for hiding this comment

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

See my comment above.

@gautierhattenberger
Copy link
Member Author

I fixed the rotorcraft autopilot who was not using correctly the new autopilot API.
Regarding the issue when setting the USE_GENERATED_AUTOPILOT to false, I haven't found a proper solution so far. The only way to fix this, is to disable the generation of ap settings if set to false, or use generated AP as soon as it is declared in the airframe file. I was planning to choose the second option for the final design, this was only a temporary stage for development. I just didn't expect the issue your pointing at, which makes it more or less useless...

@podhrmic
Copy link
Member

@gautierhattenberger just to clarify - the confusing case is that if I want to use the generated autopilot, I have to specify both <autopilot name="fixedwing_autopilot.xml"/> and <configure name="USE_GENERATED_AUTOPILOT" value="TRUE"/>

I would prefer if either I had to always define <autopilot name="fixedwing_autopilot.xml"/> and then toggle the generated/static autopilot with <configure name="USE_GENERATED_AUTOPILOT" value="TRUE/FALSE"/> or use only <autopilot name="fixedwing_autopilot.xml"/> and that tag by itself would mean using the generated ap. Indeed, it is not very descriptive by itself, maybe adding another attribute would help? Something like:

<autopilot name="fixedwing_autopilot.xml" type="generated"/>

or

<autopilot name="fixedwing_autopilot.xml" type="static"/>

where static would be the default option in case type attribute is not specified.

@podhrmic
Copy link
Member

As for rotorcraft autopilot- I tested it in NPS and for some reason the generated autopilot cannot get off the ground. I am using Ark_Quad A/C and more importantly https://github.com/paparazzi/paparazzi/blob/master/conf/flight_plans/rotorcraft_basic_geofence.xml flightplan.

I end up in take-off block, motors running but no throttle is applied (it stays at 0%), not sure what us causing it.

@gautierhattenberger
Copy link
Member Author

Now, the generated autopilot is used when an autopilot is defined in the airframe file.

The simulation for rotorcraft should work again as well.

@podhrmic
Copy link
Member

Indeed it works now!

One comment - I noticed that for rotorcraft I have to change AP_MODE_HOVER_Z_HOLD to AP_MODE_ATTITUDE_Z_HOLD in order to compile. That is probably intended as the autopilot mode names changed, but having some kind of warning (or just rename all the airframe files) would be nice.

Otherwise the only missing thing is the documentation. Do you want to merge as is and do the docs/wiki page later, or should we wait till it is a whole package?

@gautierhattenberger
Copy link
Member Author

The reason it fails is because your airframe file is most likely defining MODE_MANUAL, MODE_AUTO1 or MODE_AUTO2 to AP_MODE_HOVER_Z_HOLD, which is not defined in this autopilot xml file. Of course, with this system you only "build" the autopilot with the modes that you actually need, so it fails if you ask for a mode that doesn't exist. A "full" autopilot could be provided, with all the existing mode, so people can start without being totally lost, but that is not the end goal. The difficult part, is to deal with the hardcoded tests for some modes that are still here and there in the code.

Regarding the documentation, I would prefer to split the two things. I'm very busy so I won't have much time to do it soon, and when things last for too long, solving merge conflicts is endless.

@gautierhattenberger
Copy link
Member Author

Extra practical question: at the moment, the autopilot node is a the same level than firmware. Should it be part of firmware or even target like the module node so that you can have different ones for different firmware and/or targets within the same airframe (if that makes sense) ?
What do you think ?

@podhrmic
Copy link
Member

I think it makes most sense to have the autopilot selection as a part of firmware - so you can choose only one type of autopilot per firmware. Having lower granularity (e.g. one autopilot per target imho doesn't make sense). And having one autopilot per airfame probably is probably too much - because if you have a test firmware, it doesn't need to know about the autopilot you want to use.

But I am happy with it as is, so it is up to you.

The autopilot can be specific to one of the firmware, or one of the
target.
At the moment, it will fail at runtime if more than one autopilot in the
same airframe since server don't know which target is in use.
@gautierhattenberger
Copy link
Member Author

Finally, I made the autopilot node available at all level, but for now, only one per airframe is allowed, otherwise it will fail at runtime. Solving this is possible but needs some work and can be done later.

@podhrmic podhrmic merged commit 0a65e14 into master May 6, 2017
@podhrmic podhrmic deleted the autopilot_fw branch May 6, 2017 20:24
podhrmic pushed a commit that referenced this pull request May 7, 2017
* [autopilot] compiling generated AP for fixedwing

* update some old variable names for autopilot mode

* prevent oscillation on the ground with real aircraft

* add guidance loop to 'new' control

all this guidance code should really be better factorized

* fix rotorcraft autopilot for new API

* read guidance RC if needed

* use generated autopilot as soon as an autopilot file is defined

* autopilot node at airframe, firmware or target level

The autopilot can be specific to one of the firmware, or one of the
target.
At the moment, it will fail at runtime if more than one autopilot in the
same airframe since server don't know which target is in use.
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.

3 participants