Closed tzachi-dar closed 8 years ago
Even though I would do things a bit differently (boolean flag instead of magic character at the beginning of a String), I think this is very good work!
I'll be travelling the next two weeks. If you want me to test it, It'll take some time, sorry.
I'll pull it down and give it a run Tzachi-dar, if you think it is stable enough. I'll merge it with my PebbleTrend branch though. They shouldn't clash in any way. Cheers
On Tue, Jun 14, 2016 at 12:55 AM, AdrianLxM notifications@github.com wrote:
Even though I would do things a bit differently (boolean flag instead of magic character at the beginning of a String), I think this is very good work!
I'll be travelling the next two weeks. If you want me to test it, It'll take some time, sorry.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/pull/352#issuecomment-225605811, or mute the thread https://github.com/notifications/unsubscribe/AIQs80_6MXqG3YQ-xfDXS4Kl_grPvPijks5qLW9egaJpZM4Iz6Dh .
John Stevens "You are how you live, not what you have."
This would be great if you can test that john.
Hi Tzachi-dar, and all, I have tested this briefly. The PR allows me to disable the Unchangeable 3.0mmol alert. I have created a new "Hypo" alert set above my current reading to test. Just waiting to see if that one raises. But otherwise appears to be all good.
This will be great. I will be able t set two hypo alarms, one for daytime, one for nighttime. Might keep my wife happy. Do we want some form of "Do you really want to do this?" message when the "unchangeable" alert is disabled? Or at least a check to see if another low alert is configured, and if there isn't one, then ask?
I am sure the Dexcom unchangeable alert is there purely for FDA approval reasons. But we might need to make it obvious that disabling this alert is a risk.
Great work. Cheers Cheers
On Tue, Jun 14, 2016 at 5:23 PM, tzachi-dar notifications@github.com wrote:
This would be great if you can test that john.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/pull/352#issuecomment-225800021, or mute the thread https://github.com/notifications/unsubscribe/AIQs87fy2wKQhMiIrYyEpTon06Yv8aqVks5qLlcFgaJpZM4Iz6Dh .
John Stevens "You are how you live, not what you have."
Hi All, Alert disable, and alerts working as expected. Please merge at will. Cheers
On Fri, Jun 17, 2016 at 10:33 AM, John Stevens jstevensog@gmail.com wrote:
Hi Tzachi-dar, and all, I have tested this briefly. The PR allows me to disable the Unchangeable 3.0mmol alert. I have created a new "Hypo" alert set above my current reading to test. Just waiting to see if that one raises. But otherwise appears to be all good.
This will be great. I will be able t set two hypo alarms, one for daytime, one for nighttime. Might keep my wife happy. Do we want some form of "Do you really want to do this?" message when the "unchangeable" alert is disabled? Or at least a check to see if another low alert is configured, and if there isn't one, then ask?
I am sure the Dexcom unchangeable alert is there purely for FDA approval reasons. But we might need to make it obvious that disabling this alert is a risk.
Great work. Cheers Cheers
On Tue, Jun 14, 2016 at 5:23 PM, tzachi-dar notifications@github.com wrote:
This would be great if you can test that john.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/pull/352#issuecomment-225800021, or mute the thread https://github.com/notifications/unsubscribe/AIQs87fy2wKQhMiIrYyEpTon06Yv8aqVks5qLlcFgaJpZM4Iz6Dh .
John Stevens "You are how you live, not what you have."
John Stevens "You are how you live, not what you have."
Thanks for testing this.
I'll merge it soon, but I'll soon add a warning message about disabling all low alerts.
Tzachi
On Fri, Jun 17, 2016 at 4:00 AM, John notifications@github.com wrote:
Hi All, Alert disable, and alerts working as expected. Please merge at will. Cheers
On Fri, Jun 17, 2016 at 10:33 AM, John Stevens jstevensog@gmail.com wrote:
Hi Tzachi-dar, and all, I have tested this briefly. The PR allows me to disable the Unchangeable 3.0mmol alert. I have created a new "Hypo" alert set above my current reading to test. Just waiting to see if that one raises. But otherwise appears to be all good.
This will be great. I will be able t set two hypo alarms, one for daytime, one for nighttime. Might keep my wife happy. Do we want some form of "Do you really want to do this?" message when the "unchangeable" alert is disabled? Or at least a check to see if another low alert is configured, and if there isn't one, then ask?
I am sure the Dexcom unchangeable alert is there purely for FDA approval reasons. But we might need to make it obvious that disabling this alert is a risk.
Great work. Cheers Cheers
On Tue, Jun 14, 2016 at 5:23 PM, tzachi-dar notifications@github.com wrote:
This would be great if you can test that john.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/pull/352#issuecomment-225800021 , or mute the thread < https://github.com/notifications/unsubscribe/AIQs87fy2wKQhMiIrYyEpTon06Yv8aqVks5qLlcFgaJpZM4Iz6Dh
.
John Stevens "You are how you live, not what you have."
John Stevens "You are how you live, not what you have."
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/pull/352#issuecomment-226655554, or mute the thread https://github.com/notifications/unsubscribe/AHkw5MM8AlpUdww5skAevQnK98Y5TFULks5qMfG_gaJpZM4Iz6Dh .
Closing as https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/pull/353 is already merged. Thanks Tzachi.
This pr allows one to disable any alert. Yes also the 55 alert. It also solves issues that happened when editing alert and changing screen orientation. Also fixes some issues of not using super() in previous commit.
Still under testing.