nightscout / cgm-remote-monitor

nightscout web monitor
GNU Affero General Public License v3.0
2.42k stars 71.77k forks source link

/pebble doesn't use standard JSON #477

Closed shanselman closed 9 years ago

shanselman commented 9 years ago

The JSON exposed by the /pebble endpoint isn't proper/correct/valid JSON. This makes it hard to pull from other devices like Arduino, etc as strict JSON parsers (like ArduinoJson) can't handle it.

Fortunately it's easily fixed, we just add quotes around member names and make sure JSONlint.com agrees.

Was this done perhaps as a concern of data plans? Gzip'ed HTTP responses should mean any data size issues are nominal.

YYGIRL commented 9 years ago

Yes, concern of data plans. We've been having a lot of issues with that lately, especially with the addition of raw data.

We would need to test the gzip solution and see if we can keep down the data usage. The pebble seems to handle responses differently between Droid and iOS too. Theory and practice don't always match.

On Fri, Mar 13, 2015 at 1:06 PM, Scott Hanselman notifications@github.com wrote:

The JSON exposed by the /pebble endpoint isn't proper/correct/valid JSON. This makes it hard to pull from other devices like Arduino, etc as strict JSON parsers (like ArduinoJson) can't handle it.

Fortunately it's easily fixed, we just add quotes around member names and make sure JSONlint.com agrees.

Was this done perhaps as a concern of data plans? Gzip'ed HTTP responses should mean any data size issues are nominal.

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477.

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

Hm...maybe I am mistaken. Some endpoints DO have quotes? Does your endpoint validate on jsonlint.com @YYGIRL ?

Honest question, why are folks sweating data plans so much? I feel like we're fighting sometimes over bytes to save literal pennies.

YYGIRL commented 9 years ago

A lot of users are on the Ting plan, which is $9/month for 200M.

On Fri, Mar 13, 2015 at 2:43 PM, Scott Hanselman notifications@github.com wrote:

Hm...maybe I am mistake. Some endpoints DO have quotes?

Honest question, why are folks sweating data plans so much? I feel like we're fighting sometimes over bytes to save literal pennies.

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79238323 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

Ah, gotcha. Honestly a LOT of work goes into trying to keep it under $9 a month.

Ok, what about Gzip? That's a OLD and reliable standard, does it work on Pebble? Could be easily added to the web.config on Azure for testing.

I'm going to add this to my web.config and test:

<system.webServer>
  <httpCompression directory="%SystemDrive%\inetpub\
temp\IIS Temporary Compressed Files">
    <scheme name="gzip" dll="%Windir%\system32\inetsrv\gzip.dll"/>
    <dynamicTypes>
      <add mimeType="text/*" enabled="true"/>
      <add mimeType="message/*" enabled="true"/>
      <add mimeType="application/javascript" enabled="true"/>
      <add mimeType="*/*" enabled="false"/>
    </dynamicTypes>
    <staticTypes>
      <add mimeType="text/*" enabled="true"/>
      <add mimeType="message/*" enabled="true"/>
      <add mimeType="application/javascript" enabled="true"/>
      <add mimeType="*/*" enabled="false"/>
    </staticTypes>
  </httpCompression>
  <urlCompression doStaticCompression="true" doDynamicCompression="true"/>
</system.webServer>
YYGIRL commented 9 years ago

I honestly don't know, and it's something that I discussed with Jason and Jim yesterday. We haven't done it before. It would need to be thoroughly tested, on both Droid and iOS, and then compare data usage. Let us know what you find out, thanks.

On Fri, Mar 13, 2015 at 2:55 PM, Scott Hanselman notifications@github.com wrote:

Ah, gotcha. Honestly a LOT of work goes into trying to keep it under $9 a month.

Ok, what about Gzip? That's a OLD and reliable standard, does it work on Pebble? Could be easily added to the web.config on Azure for testing.

I'm going to add this to my web.config and test:

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79250149 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

So, I just did this. My /pebble end point is gzipped (as is the whole site). The size is getting huge data improvements. Page went from 14k to 2k. Pebble endpoint by 50%. iPhone, Pebble, uploader on Android, all still working.

@jasoncalabrese We should enable Gzip for Azure.

YYGIRL commented 9 years ago

Stupid questions - Can you still see the endpoint contents at the endpoint address? Can you measure the data usage as seen / drawn by the pebble app?

On Fri, Mar 13, 2015 at 3:09 PM, Scott Hanselman notifications@github.com wrote:

So, I just did this. My /pebble end point is gzipped (as is the whole site). The size is getting huge data improvements. Page went from 14k to 2k. Pebble endpoint by 50%. iPhone, Pebble, uploader on Android, all still working.

@jasoncalabrese https://github.com/jasoncalabrese We should enable Gzip for Azure.

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79263586 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

I don't understand the question? https://hanselsugars.azurewebsites.net/pebble

jimsiff commented 9 years ago

Scott does this change gzip all responses from the server? AFAIK, the BG data is already gzipped when sent each minute so I wonder if the savings may be primarily on initial load and manual refreshes? Even so, it would definitely have a positive effect on manual page reloads and browsers set to not cache anything.

We should definitely gzip and standardize the Pebble JSON output to reduce data usage and improve compatibility.

jasoncalabrese commented 9 years ago

Don't see how it wouldn't be standard json, but we should add gziping, was discussing with @jimsiff last night. But would do it directly in node so it's portable, maybe we'd need an option to disable it.

    var result = { status: [ {now: now} ], bgs: sgvData, cals: calData };
    res.setHeader('content-type', 'application/json');
    res.write(JSON.stringify(result));
    res.end( );
jasoncalabrese commented 9 years ago

All other places use the cleaner res.json(result) style, but stringify should work

YYGIRL commented 9 years ago

Got it. I didn't know if gzipping caused the endpoint to go hidden for some reason. I see your contents there, obviously.

However, when you gzipped it, did you measure the data usage as seen by the pebble app? Like when the pebble app made the XML request - did it see a reduction in data? Does that make sense? I just want to see it as recorded by the pebble app.

Finally if we move everything to strings, I know the JS will still work, but we would need to reformat in the JS before going to the pebble watch. I need to send the CGM time and the NOW time as an integer to the watch, because it would be way too big as a string. Would also need to reformat noise as well to an integer. Is there an easy way to do that in JS?

We would need to redo the pebble watch load and it would affect backwards compatibility. Would need some hooks.

On Fri, Mar 13, 2015 at 3:33 PM, Jason Calabrese notifications@github.com wrote:

All other places use the cleaner res.json(result) style, but stringify should work

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79287766 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

Hang in though...someone give me your existing site. I want to see if gzip is already there, built into Azure.

shanselman commented 9 years ago

@YYGIRL The pebble app, as I understand it, doesn't make the request at all. It requests the android/iphone do it via proxy, that HTTP call happens on the phone, and then the string of /pebble's response is proxied over BT to the pebble. Pebble's don't have an HTTP stack AFAIK.

shanselman commented 9 years ago

Also @YYGIRL I was WRONG and need to fall on my sword about the strings things. There's a few points of confusion.

First, I was talking about NAMES, not values. This was my mistake, as the JSON viewer in Chrome strips them. So, to be clear this GitHub issue is wrong, and my mistake.

Second, but a little related, we DO use strings as VALUES for sgv and battery. There is NO technical reason I can see for that, and it just means others need to convert them ToInt().

Make sense? (sorry for the mess up)

YYGIRL commented 9 years ago

Yes, makes sense, thank you for clarifying.

And the smaller ones like SGV and battery are fine - it actually does help, because it goes into a 3 byte string as opposed to a 4 byte integer when going down to the watch. Our biggest memory problem is that we only have 120 bytes between the pebble app and the watch, and we're almost maxed out there. And the watch actually needs a string when posting, so we just compare and post. What we can't handle is a large time string going in as all bytes as part of a string.

On Fri, Mar 13, 2015 at 3:47 PM, Scott Hanselman notifications@github.com wrote:

Also @YYGIRL https://github.com/YYGIRL I was WRONG and need to fall on my sword about the strings things. There's a few points of confusion.

First, I was talking about NAMES, not values. This was my mistake, as the JSON viewer in Chrome strips them. So, to be clear this GitHub issue is wrong, and my mistake.

Second, but a little related, we DO use strings as VALUES for sgv and battery. There is NO technical reason I can see for that, and it just means others need to convert them ToInt().

Make sense? (sorry for the mess up)

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79301084 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

@YYGIRL Forgive my pebble ignorance, I'm confused. Pebble supports uint8 which is just one byte, as an insigned integer and that has enough room for BG values. Not all ints are 32 bits. Also, the communication between watch and phone is BT so there's no bandwidth issue. Thanks for your patience!

When you say 120 bytes, is that the max size of the string that a pebble app can pass between itself and the phone?

It makes sense what you're saying about "we need a string anyway." You're using the /pebble endpoint not as a generic data endpoint but literally as a "View Model" for the pebble.

All this talk (very useful BTW, thanks!) makes me feel I'd like a /device or /now or something that is a more generic (and rather a bit larger) JSON GET endpoint for the hacking I'm doing with Microsoft Band, Android Wear, and Apple Watch - a device non-specific endpoint for current BG.

Sounds good?

YYGIRL commented 9 years ago

Yes, you only get 120 bytes for the message that can pass between the pebble app itself and the phone. For each field passed, you can send a string, integer, or array. Each field can be any size you want, and we make them as small as possible.

Note there's a big difference on what pebble JS supports (the pebble app) and what pebble C supports (the pebble watch). We can get very specific on data formatting in pebble C, with integers etc, since we are working in a very constrained embedded environment. This will be the case for any smart watch.

On Fri, Mar 13, 2015 at 3:58 PM, Scott Hanselman notifications@github.com wrote:

@YYGIRL https://github.com/YYGIRL Forgive me, I'm confused. Pebble supports uint8 which is just one byte, as an insigned integer and that has enough room for BG values. Not all ints are 32 bits. Also, the communication between watch and phone is BT so there's no bandwidth issue.

When you say 120 bytes, is that the max size of the string that a pebble app can pass between itself and the phone?

It makes sense what you're saying about "we need a string anyway." You're using the /pebble endpoint not as a generic data endpoint but literally as a "View Model" for the pebble.

All this talk (very useful BTW, thanks!) makes me feel I'd like a /device or /now or something that is a more generic (and rather a bit larger) JSON GET endpoint for the hacking I'm doing with Microsoft Band, Android Wear, and Apple Watch - a device non-specific endpoint for current BG.

Sounds good?

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79314178 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org

shanselman commented 9 years ago

Gotcha, that makes total sense, and I can see if you included raw data it would be even more of a problem.

So are you seeing compression turned on by default on your sites? Perhaps this is already the default.

On Mar 13, 2015, at 1:22 PM, YYGIRL notifications@github.com wrote:

Yes, you only get 120 bytes for the message that can pass between the pebble app itself and the phone. For each field passed, you can send a string, integer, or array. Each field can be any size you want, and we make them as small as possible.

Note there's a big difference on what pebble JS supports (the pebble app) and what pebble C supports (the pebble watch). We can get very specific on data formatting in pebble C, with integers etc, since we are working in a very constrained embedded environment. This will be the case for any smart watch.

On Fri, Mar 13, 2015 at 3:58 PM, Scott Hanselman notifications@github.com wrote:

@YYGIRL https://github.com/YYGIRL Forgive me, I'm confused. Pebble supports uint8 which is just one byte, as an insigned integer and that has enough room for BG values. Not all ints are 32 bits. Also, the communication between watch and phone is BT so there's no bandwidth issue.

When you say 120 bytes, is that the max size of the string that a pebble app can pass between itself and the phone?

It makes sense what you're saying about "we need a string anyway." You're using the /pebble endpoint not as a generic data endpoint but literally as a "View Model" for the pebble.

All this talk (very useful BTW, thanks!) makes me feel I'd like a /device or /now or something that is a more generic (and rather a bit larger) JSON GET endpoint for the hacking I'm doing with Microsoft Band, Android Wear, and Apple Watch - a device non-specific endpoint for current BG.

Sounds good?

— Reply to this email directly or view it on GitHub https://github.com/nightscout/cgm-remote-monitor/issues/477#issuecomment-79314178 .

Join the Jack Attack! Fight Type 1 Diabetes _Give to the JDRF - _www.jdrf.org http://www.jdrf.org — Reply to this email directly or view it on GitHub.

jimsiff commented 9 years ago

@shanselman it looks like Azure is applying GZIP to all data out from the server by default. Heroku does not.