Skip to content

Conversion between pressure measurement references #726

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Dec 11, 2019

Conversation

sequc82
Copy link
Contributor

@sequc82 sequc82 commented Dec 6, 2019

Based off the proposal in #422, and building upon the work of @gratestas in Units by @YektaMirkan.

Adds a wrapper class called ReferencePressure and an enum PressureReference to provide a reference for pressure measurements.

public enum PressureReference
    {
        Undefined = 0,
        Absolute,
        Gauge,
        Vacuum,
    }

Utilizes two constructors for implementation:

ReferencePressure refPressure = new ReferencePressure(Pressure.fromAtmospheres(3));
ReferencePressure refPressure = new ReferencePressure(Pressure.fromAtmospheres(3), PressureReference.Gauge);

The single-parameter constructor sets the PressureReference parameter to a default of PressureReference.Absolute

As @gratestas mentioned in #422, implementation uses the example:

double pressure = refPressure.Gauge.Atmosphere;

Extended the work of @gratestas with further testing, and handling of exceptions regarding pressures less than absolute zero.

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, love the tests!
I got some feedback for you to consider.

@codecov-io
Copy link

codecov-io commented Dec 9, 2019

Codecov Report

Merging #726 into master will increase coverage by 0.05%.
The diff coverage is 96.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #726      +/-   ##
==========================================
+ Coverage   58.51%   58.56%   +0.05%     
==========================================
  Files         170      171       +1     
  Lines       38375    38430      +55     
==========================================
+ Hits        22454    22507      +53     
- Misses      15921    15923       +2
Impacted Files Coverage Δ
UnitsNet/CustomCode/Wrappers/ReferencePressure.cs 96.36% <96.36%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 305ef08...04e9534. Read the comment docs.

@angularsen
Copy link
Owner

angularsen commented Dec 9, 2019 via email

@sequc82
Copy link
Contributor Author

sequc82 commented Dec 9, 2019

I added a ctor overload for a third parameter: the atmospheric pressure referenced for conversion between Absolute, Gauge, and Vacuum. Otherwise, ReferencePressure would always assume 1 atm.

@sequc82 sequc82 requested a review from angularsen December 9, 2019 21:09
@angularsen
Copy link
Owner

angularsen commented Dec 9, 2019 via email

@sequc82
Copy link
Contributor Author

sequc82 commented Dec 9, 2019

@angularsen, based on your comment in #724, would it be best to move the code in Pressure.Wrapper.cs and the PressureReference enum into Pressure.extra.cs?

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple minor things, but it's looking good!

@sequc82 sequc82 requested a review from angularsen December 11, 2019 16:22
Chris Sequeira added 2 commits December 11, 2019 11:48
@sequc82 sequc82 requested a review from angularsen December 11, 2019 17:57
Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is getting ready, just a few more minor things.

@sequc82 sequc82 requested a review from angularsen December 11, 2019 18:43
@angularsen angularsen merged commit 4415358 into angularsen:master Dec 11, 2019
@angularsen
Copy link
Owner

Awesome, great work!

Nuget is on the way out.

Release UnitsNet/4.42.0 · angularsen/UnitsNet

@angularsen angularsen mentioned this pull request Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants