JohanDegraeve / xdripswift

xdrip for iOS, written in Swift
GNU General Public License v3.0
329 stars 328 forks source link

Fix `Payload maximum size exceeded` error when starting Live Activity #513

Closed stami closed 6 months ago

stami commented 6 months ago

Live Activities have maximum payload size of 4kB.

At least Libre 2 EU produces so many values over 12 hours (some 400 readings for me) that they cannot be pushed to Live Activity unfiltered, causing Live Activity start to fail with error: Payload maximum size exceeded.

Fix by downsampling (just pick every nth reading) to maximum of 100 readings.

The threshold value is got by trial and error, then tuning it down a bit to leave some room for different length strings.

paulplant commented 6 months ago

Well spotted @stami 👏

stami commented 6 months ago

Also @paulplant if it's OK and not already in progress, I'd like to make the live activity chart to use the same visible length (3h / 6h / 12h / 24h) than the main chart. I'll start working on that now.

I'm normally using 3h view and would very much prefer also the live activity to use that for increased detail.

paulplant commented 6 months ago

Also @paulplant if it's OK and not already in progress, I'd like to make the live activity chart to use the same visible length (3h / 6h / 12h / 24h) than the main chart. I'll start working on that now.

I'm normally using 3h view and would very much prefer also the live activity to use that for increased detail.

This is a bit more tricky. I added a "normal" size live activity for users that wanted just a shorter timeline (I think it's set at 3 hours).

The problem with what you're proposed is the scaling of the chart will be really off. All widgets, complications and live activities are using specific chart hours for each widget size in an attempt to keep the chart scaling as uniform as possible and I tested this with various users to end up at the current values.

And although I'd like to remove settings options instead of adding them, if you think that this change really adds value (and not just a personal preference), then maybe we could it it as a switch to development settings to see if people can use it and prefer it before taking any decisions.

stami commented 6 months ago

Yeah, thanks! I just also noticed I can use the "normal" size activity to get the three hour view. So never mind my earlier comment! 😁

But I might do some optimisation for the content state to fully utilise the available payload size for also the shorter chart, as now the downsampling is "too much" than necessary for the 3 hour's window.

stami commented 6 months ago

Uhh, I don't think that's possible after all, as the expanded dynamic island needs the full 12 hours data in any case! 😁 It is fine as it is 👍

paulplant commented 6 months ago

Uhh, I don't think that's possible after all, as the expanded dynamic island needs the full 12 hours data in any case! 😁 It is fine as it is 👍

Yes, if 100 readings plus the state data keeps it under 4Kb, then it's fine. No need to trim it further if it works well for everyone.

stami commented 6 months ago

Yes. For my setup (Libre 2 Direct) 110 readings was still fine but 120 was too much. So I think 100 should be quite safe.

paulplant commented 6 months ago

@stami We merged this directly into develop but then I added my other commits on top and tested it, but the downsampling looks really rough as it just removes one reading every "x" readings... This would be fine if we used a line graph, but with points it looks bad...I don't think we can use it like this:

AFDA1BFE-E868-4C94-AEF6-EC28597D1647_1_201_a

Is it supposed to be like this? Or did I do something wrong whilst merging it in?

stami commented 6 months ago

I see. I think it doesn't look bad for me just by accident, as I have so much more samples in the original data. 🤔

But yes, it was very rough "algorithm"...

stami commented 6 months ago

I see a couple of solutions to this.

Downsample - or resample - the data with interpolation so that the dots are spaced evenly on the projected line between the actual samples. This would cause a slight reshaping of the data, for example removing a single red dot from a top of peak. Also the dots would not be the actual data points after such transformation.

Refactor the XDripWidgetAttributes.ContentState data structures so that they take less space. I did a quick test and managed to almost triple the number of readings it was able to send to the live activity. I'll submit another PR of that so that you can check it out. For inspiration if not anything else. 😁

With those changes I can push 275 readings to the activity, hopefully significantly lessen the visual roughness you are seeing. 🤔

In addition to those, one could change the "computed properties" to be actually computed on demand and not calculated in the init, also - a bit bigger task - to move the limits and data source description etc to be part of the XDripWidgetAttributes and not the ContentState. They don't change all the time and It would be fine to reset the live activity in case some of those were changed.

I don't quite understand how iOS is serialising the ContentState and measuring the bytes as my naive byte calculations don't match up with the reported byte sizes in the error it's throwing when it goes over...

stami commented 6 months ago

Opened a draft PR #517 with the mentioned changes. Let me know what you think 😁

Another thing that came to my mind would be to downsample the earlier end of the readings array more coarsely and leave the latest readings as is. 🤔

paulplant commented 6 months ago

Downsample - or resample - the data with interpolation so that the dots are spaced evenly on the projected line between the actual samples. This would cause a slight reshaping of the data, for example removing a single red dot from a top of peak. Also the dots would not be the actual data points after such transformation.

I thought about this too, but not sure how costly it would be from a processing point of view. I don't think that the reshaping should be that much of a problem, but it could make the chart incoherent compared to what the user is seeing in the main app as you mention.

  • Changed bgReadingValues: [Double] to [Float16], lowering needed bytes per sample from 64 to 16.
  • Changed bgReadingDates: [Date] to have only the earliest date as Date and all other as seconds since that. Also lowering bytes needed from 64 to 16.

With those changes I can push 275 readings to the activity, hopefully significantly lessen the visual roughness you are seeing. 🤔

Excellent idea... this could probably be the best solution. Having the full 12 hour of data points gives us more scope to play with things.

I also thought about limiting to just 8 hours of data which, for Dexcom, would be 96 data points instead of 144 and therefore your filtering wouldn't affect the values. It would only start filtering for 60-second values like you get with L2 BLE and which look fine after filtering.

But I think your above solution to reduce footprint by changing the variable type is much more elegant and much better programming.

In addition to those, one could change the "computed properties" to be actually computed on demand and not calculated in the init, also - a bit bigger task - to move the limits and data source description etc to be part of the XDripWidgetAttributes and not the ContentState. They don't change all the time and It would be fine to reset the live activity in case some of those were changed.

The high/low/urgent/mgDl/sensor description etc can be changed on the fly from one reading to another (if you user changes things) so I think it's better to keep them as ContentState so that they can be dynamically updated.