Do not make direct calls to FGAtmosphere

[bcoconni]

This PR is somewhat an opinionated one 😄

Going back almost 12 years ago the mediator programming pattern was introduced in JSBSim by @jonsberndt via the commit 3c05fee. In JSBSim the mediator object is the FGFDMExec class and the current architecture of JSBSim is that all communications between models (i.e. classes inheriting from FGModel) are made via FGFDMExec (more specifically by the method FGFDMExec::LoadInputs). Following the merge of the PR #881 this is no longer the case as FGAuxiliary makes direct calls to FGAtmosphere. The purpose of this PR is therefore to restore a "pure" mediator pattern.

In that purpose this PR moves the computation of the calibrated airspeed, total pressure and total temperature to FGAtmosphere where the routines required to perform these calculations are located. This change is absolutely not mandatory as JSBSim works perfectly well in its current state but the current state is a small breach in the overall architecture.

In order to compute calibrated airspeed and the total pressure and temperature FGAtmosphere need to get the aircraft aerodynamic velocity which includes getting access to the wind velocity. Currently FGWinds is evaluated after FGAtmosphere which means that FGAtmosphere uses a wind velocity that lags one time step behind. I have considered swapping the order in which FGWinds and FGAtmosphere were called but I have dropped this idea because I was finding it too intrusive¹ while I don't see much problem in using the wind velocity from the previous time step. I don't expect the wind velocity changing much between 2 time steps (especially if the main loop runs at 120Hz).

The total temperature computation is also moved to FGAtmosphere as its formula actually depends on the specific heat ratio $\gamma$ (i.e. FGAtmosphere::SHRatio):
$$T_0=T\left(1+\frac{\gamma-1}{2}M^2\right)$$

This PR is a bit questionable in that computing a velocity in the class FGAtmosphere is not an obvious move. It is rather dictated by programming considerations (vcas uses methods from FGAtmosphere) rather than common sense. On the other hand, the calibrated airspeed is tightly linked to the atmospheric properties in which the aircraft flying, so... I'm submitting this PR and waiting for your comments/opinions.

Footnotes

1. See commits 51f8b3e, 88b7def and 9f775fe to convince yourself that changing the order in which models are evaluated is as much an art as a science. ↩

[codecov]

Codecov Report

Merging #885 (a0eb0b8) into master (92fa1ea) will increase coverage by 0.03%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master     #885      +/-   ##
==========================================
+ Coverage   23.06%   23.10%   +0.03%     
==========================================
  Files         167      167              
  Lines       19611    19613       +2     
==========================================
+ Hits         4524     4532       +8     
+ Misses      15087    15081       -6     

Impacted Files	Coverage Δ
src/input_output/FGOutputFG.cpp	0.00% <0.00%> (ø)
src/models/FGAuxiliary.h	83.33% <ø> (-1.58%)	⬇️
src/models/FGAuxiliary.cpp	60.89% <66.66%> (-0.30%)	⬇️
src/FGFDMExec.cpp	41.12% <100.00%> (-0.09%)	⬇️
src/models/FGAtmosphere.cpp	95.06% <100.00%> (+0.48%)	⬆️
src/models/FGAtmosphere.h	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[seanmcleod]

Hmm, although I gave the thumbs up for PR - #881, I'm now in two minds about where it would be best to perform the velocity calculations, i.e. in the auxiliary class or the atmosphere class.

Partially thinking about the required inputs for the two approaches and what it means for someone developing a new atmosphere model that they want to include. Should they need to concern themselves with velocity calculations versus simply returning atmospheric state?

So atmosphere developers now need to handle and process u,v,w body velocities plus the wind vector and using the supplied transformation matrix between local NED and body. Then they also need to know about the formulas for calculating pressures with Mach effects etc.

Hmm, so while composing this and checking the code a bit more I just realised there is an FGAtmosphere as a base class where you have implemented these changes, versus derived classes like FGStandardAtmosphere which developers of atmospheric models would implement their atmosphere, i.e. they won't have to worry about u,v,w velocities etc. which is handled in the base class.

So now I don't really mind 😉

[bcoconni]

Hmm, although I gave the thumbs up for PR - #881, I'm now in two minds about where it would be best to perform the velocity calculations, i.e. in the auxiliary class or the atmosphere class.

I agree with you that neither option is the perfect location for the computation of the CAS. However I've given more thoughts to this PR since I submitted it a week ago: whatever the choice we will make, FGAuxiliary should no longer make direct calls to FGAtmosphere.

The reason is that I am currently working on restoring the MSIS atmosphere¹ in JSBSim (see the MSIS branch on my personal fork). In order to be able to switch between the ISA model and the MSIS model at run time, we'll need to be able to untie/unplug at run time the existing instance of FGStandardAtmosphere from FGFDMExec then replace it by an instance of FGMSIS. Obviously this can be achieved by overwriting Models[eAtmosphere] in FGFDMExec. However if FGAuxiliary uses a pointer to the initial instance of FGStandardAtmosphere then this pointer will dangle as soon as the MSIS model is enabled and the instance of FGStandardAtmosphere is deleted.

So FGAuxiliary should definitely no longer make direct calls to FGAtmosphere.

So atmosphere developers now need to handle and process u,v,w body velocities plus the wind vector and using the supplied transformation matrix between local NED and body.

Not sure the alternatives are much compelling: the computation of the total pressure needs to call the method FGAtmosphere::PitotTotalPressure and get the Mach number. For the reasons I have explained above, FGAuxiliary should not make direct calls to FGAtmosphere so AFAICS the options are:

1. FGAtmosphere computes the Mach number locally and sends the total pressure to FGAuxiliary
2. FGAtmosphere uses the Mach number computed by FGAuxiliary during the previous time step.
3. Switch the order in which FGAtmosphere and FGAuxiliary are executed by FGFDMExec. In that case, that would mean that FGAuxiliary uses an air density from the previous time step.

I made the choice of using the first option for this PR but the other options can also be considered.

Hmm, so while composing this and checking the code a bit more I just realised there is an FGAtmosphere as a base class where you have implemented these changes, versus derived classes like FGStandardAtmosphere which developers of atmospheric models would implement their atmosphere, i.e. they won't have to worry about u,v,w velocities etc. which is handled in the base class.

Yes, that's really the idea. The parameters used by these formulas might be modified by atmospheric models but the formulas themselves remain valid for any atmospheric model. So the point is to save developers of atmospheric models the trouble of duplicating this code.

Footnotes

1. Actually I should say "enabling" rather than restoring since it is currently impossible to enable the MSIS model (i.e. the code from the files FGMSIS.h, FGMSIS.cpp and FGMSISData.cpp) from an FDM. ↩

[bcoconni]

PR cancelled because it has been superseded by #898.