Re: [Flightgear-devel] route manager crash

[James T. <ja...@fl...>]

> On 27 Jun 2018, at 07:41, Tobias Dammers <tda...@gm...> wrote:

> Hmm, right now it's all Nasal, and of course anyone who wants to should
> feel free to take it and whatever they want with it. Not sure if I want
> to rewrite it in C++ though; specifically, I'm not 100% sure whether it
> would work for other aircraft types.

Well, the generic autopilot is meant to be close enough to what most aircraft would do - it’s expected that for a particular aircraft you might do a custom solution. (I’d prefer people did that by adding extension hooks to the main AP logic, but I guess people prefer doing their own)
> 
> 
> Definitely needs some more tweaking though, and especially the hold code needs
> debugging, or maybe even an entirely different approach. The way it works now
> is that the autopilot can either follow a radial, defined as a point in space
> and a course, or it can just home in on a point in space. For a hold, it also
> tracks the entry mode (teardrop, parallel, direct) and the phase (which
> depends on the mode, but always starts with "approach" and ends in an
> "inbound" - "outbound" loop). It calculates a second virtual fix, right
> where the turn onto the inbound leg should be (from the holding fix, 1 minute
> worth of flying along the outbound radial, and then two standard-turn
> radii to the left or right, depending on the direction of the holding
> pattern), and then the inbound leg is just a matter of intercepting the
> (Fix, Radial) ray, and the outbound leg is intercepting (Opposite Fix,
> Opposite Radial).
> 
> The problem seems to be that turning this tightly doesn't leave much
> room for error, so when the turn overshoots due to wind, the autopilot
> has trouble correcting, and ends up circling the next fix.
> 
> So I was thinking maybe I should take a dumber approach, and just base
> the outbound leg at least on course and time alone - when reaching the
> fix, turn right until the course matches the opposite of the hold
> radial, then fly straight for 1 minute (or whatever time the hold
> specifies), then home in on the fix (or intercept the (Fix, Radial)
> ray). This requires a timer though, and it won't work for
> distance-defined holding patterns (then again, those can of course just
> measure distance from the fix).

Right, those kind of issues are why I was lazy and didn’t get as far as implementing hold support in the main autopilot code.

> Oh, and also I have patched fgfs to expose holding point data to Nasal,
> like so:
> 
> commit ca377c4ef297f0d6ed21f2a93a2bcbee3f2c12ca
> Author: Tobias Dammers <tda...@gm...>
> Date:   Thu Jun 21 18:29:49 2018 +0200
> 
>    Expose holding point data in nasal
> 
> diff --git a/src/Scripting/NasalPositioned.cxx b/src/Scripting/NasalPositioned.cxx
> index 22953bb2b..0d5fda303 100644
> --- a/src/Scripting/NasalPositioned.cxx
> +++ b/src/Scripting/NasalPositioned.cxx
> @@ -494,6 +494,42 @@ static const char* waypointCommonGetMember(naContext c, Waypt* wpt, const char*
>     *out = waypointAirport(c, wpt);
>   } else if (!strcmp(fieldName, "runway")) {
>     *out = waypointRunway(c, wpt);
> +  } else if (!strcmp(fieldName, "hld_is_left_handed")) {
> +    Hold* hold = dynamic_cast<Hold*>(wpt);
> +    if (hold)
> +        *out = naNum(hold->isLeftHanded());
> +    else
> +        return nullptr;
> +  } else if (!strcmp(fieldName, "hld_is_distance")) {
> +    Hold* hold = dynamic_cast<Hold*>(wpt);
> +    if (hold)
> +        *out = naNum(hold->isLeftHanded());
> +    else
> +        return nullptr;
> +  } else if (!strcmp(fieldName, "hld_by")) {
> +    Hold* hold = dynamic_cast<Hold*>(wpt);
> +    if (hold)
> +        *out = stringToNasal(c, hold->isDistance() ? "dist" : "time");
> +    else
> +        return nullptr;
> +  } else if (!strcmp(fieldName, "hld_inbound_radial")) {
> +    Hold* hold = dynamic_cast<Hold*>(wpt);
> +    if (hold)
> +        *out = naNum(hold->inboundRadial());
> +    else
> +        return nullptr;
> +  } else if (!strcmp(fieldName, "hld_heading_radial_deg")) {
> +    Hold* hold = dynamic_cast<Hold*>(wpt);
> +    if (hold)
> +        *out = naNum(hold->inboundRadial());
> +    else
> +        return nullptr;
> +  } else if (!strcmp(fieldName, "hld_time_or_distance")) {
> +    Hold* hold = dynamic_cast<Hold*>(wpt);
> +    if (hold)
> +        *out = naNum(hold->timeOrDistance());
> +    else
> +        return nullptr;
>   } else {
>     return nullptr; // member not found
>   }

That looks okay in concept, but your choice of names is not what we recommend for new code - we try to avoid abbreviations and underscores in Nasal names. So we’d expose names such as:

	holdIsLeftHanded, holdIsDistance, holdInboundRadial,

Also, looking at your patch, I’m a bit confused by the mappings: it looks as if hld_is_distance returns the value of isLeftHanded?  And hld_by returns the strings ‘time’ or ‘dist’?

But in principle yes, exposing this data should be fine.

Kind regards,
James