Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support paged cgm #270

Merged
merged 18 commits into from
Oct 22, 2016
Merged

Support paged cgm #270

merged 18 commits into from
Oct 22, 2016

Conversation

tmecklem
Copy link
Contributor

@tmecklem tmecklem commented Oct 20, 2016

Summary: Add support for fetching and displaying recent data from cgm pages. See #200

This PR adds to MinimedKit the ability to fetch and decode cgm pages into events similar to the way that history pages are processed. It also supports fetching the pointer to the current page of cgm data to support fetching the most recent pages first.

In RileyLinkKit, functionality is built upon the additions to MinimedKit to fetch recent cgm data from the last 24 hours and display it similarly to the fetch recent history functionality.

Add GlucosePage CRC check tests and Data extension to reverseBytes since
GlucosePages are easier to decode in reverse.
In writing this test and the code that passed the test, I encountered a
strange mismatch from decocare. Decocare had more sgv entries, but they
had values of 0. Additional discovery and discussion led to a change in
decocare: openaps/decocare#11
Independent glucose events are events that have a timestamp in the 
record and do not serve as a reference timestamp for other records.
Reference timestamped records are used to apply a timestamp to other 
records using an offset (5 minutes).
Relative timestamp records need a timestamp, but don’t have one in their
own raw data, so that’s not included in this commit.
Clean up test output a little to help in troubleshooting
Copy link
Owner

@ps2 ps2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work, Tim! Looking forward to getting this into Loop. It would also be nice to add functionality for CGM upload to NS for those users using just the RL app, though I think that's a smaller set.

@@ -71,6 +71,14 @@ extension Data {
*/
}

extension Data {
func reverseBytes() -> Data {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a method in the standard library to do this already. See https://developer.apple.com/reference/foundation/data/1780245-reversed

Copy link
Contributor Author

@tmecklem tmecklem Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble figuring out what to do with the data structure that reversed() gives me. It's a ReversedRandomAccessCollection<Data>, and it looks like I have to use a different Index structure. The change seems to leak all the way through to how I access rawData GlucoseEvent and seems messy. Can you take a look and help me understand how to appropriately use reversed() on the page Data?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Data(data.reversed()) to create a new reversed data object. There may be ways to work with RandomAccessCollection types, and avoid the copy, but I wasn't able to get that working in my swift playground.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Change made.

var events = [TimestampedGlucoseEvent]()

// Going to scan backwards in time through events, so event time should be monotonically decreasing.
let eventTimestampDeltaAllowance = TimeInterval(hours: 9)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be necessary, unless you have encountered out of order glucose events, and need to keep looking back further in history to ensure you get all events since startDate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are out of order events due to relative timestamping. I changed the timestamp check on f266ebb#diff-bf2d04fbde756ea81dd46dafafbdacddR589 to use the reference timestamped events only. That should ensure that all relevant glucose events are captured since startDate at the expense of maybe capturing a few extra independently timestamped (non-sgv) entries.

idx = top
}

let page = try GlucosePage(pageData: pageData, pumpModel: pumpModel)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I searched and didn't see any actual dependency on pumpModel in the data parsing. Can this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! 👍

Glucose events can be out of order (and are frequently). Checking for
only reference timestamps should catch all relevant events.
- failure(error): An error describing why the command failed

*/
public func getGlucoseHistoryEvents(since startDate: Date, completion: @escaping (Either<(events: [TimestampedGlucoseEvent], pumpModel: PumpModel), Error>) -> Void) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does pumpModel need to be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope!

@tmecklem
Copy link
Contributor Author

I'm planning to get this version of the app installed on the phone of someone using a 722 pump with enlite before the start of the weekend for Real World:tm: testing.

@ps2 ps2 merged commit 92c239e into ps2:dev Oct 22, 2016
@tmecklem tmecklem deleted the support-paged-cgm branch October 24, 2016 02:07
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.

2 participants