search

StephenBlackWasAlreadyTaken / xDrip-Experimental

Experimental Branches for Collaboration on DexDrip
GNU General Public License v3.0
25 stars 62 forks source link

X drip viewer7 #275

Closed tzachi-dar closed 8 years ago

tzachi-dar commented 8 years ago

So finally the xDrip viewer is ready for review.

There are a few issues that I'm aware of right now, but I don't think that are blocking: 1) build now takes bigger time. please run (for example) gradlew assemblexDripDebug to only build one project.

2) the names of the apks have changed (for example, app-xDripViewer-debug.apk)

3) a complete build prints an error message: "Execution failed for task ':app:handleXDripViewerReleaseMicroApk'." This is not a real issue since all is being built. Searching in the internet this seems to have a simple solution that doesn't work in my case. Maybe something that I'm doing wrong, or that our gradle version is too old. I'll keep trying.

4) I have added code that checks for simple errors like not setting the correct urls and so on. if you find more common casses, please let me know.

Any feedback is welcomed. even if you can't read the code please try to install it and play with it (can be done on the same phone with xdrip). one only has to set NS addresses, and after a minute all data will be in.

AdrianLxM commented 8 years ago

I cannot test this at the moment, as I have no NightScout running. :(

Looks like a lot of work you did :+1:

A few remarks from a first quick glance:

  1. CollectionServiceStarter.java - it looks as if everything got exchanged (no proper diff).
  2. The same for app/src/main/res/xml/pref_data_sync.xml
  3. I would like to have the XDrip-Viewer and the Wifi-Wixel (or what was it before?) in a separate class as a separate service.
  4. "IsxDripViewerConfigured" -> "isXDrip..." with small "i".
  5. I know we don't have it to distinguish things between Share and Wixel input and it already is hard finding the correct create method in BgReadings e.g.. It would be nice if you could tag the new methods somehow. Just a //xDripViewer before the create method would be sufficient for me.
tzachi-dar commented 8 years ago

Did first pass of changes from code review. Still need more time to split the wifi-wixel.

you can point the viewer to https://secsecsecsec@snirdar.azure... to see how things look....

Thanks Tzachi

tzachi-dar commented 8 years ago

OK, Done all comments from first code review.

Please let me know if there are any more comments.

tzachi-dar commented 8 years ago

This was actualy merged a long time ago.