Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add a unit test for FGAtmosphere #857

Merged
merged 4 commits into from
Mar 15, 2023

Conversation

bcoconni
Copy link
Member

This PR adds a unit test for the FGAtmosphere class in preparation for further changes that will address the topics mentioned in #666 (comment) (static members & methods) and #815 (comment) (MSIS atmospheric model).

The unit test in this PR will provide a means to check that no regressions will be introduced by future changes to FGAtmosphere.

In the process of writing this unit test, the following bugs have been discovered and fixed:

  • In FGAtmosphere::InitModel(), Viscosity and KinematicViscosity were computed before SLTemperature was initialized resulting in incorrect values of air viscosity.
  • In FGAtmosphere::Calculate() the density was ignoring the values from the properties atmosphere/override/temperature or atmosphere/override/pressure if one or both of these features were used.
  • In FGAtmosphere::SetPressureSL(), SLdensity was not updated despite the fact that SLpressure was modified.
  • In FGAtmosphere::SetTemperatureSL(), SLdensity and SLsoundspeed were not updated despite the fact that SLtemperature was modified.
  • Providing an erroneous unit enum value to conversion methods (ConvertToRankine, ConvertFromPSF, etc.) now always raise an exception. Previously, some conversion routines silently swallowed the error returning null temperatures or pressures in the process. And as has been discussed in issue FGStandardAtmosphere:CalculatePressureAltitude() can cause NaN values #815, null temperature and pressure are evil !
  • All the class members are now initialized (with meaningful values) when the constructor is called. Previously the constructor of FGAtmosphere did not initialize its members: that stage was defered to the call to InitModel(). This is evil because you never know what code might be executed between the construction and the call to InitModel().

* Fix `SetTemperatureSL()` and `SetPressureSL()` which did not update the sea level density and speed of sound.
* Make `Reng` a non `static` member (see JSBSim-Team#666). This was useless and prevented `Reng` from being accessed when JSBSim was compiled as a DLL.
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Merging #857 (57e3607) into master (b2172a4) will increase coverage by 0.63%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##           master     #857      +/-   ##
==========================================
+ Coverage   22.43%   23.07%   +0.63%     
==========================================
  Files         167      167              
  Lines       19608    19609       +1     
==========================================
+ Hits         4400     4525     +125     
+ Misses      15208    15084     -124     
Impacted Files Coverage Δ
src/models/FGAtmosphere.h 100.00% <ø> (+21.05%) ⬆️
src/models/FGAtmosphere.cpp 93.60% <90.00%> (+43.89%) ⬆️

... and 22 files with indirect coverage changes

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

@agodemar agodemar merged commit 1f54205 into JSBSim-Team:master Mar 15, 2023
@bcoconni bcoconni deleted the FGAtmosphereTest branch March 15, 2023 21:29
bcoconni added a commit to bcoconni/jsbsim that referenced this pull request Apr 8, 2023
* [WIP] Add a unit test for `FGAtmosphere`

* Bug fixes

* Fix `SetTemperatureSL()` and `SetPressureSL()` which did not update the sea level density and speed of sound.
* Make `Reng` a non `static` member (see JSBSim-Team#666). This was useless and prevented `Reng` from being accessed when JSBSim was compiled as a DLL.

* Forgot `JSBSIM_API`.

* Increased the code coverage and fixed some bugs in the process.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants