Add support to a different unit of measurement(mmol/L) on Nightscout #54898
Conversation
|
Hey there @marciogranzotto, mind taking a look at this pull request as it has been labeled with an integration ( |
| def _get_bgl(self, value) -> float: | ||
| if self._unit_of_measurement == MMOL_L: | ||
| return round((value / MMOL_CONVERSION_FACTOR), 1) | ||
| return value |
There was a problem hiding this comment.
We generally don't want these types of conversions or calculations in Home Assistant.
The reasoning is simple: Before we know it, we will have tons of integrations offering all kinds of conversions. If anything, such a problem needs to be solved globally and not in a case-by-case situation.
So, at this point, we generally hold on to: Provide the uom the device natively provides.
So IMHO, we should not add this.
There was a problem hiding this comment.
Hey @frenck,
Unfortunately, the API of this integration only provides data in this specific unit of measure.
It does have an endpoint indicating if the user would like to have the values displayed in a different UOM, but it does not handle the conversion itself. Changing the code on the integration would be much harder, as it is an(although not FDA approved) medical application, so the changes would be more drastic.
I understand your point. Trying to support all different types of UOMs can get overwhelming and out of control, but all the applications that I know of that integrate with Nightscout, provide a way to convert the values since there are two different UOMs used worldwide for this type of data.
I've seen some requests on this repo, on the add-on repo, and also on the forums. That's why I thought it would be a good idea to implement the feature. Links below for reference:
- nightscout integration should support mmol/l in addition to mg/ml #45002
- unit change marciogranzotto/addon-nightscout#6
- Introduce conversion to mmol/L in Nightscout #41382 (comment)
- https://community.home-assistant.io/t/nightscout-ability-to-choose-units/307571
- https://community.home-assistant.io/t/nightscout-plugin-show-mmol-l/316107
With that said, do you believe those would be legit reasons for the change?
There was a problem hiding this comment.
@frenck maybe is best to add this conversion on the py_nightscout lib?
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Breaking change
Proposed change
The existing Nightscout integration only supports one unit of measurement for blood glucose values(mg/dL). Most countries in Europe and Oceania use a different unit of measurement(mmol/L).
This PR introduces an option to chose which unit of measurement the user wants for the Nightscout integration.
I worked on this feature, but during the creation of the PR I noticed that someone else attempted to do the same thing a while ago on #41382 but it ended up being automatically closed due to inactivity. My changes are similar to the ones proposed on the other PR, but still slightly different.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: