Things noticed while porting wear from NW (TODO list)

AdrianLxM commented 8 years ago

BgGraphBuilder - make many methods static or refactor them to a separate class (no need to load all data for the graph to unitize one value e.g.)
Play-Service versions for wear integration are pretty old and versions for "app" and "wear" do not match.
Integrate raw values - remove redundency to calculate "Nightscout-raw" and NS/DexCom calibraion format for:
- local broadcast
- wear integration
- upload to Share
- upload to NightScout
Integrate charging mode that turns the screen by 90° when watch is in the night-stand charger.
NightWatch has Activities of module "mobile" in the "wear" manifest. (delete them)
better wakelocks when sending data to the watch (not just a 15 seconds timeout wakelock)
Make timestamps of type long not double (especially in former NightWatch parts but xDrip still is not free from that)
Maybe: triggering a resend to wear from the gui could start a background thread "earlier" (not just for sending like now, but also for preparing the data).
Helper Methods fragmented over 'WatchUpdaterService.java' and 'BgGraphBuilder.java'.
Quite a few utility methods are of type: (already marked some)

if(booleanExpression){
    return true;
else {
    return false;
}

PebbleSync has no wakelocks
Pebble has a "resend to watch" function on NightWatch but not on xDrip.

mgranberry commented 8 years ago

The IntelliJ 16 EAP has a pretty cool "find duplicated code" analyzer that gives a side-by-side difference between two bits of code. It might be useful to play around with it while doing this.

tzachi-dar commented 8 years ago

What do you mean by "Integrate raw values"?

AdrianLxM commented 8 years ago

@tzachi-dar I disabled that the watch faces show raw. I will enable it once we have a uniform calculation method.

There were just about 100 downloads of the apk that supports raw and I will reintegrate it before we have a release of xDrip-Viewer. So people using this feature will still have it.

AdrianLxM commented 8 years ago

@tzachi-dar http://www.nightscout.info/wiki/labs/raw-data-with-nightwatch

tzachi-dar commented 8 years ago

I was more interested in the way that you want to handle raw... what version do you propose?

On Fri, Jan 8, 2016 at 1:49 AM, AdrianLxM notifications@github.com wrote:

@tzachi-dar https://github.com/tzachi-dar http://www.nightscout.info/wiki/labs/raw-data-with-nightwatch

— Reply to this email directly or view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/issues/243#issuecomment-169843172 .

AdrianLxM commented 8 years ago

The same logic as we already have in bgSendQueue as local broadcast - and therefore already show in NightWatch. It is the same as the NightScout-Website shows when enabling raw.

tzachi-dar commented 8 years ago

can you point me to some place in the code so I'll be sure about what you mean?

Thanks Tzachi

On Fri, Jan 8, 2016 at 2:00 AM, AdrianLxM notifications@github.com wrote:

The same logic as we already have in bgSendQueue as local broadcast - and therefore already show in NightWatch. It is the same as the NightScout-Website shows when enabling raw.

— Reply to this email directly or view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/issues/243#issuecomment-169844890 .

AdrianLxM commented 8 years ago

@tzachi-dar: https://github.com/nightscout/cgm-remote-monitor/blob/master/lib/plugins/rawbg.js#L59

@mgranberry: the android code cleanup also does a lot of things automatically. Unfortunately also some things I don't like. Code duplication just to a very small extent. This one is typical:

diff --git a/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java b/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java
index dfbc869..d3c8979 100644
--- a/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java
+++ b/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java
@@ -665,10 +665,7 @@ public class EditAlertActivity extends ActivityWithMenu {
             return false;
         }
         Ringtone ringtone = RingtoneManager.getRingtone(context, Uri.parse(path));
-        if(ringtone == null) {
-            return false;
-        }
-        return true;
+        return ringtone != null;
     }

     public String shortPath(String path) {

tzachi-dar commented 8 years ago

Is this the answer to my question about raw?

On Fri, Jan 8, 2016 at 2:21 AM, AdrianLxM notifications@github.com wrote:

@tzachi-dar https://github.com/tzachi-dar: https://github.com/nightscout/cgm-remote-monitor/blob/master/lib/plugins/rawbg.js#L59

@mgranberry https://github.com/mgranberry: the android code cleanup also does a lot of things automatically. Unfortunately also some things I don't like. This one is typical:

diff --git a/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java b/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java index dfbc869..d3c8979 100644 --- a/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java +++ b/app/src/main/java/com/eveningoutpost/dexdrip/EditAlertActivity.java @@ -665,10 +665,7 @@ public class EditAlertActivity extends ActivityWithMenu { return false; } Ringtone ringtone = RingtoneManager.getRingtone(context, Uri.parse(path));
if(ringtone == null) {

return false;

}

return true;
 return ringtone != null;
}

public String shortPath(String path) {
— Reply to this email directly or view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/issues/243#issuecomment-169848450 .

AdrianLxM commented 8 years ago

Yes, that's the formula. But it's already tested that it's the same we have in BgSendQueue.java.

tzachi-dar commented 8 years ago

oops, I was thinking about the ringtone code...

On Fri, Jan 8, 2016 at 2:26 AM, AdrianLxM notifications@github.com wrote:

Yes, that's the formula.

— Reply to this email directly or view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/issues/243#issuecomment-169849415 .

AdrianLxM commented 8 years ago

Ah, sorry, I replied do you and to Matthias in one comment. The Ringtone code was an example of what the automatic code cleanup in Android Studio does.

tzachi-dar commented 8 years ago

This is very different than what is done in xDrip and I guess that in most places (expect for dexcom).

I believe that we should stay with the simple formula:

calculated = raw * slope + intercept.

https://en.wikipedia.org/wiki/Linear_equation

[image: Inline image 1] where m is the slope of the line and b is the y intercept, which is the y coordinate of the location where the line crosses the y axis. This can be seen by letting x = 0, which immediately gives y = b. It may be helpful to think about this in terms ofy = b + mx; where the line passes through the point (0, b) and extends to the left and right at a slope of m. Vertical lines, having undefined slope, cannot be represented by this form.

On Fri, Jan 8, 2016 at 2:28 AM, Tzachi Dar tzachi.dar@gmail.com wrote:

oops, I was thinking about the ringtone code...

On Fri, Jan 8, 2016 at 2:26 AM, AdrianLxM notifications@github.com wrote:

Yes, that's the formula.

— Reply to this email directly or view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/issues/243#issuecomment-169849415 .

AdrianLxM commented 8 years ago

This is exactly what we do in xDrip and broadcast to the other apps: https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/blob/master/app/src/main/java/com/eveningoutpost/dexdrip/UtilityModels/BgSendQueue.java#L128

If it makes sense to show the "NightScout-Raw" like this is another discussion, that should be discussed in a separate issue over there: https://github.com/nightscout/cgm-remote-monitor/

But the watchface should show the same number that people have on their Nightscout site, their pebbles, the NightWatch watchfaces, ...

AdrianLxM commented 8 years ago

To make it more clear that this is slope, intercept, scale and raw as Nightscout understands it and NOT the slope, intercept, ... xDrip internally calculates with: I extracted methods from the code called getNightscoutSlope(), getNighscoutScale(), ... and put them into NightscoutUploader.java where we already calculated most of that for uploading it to NightScout: Here is the commit/diff: https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/commit/542ac5cfb67f5e560f7308dc3fefdbcb4322d1de

tzachi-dar commented 8 years ago

Here is the way I think it should be, not very far from the way that you are doing this...

Before I speak of that, let's see how we deal with mgdl and mmol different units. Since there is a one to one conversion from one to the other, our live have been simple. Xdrip always work in mgdl. Only before printing the data, it might change that to mmo/l. When data is entered, it is converted from mmo/l to mgdl. And as I said calculations are always done with mgdl.

On the intercept/slope field, we did not use this method, we have sometimes been using one of them and sometimes the other, and from here comes the confusion. So, I would really like to have all slope intercept used in the xDrip method and changed only before sending them out. However, since we have them sometimes stored in one way and sometimes on the other way, we will probably need to write a convertor in order to do that. (that is go over our DB and convert from one method to the other when needed). Frankly, creating this convertor seems too much for me... so we might end with the mess we already have.

Does this sound reasonable?

Thanks Tzachi

On Fri, Jan 8, 2016 at 5:05 AM, AdrianLxM notifications@github.com wrote:

To make it more clear that this is slope, intercept, scale and raw as Nightscout understands it and NOT the slope, intercept, ... xDrip internally calculates with: I extracted methods from the code called getNightscoutSlope(), getNighscoutScale(), ... and put them into NightscoutUploader.java where we already calculated most of that for uploading it to NightScout: 542ac5c https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/commit/542ac5cfb67f5e560f7308dc3fefdbcb4322d1de

— Reply to this email directly or view it on GitHub https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/issues/243#issuecomment-169879973 .

AdrianLxM commented 8 years ago

Yes, the problem is that we use the same data structure for two types of calibration and distinguish them by the boolean flag check_in. That happened, when the Share support got merged in I guess. Maybe even earlier as you can add calibrations by connecting a G4 receiver via USB but still use them with transmitter data received by a wixel. This code I didn't touch. It works as it is and changing things here would need extensive care and testing and could break a lot of things.

The methods I created are, as you suggested, just to create data sent to external components (Nightscout and broadcast to other apps that expect it in this format).

Like you said, it sounds reasonable to me as well, not converting the whole base.

StephenBlackWasAlreadyTaken / xDrip-Experimental

Things noticed while porting wear from NW (TODO list) #243