Unbind FGAtmosphere properties

[bcoconni]

As I mentioned previously in #885 (comment), I'm currently working on adding the NRLMSIS2.0 atmospheric model to JSBSim which is kind of a revival of the existing src/models/atmosphere/FGMSIS.cpp code (which was based on the previous version 1.0 of NRLMSIS)¹.

In order for this feature to work, we will need to modify the atmosphere model that is currently created by default in FGFDMExec:

jsbsim/src/FGFDMExec.cpp

Line 229 in 39e7058

Models[eAtmosphere] = std::make_shared<FGStandardAtmosphere>(this);

Basically, the idea is to call at some point the following code:

if (some_XML_instructions_are_found) {
  // Delete the FGStandardAtmosphere instance and replace it by an FGMSIS instance
  Models[eAtmosphere] = std::make_shared<FGMSIS>(this);
  // ERROR messages : Failed to tie property XXX to object methods.
}

The trick is that if no special care is taken, the properties atmosphere/T-R, atmosphere/P-Psf, etc. will still be bound to the old instance of FGStandardAtmosphere even after the above code has been executed. With the current code, FGMSIS would fail to bind itself to the aforementioned properties as they would already be tied to FGStandardAtmosphere and JSBSim would complain with the infamous error message Failed to tie property atmosphere/T-R to object methods. Even worse, if one would try to access atmosphere/T-R and the likes, it would cause a SEGFAULT as the property would try to call the method FGAtmosphere::GetTemperature() on a deleted FGStandardAtmopshere instance.

To avoid this issue, we need to tell JSBSim to unbind FGStandardAtmosphere before an instance of FGMSIS is created. Currently there is no generic code in JSBSim that allows to unbind a single FGModel instance (you'd need to untie the properties one by one which is less than ideal from a maintenance standpoint).

To address this issue the current PR adds a new method FGPropertyManager::Unbind(void* instance) to unbind the properties of a given class instance. With this new method, the code above will become something like:

if (some_XML_instructions_are_found) {
  PropertyManager->Unbind(Models[eAtmosphere]);
  Models[eAtmosphere] = std::make_shared<FGMSIS>(this);
  // SUCCESS! No "Failed to tie..." error messages.
}

The new Unbind method also restores the read/write attributes of each properties as the newly bound instance might want to set them differently.

Finally, the unit test FGAtmosphereTest is amended by the PR to test the new feature and checks that the properties such atmosphere/T-R are tied to the correct instance (see the method testRun() in FGAmosphereTest).

Footnotes

1. For the record the code in FGMSIS.cpp is dead code as there is currently no means to execute this code from JSBSim. ↩

[codecov]

Codecov Report

Merging #902 (cee8801) into master (39e7058) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #902      +/-   ##
==========================================
+ Coverage   23.09%   23.16%   +0.07%     
==========================================
  Files         167      167              
  Lines       19626    19644      +18     
==========================================
+ Hits         4533     4551      +18     
  Misses      15093    15093              

Impacted Files	Coverage Δ
src/input_output/FGPropertyManager.cpp	52.89% <100.00%> (+3.33%)	⬆️
src/input_output/FGPropertyManager.h	87.27% <100.00%> (+2.82%)	⬆️

[seanmcleod]

Assuming we're not planning on allowing a user to switch atmospheric models part-way through a simulation, i.e. a single atmospheric model is chosen/set at startup then instead of having to handle, binding, unbinding, binding again can't we simply create the relevant instance of the atmosphere model at:

jsbsim/src/FGFDMExec.cpp

Line 229 in 39e7058

Models[eAtmosphere] = std::make_shared<FGStandardAtmosphere>(this);

[bcoconni]

Assuming we're not planning on allowing a user to switch atmospheric models part-way through a simulation,

I confirm there is no such plan.

i.e. a single atmospheric model is chosen/set at startup then instead of having to handle, binding, unbinding, binding again can't we simply create the relevant instance of the atmosphere model at:

jsbsim/src/FGFDMExec.cpp

Line 229 in 39e7058

Models[eAtmosphere] = std::make_shared<FGStandardAtmosphere>(this);

Unfortunately we can't because the line you mentioned is called from the FGFDMExec constructor but it's only when FGFDMExec::LoadModel() parses the XML files that you discover which atmospheric model should be used.