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

Avoid non physical values for the ambient pressure and temperature #836

Merged
merged 3 commits into from
Feb 16, 2023

Conversation

bcoconni
Copy link
Member

This PR is addressing the bugs reported in the issue #815. As discussed, it caps pressure and temperature to small values:

JSBSim caps the pressure and temperature to the above mentioned values, issues an error message and continues execution.

I took this opportunity to add checks for other seting methods in FGAtmosphere and FGStandardAtmosphere.

@codecov
Copy link

codecov bot commented Feb 11, 2023

Codecov Report

Merging #836 (637cd1d) into master (ec99f79) will increase coverage by 0.05%.
The diff coverage is 18.29%.

@@            Coverage Diff             @@
##           master     #836      +/-   ##
==========================================
+ Coverage   22.37%   22.43%   +0.05%     
==========================================
  Files         167      167              
  Lines       19535    19600      +65     
==========================================
+ Hits         4371     4397      +26     
- Misses      15164    15203      +39     
Impacted Files Coverage Δ
src/models/FGAtmosphere.h 78.94% <ø> (ø)
src/models/atmosphere/FGStandardAtmosphere.cpp 53.61% <1.81%> (-7.47%) ⬇️
src/models/FGAtmosphere.cpp 53.80% <51.85%> (+9.80%) ⬆️

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 0c519f8 into JSBSim-Team:master Feb 16, 2023
@seanmcleod
Copy link
Member

@agodemar I had a pending code review comment/suggestion for @bcoconni that he hadn't replied to yet.

@bcoconni
Copy link
Member Author

Sorry @seanmcleod but I can't find your code review. Could you remind me what were the modifications that you were suggesting ?

@seanmcleod
Copy link
Member

Hi @bcoconni but they appear just above if you visit the URL for this pull request - #836

Do they not show for you?

@bcoconni
Copy link
Member Author

Hi @bcoconni but they appear just above if you visit the URL for this pull request - #836

Do they not show for you?

I'm afraid not. Below is a screen capture of what I'm seeing.
May be there remains a GitHub button that you didn't push ?

image

@seanmcleod
Copy link
Member

Hmm, I'll take a look, in the meantime this is what I see.

It says Pending but I assumed that means it was pending feedback from you.

image

@bcoconni
Copy link
Member Author

bcoconni commented Feb 18, 2023

Hmm, I'll take a look, in the meantime this is what I see.

It says Pending but I assumed that means it was pending feedback from you.

It seems it is pending for you according to the GitHub docs:

Before you submit your review, your line comments are pending and only visible to you. You can edit pending comments anytime before you submit your review. To cancel a pending review, including all of its pending comments, scroll down to the end of the timeline on the Conversation tab, then click Cancel review.

The last steps you need to follow are described in the GitHub docs: Submitting your review

@bcoconni bcoconni deleted the bcoconni/issue815 branch February 18, 2023 11:49
@bcoconni bcoconni mentioned this pull request Feb 18, 2023
bcoconni added a commit to bcoconni/jsbsim that referenced this pull request Apr 8, 2023
…SBSim-Team#836)

* Validate input values to avoid NaNs and other silly behaviors.

* Add tests for invalid input values

* Add a test for the sea level pressure
# 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.

3 participants