X22 sensor timestamp refinement #296
Conversation
Allow page decoding if no relative events are queued before sensor ts Also count a page end as a forward reference event since the page is closed already anyway. No new timestamp can be added even if we wanted.
| case gap | ||
| case unknown | ||
|
|
||
| public static func eventType(code: UInt8) -> SensorTimestampType { |
There was a problem hiding this comment.
This would be more semantic as an initializer:
public init(code: UInt8)
(also it might not need to be public)
| default: | ||
| timestampType = "unknown" | ||
| } | ||
| timestampType = SensorTimestampType.eventType(code: UInt8(d(3) >> 5) & 0b00000011) |
There was a problem hiding this comment.
Sorry for just noticing this now, but "d" is useless here as you're undoing the unnecessary Int cast. Coupled with the initializer change:
timestampType = SensorTimestampType(code: availableData[3] >> 5 & 0b11)is much cleaner
There was a problem hiding this comment.
I wondered if there wasn't a better way. Thanks for the feedback on this.
| } else if let event = event as? ReferenceTimestampedGlucoseEvent { | ||
| let offsetDate = calendar.date(byAdding: Calendar.Component.minute, value: 5 * relativeEventCount, to: event.timestamp.date!)! | ||
| } else if let sensorTimestampEvent = event as? SensorTimestampGlucoseEvent, | ||
| relativeEventCount == 0 || (event as! SensorTimestampGlucoseEvent).isForwardOffsetReference() { |
There was a problem hiding this comment.
Replace that last part with simply sensorTimestampEvent.isForwardOffsetReference() as your unwrapping statement is in-scope at this point.
There was a problem hiding this comment.
I thought I might be able to do that but Xcode was throwing a fit about it. Must have been a stale compiler error because it worked this time around. 👍
| } else if let sensorTimestampEvent = event as? SensorTimestampGlucoseEvent, | ||
| relativeEventCount == 0 || (event as! SensorTimestampGlucoseEvent).isForwardOffsetReference() { | ||
|
|
||
| let offsetDate = calendar.date(byAdding: Calendar.Component.minute, value: 5 * relativeEventCount, to: sensorTimestampEvent.timestamp.date!)! |
There was a problem hiding this comment.
byAdding: .minute, since the type is inferred.
Also, I'm not sure this line to be a calendar operation if you're just adding a double to a double (the next one absolutely does)
sensorTimestampEvent.timestamp.date?.addingTimeInterval(TimeInterval(minutes: 5 * relativeEventCount))
There was a problem hiding this comment.
I'm not sure how to go about changing this. Could you help me out with an example of what you think could improve it?
There was a problem hiding this comment.
I thought I did?
let offsetDate = sensorTimestampEvent.timestamp.date!.addingTimeInterval(TimeInterval(minutes: 5 * relativeEventCount))force-unwrapping should be avoided when possible, so I assume you're certain that timestamp.date is non-nil?
| } else if let sensorTimestampEvent = event as? SensorTimestampGlucoseEvent, | ||
| relativeEventCount == 0 || (event as! SensorTimestampGlucoseEvent).isForwardOffsetReference() { | ||
|
|
||
| let offsetDate = calendar.date(byAdding: Calendar.Component.minute, value: 5 * relativeEventCount, to: sensorTimestampEvent.timestamp.date!)! | ||
| var relativeTimestamp = calendar.dateComponents([.year, .month, .day, .hour, .minute], from: offsetDate) | ||
| relativeTimestamp.calendar = calendar |
There was a problem hiding this comment.
Are you sure you need this line?
There was a problem hiding this comment.
Which line are you referring to? 72?
There was a problem hiding this comment.
75/76. I assumed that creating DateComponents from a Calendar would result in components that included a reference to the calendar.
There was a problem hiding this comment.
Ah, good question. I'll try some things out.
|
@loudnate, I appreciate you taking some of your time to review this code and make it better. Both because it's not a language I'm all that familiar with yet, and because you know this codebase so well. 👍 |
Refines the cgm page timestamping and adds low and high sgv to nightscout