duplication of SMB ratio for tags

[mountrcg]

Some duplication that leads to a repeated tag in popup and also missaligns the popup, due to conclusion semicolon in wrong place.

bug	solution

[mountrcg]

I would suspect that more of those foul easter eggs can be hidden in oref.

I can't test that thoroughly as I am on a completely different oref and iAPS branch. But this bug would have been very obvious to everyone using a different SMB ratio (!=0.5) in iAPS, if that oref-release would have been used in minimized version.

[bjornoleh]

Perhaps we should look for duplicate code in determine-basal after inclusion of findings from 05a1a2e

That would probably have caught this. Can someone check this?

[bjornoleh]

Here are the lines that were included from minimised files in 05a1a2e

https://github.com/nightscout/open-iaps-oref/blob/97b012ab8e8f0062cecc9d2e02d915ef0124cd2e/lib/determine-basal/determine-basal.js#L756-L758

It is not exactly the same as the lines that ended up as duplicate code and suggested to be removed by @mountrcg

https://github.com/nightscout/open-iaps-oref/blob/97b012ab8e8f0062cecc9d2e02d915ef0124cd2e/lib/determine-basal/determine-basal.js#L1573-L1576

It looks like the duplication is caused by this line, where tddReason is appended to rT.reason, which already includes SMB Ratio:
https://github.com/nightscout/open-iaps-oref/blob/97b012ab8e8f0062cecc9d2e02d915ef0124cd2e/lib/determine-basal/determine-basal.js#L1571

I agree we must remove the duplicate code, while ensuring this works as expected. Perhaps @JeremyStorring could take a look here?

Specifically, I wonder why the printed SMB Ratio is capped at 1 by Math.min(profile.smb_delivery_ratio, 1). There may be reasons to limit smb_delivery_ratio, but capping the output to enacted/suggested would only lead to false information if the setting was actually higher than 1?

[bjornoleh]

I think cd1aa93 is the correct approach for now. We may also want to introduce a real limit to avoid setting smb_delivery_ratio, but I have not looked into where that should happen.

Currently, any value for smb_delivery_ratio is accepted, which can let the app issue SMBs that are larger than insulinReq (SMB = insulinReq * smb_delivery_ratio, unless otherwise restricted by SMB basal minutes or max bolus). This is in no way in line with the original oref0.

[mountrcg]

I think cd1aa93 is the correct approach for now. We may also want to introduce a real limit to avoid setting smb_delivery_ratio, but I have not looked into where that should happen.

Currently, any value for smb_delivery_ratio is accepted, which can let the app issue SMBs that are larger than insulinReq (SMB = insulinReq * smb_delivery_ratio, unless otherwise restricted by SMB basal minutes or max bolus). This is in no way in line with the original oref0.

I agree, capping should be done in Settings or while applying the SMB ratio, as done for many other settings in oref. I would prefer to have that done while defining the setting. I actually know people who use SMB ratio > 1, they would be really hooped with the current implementation.

[bjornoleh]

I will drop the above commit to keep the limit for logs/output while (re-)introducing the missing max limit for smb_ratio:

-            var smb_ratio = profile.smb_delivery_ratio;
+            var smb_ratio = Math.min(profile.smb_delivery_ratio, 1);

Together with this, the limit of max 1 is functional:

https://github.com/nightscout/open-iaps-oref/blob/975245b2adcc17d51b45c4cbd6e2f765d55e23f1/lib/determine-basal/determine-basal.js#L756-L758

[bjornoleh]

After locally merging devinto this branch, and building OiAPS based on the minimised js, I have confirmed that SMB ratio is limited to max 1 by the above changes, both in enacted and in the actually delivered SMBs. And the tags that this PR originally targeted fixing still look good.

[bjornoleh]

@JeremyStorring , I think this should be good now. Could you give a review, and then we can probably merge with dev? Thanks!

[JeremyStorring]

Everything looks good here! No further changes required :)