-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: convert BACnet units to unit symbols (use SI where possible) #16
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally it looks like it is all sane SI units plus Non-SI units accepted for use with the International System of Units. But I am not sure this will really help us towards our goal: Being able to simply slap on a SI-Unit prefix.
- So many existing prefixes, so we need to parse them. But there is a lot of ambiguity, e.g. "dB" "min"
- Parsing prefixes is even more error prone with squares
- Adding prefixes with squares is error prone
- Parsing and adding prefixes within ratios
At the end of the day, I think we need to find a library to actually achieve our goal (maybe this means in JS). And then we need to figure out if the library eats what the output of this.
229: "mSv", | ||
230: "µSv", | ||
231: "µSv/h", | ||
234: "pH", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless that is picoHenry - the pH value is dimensionless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we need an empty unit here otherwise the BACnet unit name (pH
) is used
if unit_int in BACNET_TO_SI_UNITS: | ||
unit = BACNET_TO_SI_UNITS.get(unit_int) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why you'd combine if
/ get
. Could use get(unit_id, unit)
or try
/ except KeyError
167: "A/m", | ||
168: "A/m²", | ||
169: "A m²", | ||
199: "dB", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dB is explicitly a non-SI with the following remark:
In using these units it is important that the nature of the quantity be specified, and that any
reference value used be specified. These units are not SI units, but they have been accepted by
the CIPM for use with the SI.
200: "dBmV", | ||
201: "dBV", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That may even be "acceptable non-SI" but parsing hell...
214: "g/l", | ||
215: "mg/l", | ||
216: "µg/l", | ||
217: "g /m³", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
217: "g /m³", | |
217: "g/m³", |
187: "N s", | ||
188: "N/m", | ||
96: "ppm", | ||
97: "ppb", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The terms “parts per billion”, and “parts per trillion”, and their respective abbreviations “ppb”,
and “ppt”, are also used, but their meanings are language dependent. For this reason
the terms ppb and ppt are best avoided. (In English-speaking countries, a billion is
now generally taken to be 10^9 and a trillion to be 10^12; however, a billion may still
sometimes be interpreted as 10^12 and a trillion as 10^18. The abbreviation ppt is also
sometimes read as parts per thousand, adding further confusion.)
96: "ppm", | ||
97: "ppb", | ||
98: "%", | ||
99: "%/s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... maybe ....
To make this clear, the list of units is defined by BACnet and every unit not in the map will use is unit name from BACnet |
Currently we use the following units:
And yes, |
Fixes #3
Includes a script to check the conversation between BACnet units and unit symbols and also show missing units.
python helper_scripts/check_bacnet_units.py --all