Skip to content

Conversation

@ELUTE
Copy link
Contributor

@ELUTE ELUTE commented Jul 29, 2014

  • Implemented UI design from Kate F
  • New battery indicator from Jon M
  • Bigger fonts in general, especially for time
  • New special value icons instead of special value numbers (??? instead
    of "10")
  • Switched black and white backgrounds, to improve readability
  • New space for T1D name
  • Made JS faster based on inputs from John C and Christine D, to help
    with reliability and refresh problems
  • New "two time" fix, to quickly diagnose refresh problem (when pebble
    app has crashed or gotten lost)
  • Time and date are from system, so always up to date and watch can
    function like a watch
  • More descriptive error messages
  • Implemented bug fixes, especially for Rajat build users
  • Commented code in much greater detail
  • Will publish FAQ to quickly identify problems and address faster

* Implemented UI design from Kate F
* New battery indicator from Jon M
* Bigger fonts in general, especially for time
* New special value icons instead of special value numbers (??? instead
of "10")
* Switched black and white backgrounds, to improve readability
* New space for T1D name
* Made JS faster based on inputs from John C and Christine D, to help
with reliability and refresh problems
* New "two time" fix, to quickly diagnose refresh problem (when pebble
app has crashed or gotten lost)
* Time and date are from system, so always up to date and watch can
function like a watch
* More descriptive error messages
* Implemented bug fixes, especially for Rajat build users
* Commented code in much greater detail
* Will publish FAQ to quickly identify problems and address faster
@jasoncalabrese
Copy link
Member

Can't wait to test this out

@ELUTE
Copy link
Contributor Author

ELUTE commented Jul 29, 2014

There are a few icon changes pending... If you test it and have a cropped icon, please let me know. I am trying to maximize the size of I icons so I am sizing them individually.

Sent from my iPhone

On Jul 29, 2014, at 1:30 PM, Jason Calabrese [email protected] wrote:

Can't wait to test this out


Reply to this email directly or view it on GitHub.

@jasoncalabrese
Copy link
Member

On my watch!

Should the bottom clock be updating even when the watch is offline? That doesn't seem to be happening. I was hoping the X min ago would also go up even without a connection. Maybe putting my phone in airplane mode isn't the best way to test?

@jasoncalabrese
Copy link
Member

I like the logo when loading, I always hated the loading since it was slow, but with the logo, the nicer status, and clock already visible I don't mind it.

Some more things I found:

  • My fixes from fa3617e were lost, the trend field isn't consistent across versions (and should be ignored), but it the direction field is consistent. We shouldn't need the isRajatBuild check at all if we use the direction field like in the commit above.
  • The single up arrow is too small with a 2 digit number (same with 3 digit), so there is a black line on the right side

png

  • It's hard to see what was really changed, since all the indenting changed github shows that everything changed
  • I've never seen the 2 clocks show different values, is the upper clock the time of the last reading?

@ELUTE
Copy link
Contributor Author

ELUTE commented Jul 30, 2014

I have a new arrow. Small time is CGM time. Christine found that sometimes the pebble would read 3 mins ago etc but get stuck... This allows the user to see there is a problem when the times do not match.

On Jul 29, 2014, at 10:45 PM, Jason Calabrese [email protected] wrote:

I like the logo when loading, I always hated the loading since it was slow, but with the logo, the nicer status, and clock already visible I don't mind it.

Some more things I found:

My fixes from fa3617e were lost, the trend field isn't consistent across versions (and should be ignored), but it the direction field is consistent. We shouldn't need the isRajatBuild check at all if we use the direction field like in the commit above.
The single up arrow is too small with a 2 digit number (same with 3 digit), so there is a black line on the right side

It's hard to see what was really changed, since all the indenting changed github shows that everything changed
I've never seen the 2 clocks show different values, is the upper clock the time of the last reading?

Reply to this email directly or view it on GitHub.

@YYGIRL
Copy link
Contributor

YYGIRL commented Jul 30, 2014

I pulled the original code from the master, things shouldn't have gotten lost. And I tried to format the code to conform the coding standards, plus add comments where I could so that it would make it easier for everyone to understand and update it going forward. Wasn't trying to confuse things.

Two times - The little time is from the JS / pebble app and the big time is from the C routine. I've seen it many times where the timeago will say "4 mins ago" but the time did not update. It will be out of sync by hours. This happens when the pebble app / javascript dies and doesn't return an error, so the watch thinks it's still talking for some reason and you don't get a data update. I've seen it happen on my husband's watch a lot, and it's the relatively common "data refresh" problem. So people look at their watch and think they are up to date, and they are really not. Take the case of the person today. They think their child is 104 going down but what if it had said "4 mins ago"? But it was really from an hour ago? And you didn't notice the time? Now if the times don't match you'll know. I've talked this through with John Costik as well, and he admits it's a common problem too. It doesn't happen as much after we implemented message queue-ing, but it still happens because the JS / pebble app is never going to be as reliable as we want - especially in a real-time environment like we have here. If you read the pebble documentation, it very clearly states that it was never meant for any real-time applications. This allows us to flag an error, and someone can switch the watch face which will usually restart the JS / app and update the data.

So note you can triangulate data refresh problems now - the "ago" time is from the dexcom, the little time is from the JS / pebble app, and the big time is from the watch. So if something is off, you know where the problem is and how to fix it.

IsRajat - Specifically with the arrows. I think the direction field is the string field? The JS was handling it through a string parse and a string compare before, which was slowing down the JS. Now that there is no parsing and comparing, and it's passing the number straight to the C code, things are running much faster. I know there are two integer arrays in the C code, but it's helped a lot. I actually tracked and saw it in the logs. One of the big issues with the data refresh problem is the slowness of the JS, and again this helps that. I talked this through with John Costik as well, and he agreed. Plus the trend field number was still going back to the C code, not the direction field? I was confused on that too, since that would mean the C code would still report the wrong arrow for Rajat users. I never had the right arrows and just ended up using my own load for the last month or so, so don't really know.

Airplane mode - I didn't test it in that state - I thought the watch would go offline, like it does when the bluetooth is off. Need to see if it's returning an error that we can flag.

@jasoncalabrese
Copy link
Member

(sorry this got long, but wanted to try explaining why...)

2 Clocks

I understand what they are for, but I don't think it's fully working. Right now the time on the bottom clock only update when the we get a response from the server. If there is some communication issue, bluetooth, wifi, server, cell, etc the time doesn't change. It would be great if the bottom time/date was only dependent on the pebble, and even in airplane mode would stay current. If we could do that then we can always update the X time ago status, no matter what. To make that work the X time ago status would need to be formatted in the c code based on a last entry timestamp that would stay in memory, or even persisted.

Maybe TickTimerService would work? More info here: http://developer.getpebble.com/2/guides/event-service-guide.html

Not saying this has to be done on this branch, but if it could we'd have a much more reliable system.

Direction vs Trend

We should pretend the trend field doesn't exist, I've been planning to rip it out, but didn't want to break people using older watch faces.

history
Way back I aligned the trends ids on the pebble to match the the trend ids on android and the dexcom, but right before I did that @rajatgupta431 forked my fork, and went on to setup tons of people.

The solution that we figured out was to ignore the trend field and only use the direction string. It's the same value everywhere, in the android code, in the server code, in mongo, and now in the pebble.

We just need to add DIRECTIONS{} and directionToTrend() back use currentDirection for the alerts and then convert currentDirection to the trend id before sending to the c code

@YYGIRL
Copy link
Contributor

YYGIRL commented Jul 30, 2014

Clocks - when the watch loses bluetooth, it says watch offline. We would just need a solution to come up with an error when we lose cellular and wifi. It's also not very intuitive either, which I think is a problem too. However, I feel very safe now when those two clocks match up. Haven't checked out that function yet.

Direction to Trend - I understand. However, it was making the JS a lot slower. And when the trend id is returned to the c code, isn't it still returning the wrong value for the rajat users? I thought I saw that in the debug logs.

The current data refreshing problems, as it pertains to the pebble app and the JS (which I have also seen many people comment on as well), I can consistently reproduce on my husband's phone and watch. (The message Q load made my watch setup better, but not his.) He has been running this new load for the past few days and he said it is now much more reliable. He has never said that before, and he complains about it constantly. I can also see a significant speed difference with the JS, with the various changes I made, which I have never seen before either. If the JS can keep up, then it can't die or get lost or stop responding.

My only motivation is to make things more reliable. However, it's your community and your code. It's easy to identify and separate out the UI changes, and you already know about the battery changes.

@hackingtype1
Copy link
Contributor

TREND issue - Rajat needs to fix the issues that creates the need for isRajatBuild(), but until then, YY's code is a more efficient implementation than previous fixes.

Formatting changes hiding code changes - this is a pretty small set of code, and using proper formatting from this point on is desirable, with the good commenting, I don't see this as a hindrance to future development.

Going forward - 90% or more of the logic in the JS needs to go up to the API, and this will open the door to using android wear and many other devices with the same endpoint.

@jasoncalabrese
Copy link
Member

I don't see how directionToTrend could possibly be slow, it's a simple in memory lookup that happens 1 time a minute.

Rajat isn't going to be able to fix it for his users, there is no automation in his deploy so he'd need to manually fix it 96 times.

Detecting the version by looking for Heroku won't work since some people including Rajat are deploying the community version at Heroku.

@hackingtype1
Copy link
Contributor

Okay, then please merge your trend handling code in, and let's test it out - the rest of the features are too good to hold up over the trend transform issues.

@jasoncalabrese
Copy link
Member

@hackingtype1, I can't push commits to this branch since it's in @YYGIRL's repo. I could create a new branch in the nightscout repo, merge these changes in, and then restore the directionToTrend code. To do that I'd need to close this PR and create a new one.

Also @hackingtype1, in your version do you use the TickTimerService? Using that for the bottom clock would be ideal, then we could always trust the time on the bottom. The clock shouldn't need a response from the server to update.

One more thing with the JS code, that I'm not sure everyone know (it confused me for a while). It runs on the phone, not on the pebble, so we shouldn't have to be very worry about performance for things like this.

@ELUTE
Copy link
Contributor Author

ELUTE commented Jul 30, 2014

It would be great to have the time stay when the bluetooth is off. Agreed.

Rajat is no longer providing the Rajat Version... only the community version. I am sure many of the users will make the switch anyway and we could just tell them that they use the other pebble app until they do.

The battery piece is excellent. I really hope that it can be put in the uploader soon so that the community can take advantage of it. We are also working on having it alert user via vibe when it gets to 10% and flash the icon to make it stand out more...

On Jul 30, 2014, at 10:55 AM, Jason Calabrese [email protected] wrote:

@hackingtype1, I can't push commits to this branch since it's in @YYGIRL's repo. I could create a new branch in the nightscout repo, merge these changes in, and then restore the directionToTrend code. To do that I'd need to close this PR and create a new one.

Also @hackingtype1, in your version do you use the TickTimerService? Using that for the bottom clock would be ideal, then we could always trust the time on the bottom. The clock shouldn't need a response from the server to update.

One more thing with the JS code, that I'm not sure everyone know (it confused me for a while). It runs on the phone, not on the pebble, so we shouldn't have to be very worry about performance for things like this.


Reply to this email directly or view it on GitHub.

@hackingtype1
Copy link
Contributor

Branch from YY's repo, merge - go ahead and close the PR and create a new
one

I do not use the TickTimerService - I have additional notifications and
alerts for stale data that get pushed from my backend, so I have not needed
a true "clock" on my view. I post from the JS that my device is active, and
the backend service will push a notification to my native app if it hasn't
seen the JS post in 10-15 minutes.

I'm aware that the JS runs on the phone, and though we shouldn't *have *to
worry about performance - with the wide array of Android devices, and to a
lesser extent, the iPhone variations in firmware (even at the carrier model
level) - its best to handle anything other than the "offline" error at the
api level.

On Wed, Jul 30, 2014 at 10:55 AM, Jason Calabrese [email protected]
wrote:

@hackingtype1 https://github.com/hackingtype1, I can't push commits to
this branch since it's in @YYGIRL https://github.com/YYGIRL's repo. I
could create a new branch in the nightscout repo, merge these changes in,
and then restore the directionToTrend code. To do that I'd need to close
this PR and create a new one.

Also @hackingtype1 https://github.com/hackingtype1, in your version do
you use the TickTimerService? Using that for the bottom clock would be
ideal, then we could always trust the time on the bottom. The clock
shouldn't need a response from the server to update.

One more thing with the JS code, that I'm not sure everyone know (it
confused me for a while). It runs on the phone, not on the pebble, so we
shouldn't have to be very worry about performance for things like this.


Reply to this email directly or view it on GitHub
#13 (comment).

@jasoncalabrese
Copy link
Member

@hackingtype1 @ELUTE @YYGIRL, I'll do this tonight, in the mean time if there are any changes you can continue pushing commits to yygirl/master and I'll make sure to include them.

@jasoncalabrese
Copy link
Member

Moved to #14 for easier collaboration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants