Re: [Flightgear-devel] route manager crash

[Tobias D. <tda...@gm...>]

On Wed, Jun 27, 2018 at 09:01:46AM +0100, James Turner wrote:
> 
> 
> > 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)

I'd prefer doing that, but things are somewhat underdocumented, and I
didn't feel confident to rely on knowledge gained through
trial-and-error and (possibly misguided) RTFS attempts. And part of the
problem with this part of the codebase specifically is that there's some
unfinished business around, and it's hard to tell which parts are
supposed to be stable, which parts are working, what behavior is
intentional vs. a bug / missing feature, etc. IOW, even if I can figure
out how things work, I can't be sure that what it does is what it's
supposed to do, and whether I'm supposed to rely on it.

> 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,

No problem, I can change them. However, the other waypoint and leg
properties currently use `snake_case`, e.g. `fly_type`, `leg_bearing`;
so I thought I'd keep it consistent. `hld` vs. `hold`, I don't have
strong opinions about.

> 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’?

hld_is_distance is a blatant mistake on my end; it should return
isDistance as a boolean. hld_by is a convenience alias, returning "time"
if isDistance == false, "dist" if isDistance == true. Figured it might
be useful, e.g. for reporting hold parameters in an FMS or PFD/MFD
screen.